Introduce flag --incompatible_no_kwargs_in_build_files
Implements https://github.com/bazelbuild/bazel/issues/8021
When it is enabled, **kwargs and *args are not allowed in BUILD files.
Without this flag, the ban on **kwarg and *args is not fully working.
RELNOTES: New flag `--incompatible_no_kwargs_in_build_files`. See https://github.com/bazelbuild/bazel/issues/8021
PiperOrigin-RevId: 243325744
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 62cd394..0bf0114 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 @@
}
}
- 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 47ec752..7a1b53d 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
@@ -444,6 +444,20 @@
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",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -525,6 +539,7 @@
.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 adb4983..520d588 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 @@
}
}
- 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 65fc952..d4cb286 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 abstract boolean incompatibleNoAttrLicense();
+ public abstract boolean incompatibleNoKwargsInBuildFiles();
+
public abstract boolean incompatibleNoOutputAttrDefault();
public abstract boolean incompatibleNoSupportToolsInActionInputs();
@@ -218,6 +220,7 @@
.incompatibleExpandDirectories(true)
.incompatibleNewActionsApi(false)
.incompatibleNoAttrLicense(true)
+ .incompatibleNoKwargsInBuildFiles(false)
.incompatibleNoOutputAttrDefault(false)
.incompatibleNoSupportToolsInActionInputs(false)
.incompatibleNoTargetOutputGroup(false)
@@ -280,6 +283,8 @@
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 8c28293..e53fde0 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 @@
* **kwargs. This creates a better separation between code and data.
*/
public static boolean checkBuildSyntax(
- List<Statement> statements, final EventHandler eventHandler) {
+ List<Statement> 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 @@
+ "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 623206c..2b533f9 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 @@
"--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 @@
.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 df0847c..78700ea 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 @@
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 88bc26e..365ba1d 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 @@
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);
}
