diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 62cd3948c300cc..0bf0114695e20c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -1770,7 +1770,8 @@ public Package.Builder evaluateBuildFile( } } - if (!ValidationEnvironment.checkBuildSyntax(buildFileAST.getStatements(), eventHandler)) { + if (!ValidationEnvironment.checkBuildSyntax( + buildFileAST.getStatements(), eventHandler, pkgEnv)) { pkgBuilder.setContainsErrors(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 47ec7520b50248..7a1b53dce55c13 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -443,6 +443,20 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + "symbols introduced by load are not implicitly re-exported.") public boolean incompatibleNoTransitiveLoads; + @Option( + name = "incompatible_no_kwargs_in_build_files", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If set to true, *args and **kwargs are not allowed in BUILD files. See " + + "https://github.com/bazelbuild/bazel/issues/8021") + public boolean incompatibleNoKwargsInBuildFiles; + @Option( name = "incompatible_remap_main_repo", defaultValue = "false", @@ -525,6 +539,7 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleExpandDirectories(incompatibleExpandDirectories) .incompatibleNewActionsApi(incompatibleNewActionsApi) .incompatibleNoAttrLicense(incompatibleNoAttrLicense) + .incompatibleNoKwargsInBuildFiles(incompatibleNoKwargsInBuildFiles) .incompatibleNoOutputAttrDefault(incompatibleNoOutputAttrDefault) .incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs) .incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup) diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index adb4983b37c74f..520d588fefc24d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -206,7 +206,7 @@ private void execute( } } - if (!ValidationEnvironment.checkBuildSyntax(ast.getStatements(), localReporter) + if (!ValidationEnvironment.checkBuildSyntax(ast.getStatements(), localReporter, workspaceEnv) || !ast.exec(workspaceEnv, localReporter)) { localReporter.handle(Event.error("Error evaluating WORKSPACE file")); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 65fc9520644c73..d4cb286b4f4069 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -160,6 +160,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) { public abstract boolean incompatibleNoAttrLicense(); + public abstract boolean incompatibleNoKwargsInBuildFiles(); + public abstract boolean incompatibleNoOutputAttrDefault(); public abstract boolean incompatibleNoSupportToolsInActionInputs(); @@ -218,6 +220,7 @@ public static Builder builderWithDefaults() { .incompatibleExpandDirectories(true) .incompatibleNewActionsApi(false) .incompatibleNoAttrLicense(true) + .incompatibleNoKwargsInBuildFiles(false) .incompatibleNoOutputAttrDefault(false) .incompatibleNoSupportToolsInActionInputs(false) .incompatibleNoTargetOutputGroup(false) @@ -280,6 +283,8 @@ public abstract static class Builder { public abstract Builder incompatibleNewActionsApi(boolean value); + public abstract Builder incompatibleNoKwargsInBuildFiles(boolean value); + public abstract Builder incompatibleNoAttrLicense(boolean value); public abstract Builder incompatibleNoOutputAttrDefault(boolean value); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java index 8c282935d66644..e53fde0028e08f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java @@ -389,7 +389,7 @@ private void closeBlock() { * **kwargs. This creates a better separation between code and data. */ public static boolean checkBuildSyntax( - List statements, final EventHandler eventHandler) { + List statements, final EventHandler eventHandler, Environment env) { // Wrap the boolean inside an array so that the inner class can modify it. final boolean[] success = new boolean[] {true}; // TODO(laurentlb): Merge with the visitor above when possible (i.e. when BUILD files use it). @@ -442,6 +442,9 @@ public void visit(FuncallExpression node) { + "explicitly."); } } + if (env.getSemantics().incompatibleNoKwargsInBuildFiles()) { + super.visit(node); + } } }; checker.visitAll(statements); diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 623206c06d32c3..2b533f9dcf49ea 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -148,6 +148,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_expand_directories=" + rand.nextBoolean(), "--incompatible_new_actions_api=" + rand.nextBoolean(), "--incompatible_no_attr_license=" + rand.nextBoolean(), + "--incompatible_no_kwargs_in_build_files=" + rand.nextBoolean(), "--incompatible_no_output_attr_default=" + rand.nextBoolean(), "--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(), "--incompatible_no_target_output_group=" + rand.nextBoolean(), @@ -192,6 +193,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleExpandDirectories(rand.nextBoolean()) .incompatibleNewActionsApi(rand.nextBoolean()) .incompatibleNoAttrLicense(rand.nextBoolean()) + .incompatibleNoKwargsInBuildFiles(rand.nextBoolean()) .incompatibleNoOutputAttrDefault(rand.nextBoolean()) .incompatibleNoSupportToolsInActionInputs(rand.nextBoolean()) .incompatibleNoTargetOutputGroup(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java index df0847c83d2c99..78700ea4b5e856 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java @@ -708,4 +708,17 @@ public void testArgsForbiddenInBuild() throws Exception { new BuildTest() .testIfErrorContains("*args arguments are not allowed in BUILD files", "func(*array)"); } + + @Test + public void testIncompatibleKwargsInBuildFiles() throws Exception { + new BuildTest("--incompatible_no_kwargs_in_build_files=true") + .testIfErrorContains( + "kwargs arguments are not allowed in BUILD files", "len(dict(**{'a': 1}))"); + + new BuildTest("--incompatible_no_kwargs_in_build_files=false") + .testStatement("len(dict(**{'a': 1}))", 1); + + new SkylarkTest("--incompatible_no_kwargs_in_build_files") + .testStatement("len(dict(**{'a': 1}))", 1); + } } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java index 88bc26e883fc01..365ba1d55afbd5 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java @@ -201,7 +201,7 @@ public Object eval(String... input) throws Exception { return BuildFileAST.eval(env, input); } BuildFileAST ast = BuildFileAST.parseString(env.getEventHandler(), input); - ValidationEnvironment.checkBuildSyntax(ast.getStatements(), env.getEventHandler()); + ValidationEnvironment.checkBuildSyntax(ast.getStatements(), env.getEventHandler(), env); return ast.eval(env); }