Skylark: Functions don't need to be declared in order.

--
MOS_MIGRATED_REVID=93515487

src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index 256abb8..dce9e9c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -106,6 +106,5 @@
     for (Statement stmts : statements) {
       stmts.validate(localEnv);
     }
-    env.declare(ident.getName(), getLocation());
   }
 }

src/main/java/com/google/devtools/build/lib/syntax/Parser.java

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 5c363a6..6381c15 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -194,9 +194,7 @@
     List<Statement> statements = parser.parseFileInput();
     boolean hasSemanticalErrors = false;
     try {
-      for (Statement statement : statements) {
-        statement.validate(validationEnvironment);
-      }
+      validationEnvironment.validateAst(statements);
     } catch (EvalException e) {
       // Do not report errors caused by a previous parsing error, as it has already been reported.
       if (!e.isDueToIncompleteAST()) {

src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java

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 5d38ccf..a615d67 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
@@ -20,6 +20,7 @@
 
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
@@ -154,4 +155,23 @@
     readOnlyVariables.removeAll(futureReadOnlyVariables.peek());
     clonable = false;
   }
+
+  /**
+   * Validates the AST and runs static checks.
+   */
+  public void validateAst(List<Statement> statements) throws EvalException {
+    // Add every function in the environment before validating. This is
+    // necessary because functions may call other functions defined
+    // later in the file.
+    for (Statement statement : statements) {
+      if (statement instanceof FunctionDefStatement) {
+        FunctionDefStatement fct = (FunctionDefStatement) statement;
+        declare(fct.getIdent().getName(), fct.getLocation());
+      }
+    }
+
+    for (Statement statement : statements) {
+      statement.validate(this);
+    }
+  }
 }

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

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 579268f..43e5ac8 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
@@ -664,6 +664,22 @@
   }
 
   @Test
+  public void testFunctionCallOrdering() throws Exception {
+    eval("def func(): return foo() * 2",
+         "def foo(): return 2",
+         "x = func()");
+    assertThat(lookup("x")).isEqualTo(4);
+  }
+
+  @Test
+  public void testFunctionCallBadOrdering() throws Exception {
+    checkEvalError("name 'foo' is not defined",
+         "def func(): return foo() * 2",
+         "x = func()",
+         "def foo(): return 2");
+  }
+
+  @Test
   public void testNoneTrueFalseInSkylark() throws Exception {
     eval("a = None",
       "b = True",

src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java

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 edbf550..fbbdbcc 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
@@ -107,14 +107,14 @@
 
   @Test
   public void testFunctionDefRecursion() throws Exception {
-    checkError("function 'func' does not exist",
+    parse(
         "def func():",
         "  func()\n");
   }
 
   @Test
   public void testMutualRecursion() throws Exception {
-    checkError("function 'bar' does not exist",
+    parse(
         "def foo(i):",
         "  bar(i)",
         "def bar(i):",
@@ -123,13 +123,19 @@
   }
 
   @Test
-  public void testFunctionDoesNotExistInFunctionDef() {
-    checkError("function 'foo' does not exist",
+  public void testFunctionDefinedBelow() {
+    parse(
         "def bar(): a = foo() + 'a'",
         "def foo(): return 1\n");
   }
 
   @Test
+  public void testFunctionDoesNotExist() {
+    checkError("function 'foo' does not exist",
+        "def bar(): a = foo() + 'a'");
+  }
+
+  @Test
   public void testStructMembersAreImmutable() {
     checkError("can only assign to variables and tuples, not to 's.x'",
         "s = struct(x = 'a')",