[6.4.0] Always fail on unknown attributes (#19404)

Fixes: #11000

RELNOTES[INC]: Fails on unknown attributes (even when set to None)
PiperOrigin-RevId: 562519157
Change-Id: If5e430c73485c8ae9661f4231692384a057f37d5

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -91,6 +91,7 @@
 import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.lib.starlarkbuildapi.StarlarkRuleFunctionsApi;
 import com.google.devtools.build.lib.util.FileType;
@@ -799,6 +800,9 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
  pkgContext.getBuilder(),
  ruleClass,
  attributeValues,
+ thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES),
  pkgContext.getEventHandler(),
  thread.getSemantics(),
  thread.getCallStack());

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java

@@ -76,7 +76,7 @@ public static Rule createRule(
  try {
  rule =
  RuleFactory.createAndAddRule(
- packageBuilder, ruleClass, attributeValues, eventHandler, semantics, callStack);
+ packageBuilder, ruleClass, attributeValues, true, eventHandler, semantics, callStack);
  } catch (NameConflictException e) {
  // This literally cannot happen -- we just created the package!
  throw new IllegalStateException(e);

src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -414,6 +414,9 @@ public NoneType call(StarlarkThread thread, Tuple args, Dict<String, Object> kwa
  context.pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(kwargs),
+ thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES),
  context.eventHandler,
  thread.getSemantics(),
  thread.getCallStack());

src/main/java/com/google/devtools/build/lib/packages/RuleClass.java

@@ -2053,14 +2053,15 @@ <T> Rule createRule(
  Package.Builder pkgBuilder,
  Label ruleLabel,
  AttributeValues<T> attributeValues,
+ boolean failOnUnknownAttributes,
  EventHandler eventHandler,
  Location location,
  List<StarlarkThread.CallStackEntry> callstack)
  throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException {
  Rule rule =
  pkgBuilder.createRule(
  ruleLabel, this, location, callstack, AttributeContainer.newMutableInstance(this));
- populateRuleAttributeValues(rule, pkgBuilder, attributeValues, eventHandler);
+ populateRuleAttributeValues(rule, pkgBuilder, attributeValues, failOnUnknownAttributes, eventHandler);
  checkAspectAllowedValues(rule, eventHandler);
  rule.populateOutputFiles(eventHandler, pkgBuilder);
  checkForDuplicateLabels(rule, eventHandler);
@@ -2085,7 +2086,7 @@ <T> Rule createRuleUnchecked(
  throws InterruptedException, CannotPrecomputeDefaultsException {
  Rule rule =
  pkgBuilder.createRule(ruleLabel, this, location, callstack, implicitOutputsFunction);
- populateRuleAttributeValues(rule, pkgBuilder, attributeValues, NullEventHandler.INSTANCE);
+ populateRuleAttributeValues(rule, pkgBuilder, attributeValues, true, NullEventHandler.INSTANCE);
  rule.populateOutputFilesUnchecked(pkgBuilder);
  return rule;
  }
@@ -2101,6 +2102,7 @@ private <T> void populateRuleAttributeValues(
  Rule rule,
  Package.Builder pkgBuilder,
  AttributeValues<T> attributeValues,
+ boolean failOnUnknownAttributes,
  EventHandler eventHandler)
  throws InterruptedException, CannotPrecomputeDefaultsException {
 
@@ -2109,6 +2111,7 @@ private <T> void populateRuleAttributeValues(
  rule,
  pkgBuilder.getLabelConverter(),
  attributeValues,
+ failOnUnknownAttributes,
  pkgBuilder.getListInterner(),
  eventHandler);
  populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler);
@@ -2131,14 +2134,15 @@ private <T> BitSet populateDefinedRuleAttributeValues(
  Rule rule,
  LabelConverter labelConverter,
  AttributeValues<T> attributeValues,
+ boolean failOnUnknownAttributes,
  Interner<ImmutableList<?>> listInterner,
  EventHandler eventHandler) {
  BitSet definedAttrIndices = new BitSet();
  for (T attributeAccessor : attributeValues.getAttributeAccessors()) {
  String attributeName = attributeValues.getName(attributeAccessor);
  Object attributeValue = attributeValues.getValue(attributeAccessor);
  // Ignore all None values.
- if (attributeValue == Starlark.NONE) {
+ if (attributeValue == Starlark.NONE && !failOnUnknownAttributes) {
  continue;
  }
 
@@ -2160,10 +2164,15 @@ private <T> BitSet populateDefinedRuleAttributeValues(
  eventHandler);
  continue;
  }
+ // Ignore all None values (after reporting an error)
+ if (attributeValue == Starlark.NONE) {
+ continue;
+ }
+
  Attribute attr = getAttribute(attrIndex);
 
  if (attributeName.equals("licenses") && ignoreLicenses) {
- rule.setAttributeValue(attr, License.NO_LICENSE, /*explicit=*/ false);
+ rule.setAttributeValue(attr, License.NO_LICENSE, /* explicit= */ false);
  definedAttrIndices.set(attrIndex);
  continue;
  }

src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java

@@ -69,6 +69,7 @@ public static Rule createRule(
  Package.Builder pkgBuilder,
  RuleClass ruleClass,
  BuildLangTypedAttributeValuesMap attributeValues,
+ boolean failOnUnknownAttributes,
  EventHandler eventHandler,
  StarlarkSemantics semantics,
  ImmutableList<StarlarkThread.CallStackEntry> callstack)
@@ -111,6 +112,7 @@ public static Rule createRule(
  pkgBuilder,
  label,
  generator.attributes,
+ failOnUnknownAttributes,
  eventHandler,
  generator.location, // see b/23974287 for rationale
  callstack);
@@ -143,12 +145,13 @@ public static Rule createAndAddRule(
  Package.Builder pkgBuilder,
  RuleClass ruleClass,
  BuildLangTypedAttributeValuesMap attributeValues,
+ boolean failOnUnknownAttributes,
  EventHandler eventHandler,
  StarlarkSemantics semantics,
  ImmutableList<StarlarkThread.CallStackEntry> callstack)
  throws InvalidRuleException, NameConflictException, InterruptedException {
  Rule rule =
- createRule(pkgBuilder, ruleClass, attributeValues, eventHandler, semantics, callstack);
+ createRule(pkgBuilder, ruleClass, attributeValues, failOnUnknownAttributes, eventHandler, semantics, callstack);
  pkgBuilder.addRule(rule);
  return rule;
  }

src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java

@@ -48,7 +48,7 @@ public static Rule createAndAddRepositoryRule(
  StoredEventHandler eventHandler = new StoredEventHandler();
  BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
  Rule rule =
- RuleFactory.createRule(pkg, ruleClass, attributeValues, eventHandler, semantics, callstack);
+ RuleFactory.createRule(pkg, ruleClass, attributeValues, true, eventHandler, semantics, callstack);
  pkg.addEvents(eventHandler.getEvents());
  pkg.addPosts(eventHandler.getPosts());
  overwriteRule(pkg, rule);
@@ -166,7 +166,7 @@ static void addBindRule(
  BuildLangTypedAttributeValuesMap attributeValues =
  new BuildLangTypedAttributeValuesMap(attributes);
  Rule rule =
- RuleFactory.createRule(pkg, bindRuleClass, attributeValues, handler, semantics, callstack);
+ RuleFactory.createRule(pkg, bindRuleClass, attributeValues, true, handler, semantics, callstack);
  overwriteRule(pkg, rule);
  rule.setVisibility(ConstantRuleVisibility.PUBLIC);
  }

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -652,6 +652,16 @@ public final class BuildLanguageOptions extends OptionsBase {
  + " specified through features configuration.")
  public boolean experimentalGetFixedConfiguredEnvironment;
 
+ // cleanup, flip, remove after Bazel LTS in Nov 2023
+ @Option(
+ name = "incompatible_fail_on_unknown_attributes",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ help = "If enabled, targets that have unknown attributes set to None fail.")
+ public boolean incompatibleFailOnUnknownAttributes;
+
  /**
  * An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
  * never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -743,6 +753,7 @@ public StarlarkSemantics toStarlarkSemantics() {
  .setBool(
  EXPERIMENTAL_GET_FIXED_CONFIGURED_ACTION_ENV,
  experimentalGetFixedConfiguredEnvironment)
+ .setBool(INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES, incompatibleFailOnUnknownAttributes)
  .build();
  return INTERNER.intern(semantics);
  }
@@ -831,6 +842,8 @@ public StarlarkSemantics toStarlarkSemantics() {
  "-incompatible_disable_starlark_host_transitions";
  public static final String EXPERIMENTAL_GET_FIXED_CONFIGURED_ACTION_ENV =
  "-experimental_get_fixed_configured_action_env";
+ public static final String INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES =
+ "-incompatible_fail_on_unknown_attributes";
 
  // non-booleans
  public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =

src/test/java/com/google/devtools/build/lib/analysis/BUILD

@@ -9,13 +9,13 @@ filegroup(
  name = "srcs",
  testonly = 0,
  srcs = glob(["**"]) + [
+ "//src/test/java/com/google/devtools/build/lib/analysis/allowlisting:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/extra:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/mock:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/platform:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/starlark/annotations/processor:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/testing:srcs",
  "//src/test/java/com/google/devtools/build/lib/analysis/util:srcs",
- "//src/test/java/com/google/devtools/build/lib/analysis/allowlisting:srcs",
  ],
  visibility = ["//src:__subpackages__"],
 )
@@ -348,6 +348,7 @@ java_test(
  "//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
  "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
  "//src/main/java/com/google/devtools/build/lib/analysis:required_config_fragments_provider",
+ "//src/main/java/com/google/devtools/build/lib/cmdline",
  "//src/main/java/com/google/devtools/build/lib/events",
  "//src/test/java/com/google/devtools/build/lib/analysis/util",
  "//third_party:guava",

src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java

@@ -17,7 +17,9 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -435,4 +437,62 @@ public void testRulesDontProvideRequiredFragmentsByDefault() throws Exception {
  assertThat(getConfiguredTarget("//a:config").getProvider(RequiredConfigFragmentsProvider.class))
  .isNull();
  }
+
+ @Test
+ public void testNativeRuleAttrSetToNoneFails() throws Exception {
+ setBuildLanguageOptions("--incompatible_fail_on_unknown_attributes");
+ scratch.file(
+ "p/BUILD", //
+ "genrule(name = 'genrule', srcs = ['a.java'], outs = ['b'], cmd = '', bat = None)");
+
+ reporter.removeHandler(failFastHandler);
+ getTarget("//p:genrule");
+
+ assertContainsEvent("no such attribute 'bat' in 'genrule' rule");
+ }
+
+ @Test
+ public void testNativeRuleAttrSetToNoneDoesntFails() throws Exception {
+ setBuildLanguageOptions("--noincompatible_fail_on_unknown_attributes");
+ scratch.file(
+ "p/BUILD", //
+ "genrule(name = 'genrule', srcs = ['a.java'], outs = ['b'], cmd = '', bat = None)");
+
+ getTarget("//p:genrule");
+ }
+
+ @Test
+ public void testStarlarkRuleAttrSetToNoneFails() throws Exception {
+ setBuildLanguageOptions("--incompatible_fail_on_unknown_attributes");
+ scratch.file(
+ "p/rule.bzl", //
+ "def _impl(ctx):",
+ " pass",
+ "my_rule = rule(_impl)");
+ scratch.file(
+ "p/BUILD", //
+ "load(':rule.bzl', 'my_rule')",
+ "my_rule(name = 'my_target', bat = None)");
+
+ reporter.removeHandler(failFastHandler);
+ getTarget("//p:my_target");
+
+ assertContainsEvent("no such attribute 'bat' in 'my_rule' rule");
+ }
+
+ @Test
+ public void testStarlarkRuleAttrSetToNoneDoesntFail() throws Exception {
+ setBuildLanguageOptions("--noincompatible_fail_on_unknown_attributes");
+ scratch.file(
+ "p/rule.bzl", //
+ "def _impl(ctx):",
+ " pass",
+ "my_rule = rule(_impl)");
+ scratch.file(
+ "p/BUILD", //
+ "load(':rule.bzl', 'my_rule')",
+ "my_rule(name = 'my_target', bat = None)");
+
+ getTarget("//p:my_target");
+ }
 }

src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java

@@ -958,6 +958,7 @@ private Rule createRule(
  pkgBuilder,
  ruleLabel,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  reporter,
  location,
  callstack);

src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java

@@ -84,6 +84,7 @@ public void testCreateRule() throws Exception {
  pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  new Reporter(new EventBus()),
  StarlarkSemantics.DEFAULT,
  DUMMY_STACK);
@@ -144,6 +145,7 @@ public void testCreateWorkspaceRule() throws Exception {
  pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  new Reporter(new EventBus()),
  StarlarkSemantics.DEFAULT,
  DUMMY_STACK);
@@ -168,6 +170,7 @@ public void testWorkspaceRuleFailsInBuildFile() throws Exception {
  pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  new Reporter(new EventBus()),
  StarlarkSemantics.DEFAULT,
  DUMMY_STACK));
@@ -192,6 +195,7 @@ public void testBuildRuleFailsInWorkspaceFile() throws Exception {
  pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  new Reporter(new EventBus()),
  StarlarkSemantics.DEFAULT,
  DUMMY_STACK));
@@ -228,6 +232,7 @@ public void testOutputFileNotEqualDot() throws Exception {
  pkgBuilder,
  ruleClass,
  new BuildLangTypedAttributeValuesMap(attributeValues),
+ true,
  new Reporter(new EventBus()),
  StarlarkSemantics.DEFAULT,
  DUMMY_STACK));