BuildFileAST.eval() doesn't execute the code when there's a static error.

This has no visible effect on Bazel (eval is used just for internal
expressions that don't fail), and is therefore not tested.

It will be tested when we add tests to the standalone Skylark interpreter.

--
PiperOrigin-RevId: 147495990
MOS_MIGRATED_REVID=147495990

5 files changed, 17 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index d0efaca097..6ad5a4897b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -339,11 +339,17 @@ public class BuildFileAST extends ASTNode {
     return last;
   }
 
+  /**
+   * Evaluates the lines from input and return the value of the last statement if it's an
+   * Expression or else null. In case of error (either during validation or evaluation), it
+   * throws an EvalException.
+   */
+  @Nullable
   public static Object eval(Environment env, String... input)
       throws EvalException, InterruptedException {
     BuildFileAST ast = parseSkylarkString(env.getEventHandler(), input);
     ValidationEnvironment valid = new ValidationEnvironment(env);
-    valid.validateAst(ast.getStatements(), env.getEventHandler());
+    valid.validateAst(ast.getStatements());
     return ast.eval(env);
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index a642f63879..6f1904422d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -298,8 +298,8 @@ public class EnvironmentTest extends EvaluationTestCase {
     try {
       BuildFileAST.eval(env, "special_var = 41");
       throw new AssertionError("failed to fail");
-    } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains("ERROR 1:1: Variable special_var is read only");
+    } catch (EvalException e) {
+      assertThat(e.getMessage()).contains("Variable special_var is read only");
     }
 
     try {
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 db5d95cfe1..4a71755ab2 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
@@ -477,8 +477,9 @@ public class EvaluationTest extends EvaluationTestCase {
 
   @Test
   public void testListComprehensionModifiesGlobalEnv() throws Exception {
-    new SkylarkTest().update("x", 42).testIfErrorContains("ERROR 1:1: Variable x is read only",
-        "[x + 1 for x in [1,2,3]]");
+    new SkylarkTest()
+        .update("x", 42)
+        .testIfErrorContains("Variable x is read only", "[x + 1 for x in [1,2,3]]");
     new BuildTest().update("x", 42).setUp("y =[x + 1 for x in [1,2,3]]")
         .testExactOrder("y", 2, 3, 4).testLookup("x", 3); // (x is global)
   }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index ed91471964..31810415f4 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -504,20 +504,14 @@ public class MethodLibraryTest extends EvaluationTestCase {
 
   @Test
   public void testPyStringJoin() throws Exception {
-    new BothModesTest().testStatement("'-'.join([ 'a', 'b', 'c' ])", "a-b-c");
+    new BothModesTest().testStatement("'-'.join(['a', 'b', 'c'])", "a-b-c");
   }
 
   @Test
   public void testPyStringGlobalJoin() throws Exception {
-    // TODO(bazel-team): BUILD and Skylark should use the same code path (and same error message).
-    new BuildTest()
-        .testIfErrorContains("name 'join' is not defined", "join(' ', [ 'a', 'b', 'c' ])");
-
-    new SkylarkTest()
-        .testIfErrorContains(
-            "ERROR 1:1: name 'join' is not defined", "join(' ', [ 'a', 'b', 'c' ])");
-
-    new BothModesTest().testStatement("' '.join([ 'a', 'b', 'c' ])", "a b c");
+    new BothModesTest()
+        .testIfErrorContains("name 'join' is not defined", "join(' ', ['a', 'b', 'c'])")
+        .testStatement("' '.join(['a', 'b', 'c'])", "a b c");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index f5ef419f25..c2d602453c 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -1230,9 +1230,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     // can't reuse the same local variable twice(!)
     new SkylarkTest()
         .testIfErrorContains(
-            "ERROR 2:1: Variable x is read only",
-            "[x + y for x, y in (1, 2)]",
-            "[x + y for x, y in (1, 2)]");
+            "Variable x is read only", "[x + y for x, y in (1, 2)]", "[x + y for x, y in (1, 2)]");
 
     new SkylarkTest()
         .testIfErrorContains("type 'int' is not a collection", "[x2 + y2 for x2, y2 in (1, 2)]");