diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index b0ab97ac757f2f..ca2f9124f58962 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/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 kwarg pkgContext.getBuilder(), ruleClass, attributeValues, + thread + .getSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES), pkgContext.getEventHandler(), thread.getSemantics(), thread.getCallStack()); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java index c26fe22d4a5502..221c39c512c7b4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java +++ b/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); 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 4b9522da2c077a..8a87df886468e5 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 @@ -414,6 +414,9 @@ public NoneType call(StarlarkThread thread, Tuple args, Dict kwa context.pkgBuilder, ruleClass, new BuildLangTypedAttributeValuesMap(kwargs), + thread + .getSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES), context.eventHandler, thread.getSemantics(), thread.getCallStack()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index 5522a6e30028fd..4b1f878b58545b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -2053,6 +2053,7 @@ Rule createRule( Package.Builder pkgBuilder, Label ruleLabel, AttributeValues attributeValues, + boolean failOnUnknownAttributes, EventHandler eventHandler, Location location, List callstack) @@ -2060,7 +2061,7 @@ Rule createRule( 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 @@ 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 void populateRuleAttributeValues( Rule rule, Package.Builder pkgBuilder, AttributeValues attributeValues, + boolean failOnUnknownAttributes, EventHandler eventHandler) throws InterruptedException, CannotPrecomputeDefaultsException { @@ -2109,6 +2111,7 @@ private void populateRuleAttributeValues( rule, pkgBuilder.getLabelConverter(), attributeValues, + failOnUnknownAttributes, pkgBuilder.getListInterner(), eventHandler); populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler); @@ -2131,6 +2134,7 @@ private BitSet populateDefinedRuleAttributeValues( Rule rule, LabelConverter labelConverter, AttributeValues attributeValues, + boolean failOnUnknownAttributes, Interner> listInterner, EventHandler eventHandler) { BitSet definedAttrIndices = new BitSet(); @@ -2138,7 +2142,7 @@ private BitSet populateDefinedRuleAttributeValues( 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 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; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index 83d6540528f00e..1d3bb2e076f1b0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/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 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 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; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java index b8af1b9fa69c3f..32486a289e6d10 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java +++ b/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); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 4e3119916db530..cb43c7cf6a773a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/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 EXPERIMENTAL_BUILTINS_BZL_PATH = diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD index d89744dfd73bd8..00e004beaaa340 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/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", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java index da4a034cd4cf18..6bf2de28f7e0f0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java +++ b/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"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index b174f97486abe3..12657c66b7bf5f 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/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); diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java index ba68d615e0de53..be8f33f648723a 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java +++ b/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));