Enable and remove the flag for --incompatible_restrict_attribute_names
RELNOTES: incompatible_restrict_attribute_names is enabled (https://github.com/bazelbuild/bazel/issues/6437)
PiperOrigin-RevId: 263841325
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 9f65aec..949c69d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -417,15 +417,9 @@
return new SkylarkRuleFunction(builder, type, attributes, ast.getLocation());
}
- private static void checkAttributeName(Location loc, Environment env, String name)
- throws EvalException {
- if (env.getSemantics().incompatibleRestrictAttributeNames() && !Identifier.isValid(name)) {
- throw new EvalException(
- loc,
- "attribute name `"
- + name
- + "` is not a valid identfier. "
- + "This check can be disabled with `--incompatible_restrict_attribute_names=false`.");
+ private static void checkAttributeName(Location loc, String name) throws EvalException {
+ if (!Identifier.isValid(name)) {
+ throw new EvalException(loc, "attribute name `" + name + "` is not a valid identifier.");
}
}
@@ -438,7 +432,7 @@
castMap(attrs, String.class, Descriptor.class, "attrs").entrySet()) {
Descriptor attrDescriptor = attr.getValue();
AttributeValueSource source = attrDescriptor.getValueSource();
- checkAttributeName(loc, env, attr.getKey());
+ checkAttributeName(loc, attr.getKey());
String attrName = source.convertToNativeName(attr.getKey(), loc);
attributes.add(Pair.of(attrName, attrDescriptor));
}
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 b2b6e14..74d9280 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
@@ -615,18 +615,6 @@
public boolean incompatibleRestrictNamedParams;
@Option(
- name = "incompatible_restrict_attribute_names",
- defaultValue = "false",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help = "If set to true, restrict rule attribute names to valid identifiers")
- public boolean incompatibleRestrictAttributeNames;
-
- @Option(
name = "incompatible_depset_for_libraries_to_link_getter",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -703,7 +691,6 @@
.incompatibleObjcFrameworkCleanup(incompatibleObjcFrameworkCleanup)
.incompatibleRemapMainRepo(incompatibleRemapMainRepo)
.incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
- .incompatibleRestrictAttributeNames(incompatibleRestrictAttributeNames)
.incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)
.incompatibleRunShellCommandString(incompatibleRunShellCommandString)
.incompatibleStringJoinRequiresStrings(incompatibleStringJoinRequiresStrings)
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 57f017f..fad687d 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
@@ -191,8 +191,6 @@
public abstract boolean incompatibleRemoveNativeMavenJar();
- public abstract boolean incompatibleRestrictAttributeNames();
-
public abstract boolean incompatibleRestrictNamedParams();
public abstract boolean incompatibleRunShellCommandString();
@@ -281,7 +279,6 @@
.incompatibleRemapMainRepo(false)
.incompatibleRemoveNativeMavenJar(false)
.incompatibleRunShellCommandString(false)
- .incompatibleRestrictAttributeNames(false)
.incompatibleRestrictNamedParams(false)
.incompatibleStringJoinRequiresStrings(true)
.internalSkylarkFlagTestCanary(false)
@@ -369,8 +366,6 @@
public abstract Builder incompatibleRemoveNativeMavenJar(boolean value);
- public abstract Builder incompatibleRestrictAttributeNames(boolean value);
-
public abstract Builder incompatibleRestrictNamedParams(boolean value);
public abstract Builder incompatibleRunShellCommandString(boolean value);
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 d7d1f87..c41d770 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
@@ -158,7 +158,6 @@
"--incompatible_objc_framework_cleanup=" + rand.nextBoolean(),
"--incompatible_remap_main_repo=" + rand.nextBoolean(),
"--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
- "--incompatible_restrict_attribute_names=" + rand.nextBoolean(),
"--incompatible_restrict_named_params=" + rand.nextBoolean(),
"--incompatible_run_shell_command_string=" + rand.nextBoolean(),
"--incompatible_string_join_requires_strings=" + rand.nextBoolean(),
@@ -213,7 +212,6 @@
.incompatibleObjcFrameworkCleanup(rand.nextBoolean())
.incompatibleRemapMainRepo(rand.nextBoolean())
.incompatibleRemoveNativeMavenJar(rand.nextBoolean())
- .incompatibleRestrictAttributeNames(rand.nextBoolean())
.incompatibleRestrictNamedParams(rand.nextBoolean())
.incompatibleRunShellCommandString(rand.nextBoolean())
.incompatibleStringJoinRequiresStrings(rand.nextBoolean())
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 36d5405..559e0ce 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
@@ -2558,13 +2558,14 @@
@Test
public void testBadWhitelistTransition_onNonLabelAttr() throws Exception {
+ String whitelistAttributeName = WHITELIST_ATTRIBUTE_NAME.replace("$", "_");
scratch.file(
"test/rules.bzl",
"def _impl(ctx):",
" return []",
"",
"my_rule = rule(_impl, attrs = {'"
- + WHITELIST_ATTRIBUTE_NAME
+ + whitelistAttributeName
+ "':attr.string(default = 'blah')})");
scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'my_rule')");
@@ -2575,12 +2576,13 @@
@Test
public void testBadWhitelistTransition_noDefaultValue() throws Exception {
+ String whitelistAttributeName = WHITELIST_ATTRIBUTE_NAME.replace("$", "_");
scratch.file(
"test/rules.bzl",
"def _impl(ctx):",
" return []",
"",
- "my_rule = rule(_impl, attrs = {'" + WHITELIST_ATTRIBUTE_NAME + "':attr.label()})");
+ "my_rule = rule(_impl, attrs = {'" + whitelistAttributeName + "':attr.label()})");
scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'my_rule')");
reporter.removeHandler(failFastHandler);
@@ -2590,13 +2592,14 @@
@Test
public void testBadWhitelistTransition_wrongDefaultValue() throws Exception {
+ String whitelistAttributeName = WHITELIST_ATTRIBUTE_NAME.replace("$", "_");
scratch.file(
"test/rules.bzl",
"def _impl(ctx):",
" return []",
"",
"my_rule = rule(_impl, attrs = {'"
- + WHITELIST_ATTRIBUTE_NAME
+ + whitelistAttributeName
+ "':attr.label(default = Label('//test:my_other_rule'))})");
scratch.file(
"test/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 54ca283..fc9c05f 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -199,42 +199,16 @@
@Test
public void testAttrNameSpecialCharactersAreForbidden() throws Exception {
- ev =
- createEvaluationTestCase(
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleRestrictAttributeNames(true)
- .build());
- ev.initialize();
-
ev.setFailFast(false);
evalAndExport("def impl(ctx): return", "r = rule(impl, attrs = {'ab$c': attr.int()})");
- ev.assertContainsError("attribute name `ab$c` is not a valid identfier");
+ ev.assertContainsError("attribute name `ab$c` is not a valid identifier");
}
@Test
public void testAttrNameCannotStartWithDigit() throws Exception {
- ev =
- createEvaluationTestCase(
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleRestrictAttributeNames(true)
- .build());
- ev.initialize();
-
ev.setFailFast(false);
evalAndExport("def impl(ctx): return", "r = rule(impl, attrs = {'2_foo': attr.int()})");
- ev.assertContainsError("attribute name `2_foo` is not a valid identfier");
- }
-
- @Test
- public void testAttrNameSpecialCharactersLegacy() throws Exception {
- ev =
- createEvaluationTestCase(
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleRestrictAttributeNames(false)
- .build());
- ev.initialize();
-
- evalAndExport("def impl(ctx): return", "r = rule(impl, attrs = {'ab$c': attr.int()})");
+ ev.assertContainsError("attribute name `2_foo` is not a valid identifier");
}
@Test
