From 460ab684b4203cf87663038e67db352775adfdde Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Fri, 14 Aug 2020 13:59:36 -0700 Subject: [PATCH] Use definition location for StarlarkRuleFunction.export errors. StarlarkRuleFunction.getLocation() always returns BUILTIN. Closes #11915. PiperOrigin-RevId: 326724815 --- .../starlark/StarlarkRuleClassFunctions.java | 15 ++++++++------- .../starlark/StarlarkRuleClassFunctionsTest.java | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) 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 00101b8403decb..6fbea64c5cdce0 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 @@ -705,7 +705,7 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti if (attr.hasAnalysisTestTransition()) { if (!builder.isAnalysisTest()) { throw new EvalException( - getLocation(), + definitionLocation, "Only rule definitions with analysis_test=True may have attributes with " + "analysis_test_transition transitions"); } @@ -717,11 +717,12 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti || name.equals(FunctionSplitTransitionAllowlist.LEGACY_ATTRIBUTE_NAME)) { if (!BuildType.isLabelType(attr.getType())) { throw new EvalException( - getLocation(), "_allowlist_function_transition attribute must be a label type"); + definitionLocation, + "_allowlist_function_transition attribute must be a label type"); } if (attr.getDefaultValueUnchecked() == null) { throw new EvalException( - getLocation(), + definitionLocation, "_allowlist_function_transition attribute must have a default value"); } Label defaultLabel = (Label) attr.getDefaultValueUnchecked(); @@ -740,7 +741,7 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti .getName() .equals(FunctionSplitTransitionAllowlist.LEGACY_LABEL.getName()))) { throw new EvalException( - getLocation(), + definitionLocation, "_allowlist_function_transition attribute (" + defaultLabel + ") does not have the expected value " @@ -755,7 +756,7 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti if (hasStarlarkDefinedTransition) { if (!hasFunctionTransitionAllowlist) { throw new EvalException( - getLocation(), + definitionLocation, String.format( "Use of Starlark transition without allowlist attribute" + " '_allowlist_function_transition'. See Starlark transitions documentation" @@ -765,7 +766,7 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti } else { if (hasFunctionTransitionAllowlist) { throw new EvalException( - getLocation(), + definitionLocation, String.format( "Unused function-based split transition allowlist: %s %s", builder.getRuleDefinitionEnvironmentLabel(), builder.getType())); @@ -775,7 +776,7 @@ public void export(Label starlarkLabel, String ruleClassName) throws EvalExcepti try { this.ruleClass = builder.build(ruleClassName, starlarkLabel + "%" + ruleClassName); } catch (IllegalArgumentException | IllegalStateException ex) { - throw new EvalException(getLocation(), ex); + throw new EvalException(definitionLocation, ex); } this.builder = null; 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 af4d7293a678c9..9813cf18c020a4 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 @@ -265,7 +265,7 @@ public void testRuleClassTooLongAttributeName() throws Exception { Event event = ev.getEventCollector().iterator().next(); assertThat(event.getKind()).isEqualTo(EventKind.ERROR); assertThat(event.getMessage()) - .matches("Attribute r\\.x{150}'s name is too long \\(150 > 128\\)"); + .matches(":2:9: Attribute r\\.x{150}'s name is too long \\(150 > 128\\)"); } @Test