Skylark: Functions don't need to be declared in order.
--
MOS_MIGRATED_REVID=93515487
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());
}
}
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()) {
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);
+ }
+ }
}
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",
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')",
