Skylark parser: Improve error messages, never display "$error$".

--
MOS_MIGRATED_REVID=90075856

2 files changed, 21 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java b/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
index 19d4483a5f..62ad77b2c4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
@@ -66,6 +66,8 @@ public final class Ident extends Expression {
   SkylarkType validate(ValidationEnvironment env) throws EvalException {
     if (env.hasSymbolInEnvironment(name)) {
       return env.getVartype(name);
+    } else if (name.equals("$error$")) {
+      throw new EvalException(getLocation(), "contains syntax error(s)", true);
     } else {
       throw new EvalException(getLocation(), "name '" + name + "' is not defined");
     }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
index 22b4325c59..feb1c1bf9d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import com.google.common.base.Joiner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
+import com.google.devtools.build.lib.events.Event;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -682,6 +683,24 @@ public class ValidationTests extends AbstractParserTestCase {
   }
 
   @Test
+  public void testDollarErrorDoesNotLeak() throws Exception {
+    syntaxEvents.setFailFast(false);
+    String content = Joiner.on("\n").join(
+        "def GenerateMapNames():",
+        "  a = 2",
+        "  b = [3, 4]",
+        "  if a not b:",
+        "    print(a)");
+    parseFileForSkylark(content);
+    syntaxEvents.assertContainsEvent("syntax error at 'not': expected :");
+    // Parser uses "$error" symbol for error recovery.
+    // It should not be used in error messages.
+    for (Event event : syntaxEvents.collector()) {
+      assertThat(event.getMessage()).doesNotContain("$error$");
+    }
+  }
+
+  @Test
   public void testParentWithSkylarkModule() throws Exception {
     Class<?> emptyListClass = SkylarkList.EMPTY_LIST.getClass();
     Class<?> simpleListClass = SkylarkList.list(Arrays.<Integer>asList(1, 2, 3), SkylarkType.INT)