diff options
author | Vladimir Moskva <vladmos@google.com> | 2017-03-23 16:54:28 +0000 |
---|---|---|
committer | Yue Gan <yueg@google.com> | 2017-03-23 17:54:02 +0000 |
commit | cc07b2d4102c6891eadfdbd009f744233b3925ec (patch) | |
tree | cd69a6de340534b41ef270591778044c48b27042 /src/test/java/com | |
parent | e182500db1f331fd39f7e5d6215e6fdedbb0791a (diff) |
Remove provider safety check and make all old ctx objects featureless
It's now allowed to return anything from a rule implementation function.
If a ctx object is returned it becomes featureless and cannot be used from
anywhere else (i.e. from an implementation function of another rule).
Fixes #2319
--
PiperOrigin-RevId: 151015919
MOS_MIGRATED_REVID=151015919
Diffstat (limited to 'src/test/java/com')
3 files changed, 115 insertions, 48 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java index 976c268e97..528fe99064 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java @@ -701,34 +701,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase { } @Test - public void aspectFailingReturnsUnsafeObject() throws Exception { - scratch.file( - "test/aspect.bzl", - "def foo():", - " return 0", - "def _impl(target, ctx):", - " return struct(x = foo)", - "", - "MyAspect = aspect(implementation=_impl)"); - scratch.file("test/BUILD", "java_library(name = 'xxx',)"); - - reporter.removeHandler(failFastHandler); - try { - AnalysisResult result = update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx"); - assertThat(keepGoing()).isTrue(); - assertThat(result.hasError()).isTrue(); - } catch (ViewCreationFailedException e) { - // expect to fail. - } - assertContainsEvent( - "ERROR /workspace/test/BUILD:1:1: in //test:aspect.bzl%MyAspect aspect on java_library rule" - + " //test:xxx: \n" - + "\n" - + "\n" - + "/workspace/test/aspect.bzl:4:11: Value of provider 'x' is of an illegal type: function"); - } - - @Test public void aspectFailingOrphanArtifacts() throws Exception { scratch.file( "test/aspect.bzl", diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 14f783e915..79231d663d 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -972,26 +972,6 @@ public class SkylarkIntegrationTest extends BuildViewTestCase { } @Test - public void testProviderValidation() throws Exception { - // This does not have full coverage for SkylarkProviderValidationUtil, which eventually - // should be factored into a more general way for validating that an object is deeply - // Skylark-permissible. It's just a regression test for specific issues. - reporter.removeHandler(failFastHandler); - scratch.file( - "test/skylark/extension.bzl", - "def _impl(ctx):", - " return struct(bad=depset([depset([])]))", - "my_rule = rule(implementation = _impl)"); - scratch.file( - "test/skylark/BUILD", - "load('/test/skylark/extension', 'my_rule')", - "my_rule(name = 'r')"); - - getConfiguredTarget("//test/skylark:r"); - assertContainsEvent("Value of provider 'bad' is of an illegal type: depset\n"); - } - - @Test public void testRecursionDetection() throws Exception { reporter.removeHandler(failFastHandler); scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index b5d7108dd7..ef609dbb83 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -42,6 +42,7 @@ import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.ArrayList; import java.util.List; import org.junit.Before; import org.junit.Test; @@ -1733,4 +1734,118 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { (Label) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.values()[0]"); assertThat(valueLabel).isEqualTo(Label.parseAbsolute("//jvm:runtime")); } + + // A list of attributes and methods ctx objects have + private final List<String> ctxAttributes = ImmutableList.of( + "attr", + "split_attr", + "executable", + "file", + "files", + "workspace_name", + "label", + "fragments", + "host_fragments", + "configuration", + "host_configuration", + "coverage_instrumented(dep)", + "features", + "bin_dir", + "genfiles_dir", + "outputs", + "rule", + "aspect_ids", + "var", + "tokenize('foo')", + "expand('foo', [], Label('//test:main'))", + "new_file('foo.txt')", + "new_file(file, 'foo.txt')", + "check_placeholders('foo', [])", + "action(command = 'foo', outputs = [file])", + "file_action(file, 'foo')", + "empty_action(mnemonic = 'foo', inputs = [file])", + "template_action(template = file, output = file, substitutions = {})", + "runfiles()", + "resolve_command(command = 'foo')"); + + @Test + public void testFrozenRuleContextHasInaccessibleAttributes() throws Exception { + scratch.file("test/BUILD", + "load('/test/rules', 'main_rule', 'dep_rule')", + "dep_rule(name = 'dep')", + "main_rule(name = 'main', deps = [':dep'])"); + scratch.file("test/rules.bzl"); + + for (String attribute : ctxAttributes) { + scratch.overwriteFile("test/rules.bzl", + "def _main_impl(ctx):", + " dep = ctx.attr.deps[0]", + " file = ctx.outputs.file", + " foo = dep.dep_ctx." + attribute, + "main_rule = rule(", + " implementation = _main_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " outputs = {'file': 'output.txt'},", + ")", + "def _dep_impl(ctx):", + " return struct(dep_ctx = ctx)", + "dep_rule = rule(implementation = _dep_impl)"); + invalidatePackages(); + try { + getConfiguredTarget("//test:main"); + fail("Should have been unable to access dep_ctx." + attribute); + } catch (AssertionError e) { + assertThat(e.getMessage()).contains("cannot access field or method '" + + attribute.split("\\(")[0] + + "' of rule context for '//test:dep' outside of its own rule implementation function"); + } + } + } + + @Test + public void testFrozenRuleContextForAspectsHasInaccessibleAttributes() throws Exception { + List<String> attributes = new ArrayList<>(); + attributes.addAll(ctxAttributes); + attributes.addAll(ImmutableList.of( + "rule.attr", + "rule.executable", + "rule.file", + "rule.files", + "rule.kind")); + scratch.file("test/BUILD", + "load('/test/rules', 'my_rule')", + "my_rule(name = 'dep')", + "my_rule(name = 'mid', deps = [':dep'])", + "my_rule(name = 'main', deps = [':mid'])"); + scratch.file("test/rules.bzl"); + for (String attribute : attributes) { + scratch.overwriteFile("test/rules.bzl", + "def _rule_impl(ctx):", + " pass", + "def _aspect_impl(target, ctx):", + " if ctx.rule.attr.deps:", + " dep = ctx.rule.attr.deps[0]", + " file = ctx.new_file('file.txt')", + " foo = dep." + (attribute.startsWith("rule.") ? "" : "ctx.") + attribute, + " return struct(ctx = ctx, rule=ctx.rule)", + "MyAspect = aspect(implementation=_aspect_impl)", + "my_rule = rule(", + " implementation = _rule_impl,", + " attrs = {", + " 'deps': attr.label_list(aspects = [MyAspect])", + " },", + ")"); + invalidatePackages(); + try { + getConfiguredTarget("//test:main"); + fail("Should have been unable to access dep." + attribute); + } catch (AssertionError e) { + assertThat(e.getMessage()).contains("cannot access field or method '" + + attribute.split("\\(")[0] + + "' of rule context for '//test:dep' outside of its own rule implementation function"); + } + } + } } |