bazel syntax: improve error for dynamic use-before-definition
The error emitted for a dynamic use-before-definition in a BUILD file
used to look confusingly like a static error:
"name x is not defined; did you mean...?"
Such errors are expected in programs like 'print(x); x=1'
where there is no misspelling.
This change makes the error resemble the dynamic error in a .bzl file:
"variable x referenced before assignment", which makes clear that
there is an assignment for x, it just hasn't been executed yet.
PiperOrigin-RevId: 294428556
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index 6cb3133..e42fb64 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -607,10 +607,17 @@
if (result != null) {
return result;
}
- String error =
- ValidationEnvironment.createInvalidIdentifierException(
- name, fr.thread.getVariableNames());
- throw new EvalException(id.getStartLocation(), error);
+
+ // Assuming validation was successfully applied before execution
+ // (which is not yet true for copybara, but will be soon),
+ // then the identifier must have been resolved but the
+ // resolution was not annotated onto the syntax tree---because
+ // it's a BUILD file that may share trees with the prelude.
+ // So this error does not mean "undefined variable" (morally a
+ // static error), but "variable was (dynamically) referenced
+ // before being bound", as in 'print(x); x=1'.
+ throw new EvalException(
+ id.getStartLocation(), "variable '" + name + "' is referenced before assignment");
}
Object result;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 11abce1..167be97 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -231,8 +231,7 @@
}
}
- // This is exposed to Eval until validation becomes a precondition for evaluation.
- static String createInvalidIdentifierException(String name, Set<String> candidates) {
+ private static String createInvalidIdentifierException(String name, Set<String> candidates) {
if (name.equals("$error$")) {
return "contains syntax error(s)";
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 240d39a..e53b709 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -466,11 +466,13 @@
@Test
public void testListComprehensionDefinitionOrder() throws Exception {
new BuildTest()
- .testIfErrorContains("name 'y' is not defined", "[x for x in (1, 2) if y for y in (3, 4)]");
+ .testIfErrorContains(
+ "variable 'y' is referenced before assignment", //
+ "[x for x in (1, 2) if y for y in (3, 4)]");
new SkylarkTest()
.testIfErrorContains(
- "local variable 'y' is referenced before assignment",
+ "local variable 'y' is referenced before assignment", //
"[x for x in (1, 2) if y for y in (3, 4)]");
}
