Check if the parent rule is extendable
PiperOrigin-RevId: 574401779
Change-Id: I229188da52fd4474abc52403e8a94c569f2cb6d1
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 be9d98b..db57dcf 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
@@ -361,7 +361,12 @@
executable = parent.isExecutableStarlark();
test = parent.getRuleClassType() == RuleClassType.TEST;
- // TODO b/300201845 - verify that parent is extendable
+ failIf(
+ !parent.isExtendable(),
+ "The rule '%s' is not extendable. Only Starlark rules not using deprecated features (like"
+ + " implicit outputs, output to genfiles) may be extended. Special rules like"
+ + " analysis tests or rules using build_settings cannot be extended.",
+ parent.getName());
failIf(
executableUnchecked != Starlark.UNBOUND,
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 88dd1a2..72c5ad1 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
@@ -823,6 +823,7 @@
// the condition removes {@link StarlarkRuleClasssFunctions.baseRule} and binaryBaseRule,
// which are marked as Starlark (because of Stardoc) && abstract at the same time
starlarkParent = parents[0];
+ Preconditions.checkArgument(starlarkParent.isExtendable());
}
for (RuleClass parent : parents) {
if (parent.getValidityPredicate() != PredicatesWithMessage.<Rule>alwaysTrue()) {
@@ -928,6 +929,15 @@
this.useToolchainResolution(ToolchainResolutionMode.DISABLED);
}
+ boolean extendable =
+ starlark
+ && (type == RuleClassType.NORMAL || type == RuleClassType.TEST)
+ && implicitOutputsFunction == ImplicitOutputsFunction.NONE
+ && outputsToBindir
+ && !starlarkTestable
+ && !isAnalysisTest
+ && buildSetting == null;
+
return new RuleClass(
name,
callstack,
@@ -935,6 +945,7 @@
type,
starlarkParent,
starlark,
+ extendable,
starlarkTestable,
documented,
outputsToBindir,
@@ -1625,6 +1636,7 @@
private final RuleClassType type;
@Nullable private final RuleClass starlarkParent;
private final boolean isStarlark;
+ private final boolean extendable;
private final boolean starlarkTestable;
private final boolean documented;
private final boolean outputsToBindir;
@@ -1759,6 +1771,7 @@
RuleClassType type,
RuleClass starlarkParent,
boolean isStarlark,
+ boolean extendable,
boolean starlarkTestable,
boolean documented,
boolean outputsToBindir,
@@ -1795,6 +1808,7 @@
this.type = type;
this.starlarkParent = starlarkParent;
this.isStarlark = isStarlark;
+ this.extendable = extendable;
this.targetKind = name + Rule.targetKindSuffix();
this.starlarkTestable = starlarkTestable;
this.documented = documented;
@@ -2630,6 +2644,11 @@
return isStarlark;
}
+ /** Returns true if this RuleClass can be extended. */
+ public boolean isExtendable() {
+ return extendable;
+ }
+
/** Returns true if this RuleClass is Starlark-defined and is subject to analysis-time tests. */
public boolean isStarlarkTestable() {
return starlarkTestable;
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 df0a491..c4128ff 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
@@ -1026,6 +1026,7 @@
RuleClassType.NORMAL,
/* starlarkParent= */ null,
/* isStarlark= */ starlarkExecutable,
+ /* extendable= */ false,
/* starlarkTestable= */ false,
documented,
binaryOutput,
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
index 13f27d5..12c85ee 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
@@ -3713,7 +3713,61 @@
.containsExactly("java", "cc");
}
- // TODO: b/300201845 - verify build_setting, analysis_test, ... can't be extended
+ private String notExtendableError(String rule) {
+ return String.format(
+ "The rule '%s' is not extendable. Only Starlark rules not using deprecated features (like"
+ + " implicit outputs, output to genfiles) may be extended. Special rules like"
+ + " analysis tests or rules using build_settings cannot be extended.",
+ rule);
+ }
+
+ @Test
+ public void extendRule_notExtendable() throws Exception {
+ BazelEvaluationTestCase ev = new BazelEvaluationTestCase("//extend_rule_testing:child.bzl");
+ ev.exec(
+ "def impl():", //
+ " pass");
+
+ ev.execAndExport("parent_library = rule(impl, output_to_genfiles = True)");
+ ev.checkEvalError(notExtendableError("parent_library"), "rule(impl, parent = parent_library)");
+
+ ev.execAndExport("parent_library = rule(impl, _skylark_testable = True)");
+ ev.checkEvalError(notExtendableError("parent_library"), "rule(impl, parent = parent_library)");
+
+ ev.execAndExport("parent_test = rule(impl, analysis_test = True)");
+ ev.checkEvalError(notExtendableError("parent_test"), "rule(impl, parent = parent_test)");
+
+ ev.update("config", new StarlarkConfig());
+ ev.execAndExport("parent_library = rule(impl, build_setting = config.int())");
+ ev.checkEvalError(notExtendableError("parent_library"), "rule(impl, parent = parent_library)");
+
+ ev.execAndExport("parent_library = rule(impl, outputs = {'deploy': '%{name}_deploy.jar'})");
+ ev.checkEvalError(notExtendableError("parent_library"), "rule(impl, parent = parent_library)");
+ }
+
+ @Test
+ public void extendRule_nativeRule_notExtendable() throws Exception {
+ scratch.file(
+ "extend_rule_testing/child.bzl",
+ "def _impl(ctx):",
+ " ctx.super()",
+ "my_library = rule(",
+ " implementation = _impl,",
+ " parent = native.alias,",
+ " fragments = ['cc']",
+ ")");
+ scratch.file(
+ "extend_rule_testing/BUILD",
+ "load(':child.bzl', 'my_library')",
+ "my_library(name = 'my_target')");
+
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(ev.getEventCollector());
+ getConfiguredTarget("//extend_rule_testing:my_target");
+
+ ev.assertContainsError("Parent needs to be a Starlark rule");
+ }
+
@Test
public void extendRule_toolchains_merged() throws Exception {
scratchParentRule(
