Fix Skylark parsing of call expressions. This allows more complex expressions to be called, not just identifiers. For example, "x[0]()" is not a syntax error anymore. RELNOTES: None PiperOrigin-RevId: 165157981
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java index 6bfa55a..0eb87da 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -727,14 +727,13 @@ @Override Object doEval(Environment env) throws EvalException, InterruptedException { + // TODO: Remove this special case once method resolution and invocation are supported as + // separate steps. if (function instanceof DotExpression) { return invokeObjectMethod(env, (DotExpression) function); } - if (function instanceof Identifier) { - return invokeGlobalFunction(env); - } - throw new EvalException( - getLocation(), Printer.format("cannot evaluate function '%s'", function)); + Object funcValue = function.eval(env); + return callFunction(funcValue, env); } /** Invokes object.function() and returns the result. */ @@ -752,14 +751,6 @@ } /** - * Invokes function() and returns the result. - */ - private Object invokeGlobalFunction(Environment env) throws EvalException, InterruptedException { - Object funcValue = function.eval(env); - return callFunction(funcValue, env); - } - - /** * Calls a function object */ private Object callFunction(Object funcValue, Environment env)
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 8f5e301..03c50d3 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
@@ -440,17 +440,6 @@ return setLocation(node, startOffset, lastNode.getLocation().getEndOffset()); } - // create a funcall expression - private Expression makeFuncallExpression(Expression receiver, Identifier function, - List<Argument.Passed> args, - int start, int end) { - if (function.getLocation() == null) { - function = setLocation(function, start, end); - } - Expression fun = receiver == null ? function : new DotExpression(receiver, function); - return setLocation(new FuncallExpression(fun, args), start, end); - } - // arg ::= IDENTIFIER '=' nontupleexpr // | expr // | *args (only in Skylark mode) @@ -534,7 +523,7 @@ } // funcall_suffix ::= '(' arg_list? ')' - private Expression parseFuncallSuffix(int start, Expression receiver, Identifier function) { + private Expression parseFuncallSuffix(int start, Expression function) { List<Argument.Passed> args = Collections.emptyList(); expect(TokenKind.LPAREN); int end; @@ -546,20 +535,15 @@ end = token.right; expect(TokenKind.RPAREN); } - return makeFuncallExpression(receiver, function, args, start, end); + return setLocation(new FuncallExpression(function, args), start, end); } // selector_suffix ::= '.' IDENTIFIER - // |'.' IDENTIFIER funcall_suffix private Expression parseSelectorSuffix(int start, Expression receiver) { expect(TokenKind.DOT); if (token.kind == TokenKind.IDENTIFIER) { Identifier ident = parseIdent(); - if (token.kind == TokenKind.LPAREN) { - return parseFuncallSuffix(start, receiver, ident); - } else { - return setLocation(new DotExpression(receiver, ident), start, token.right); - } + return setLocation(new DotExpression(receiver, ident), start, token.right); } else { syntaxError(token, "expected identifier after dot"); int end = syncTo(EXPR_TERMINATOR_SET); @@ -643,10 +627,7 @@ // primary ::= INTEGER // | STRING - // | STRING '.' IDENTIFIER funcall_suffix // | IDENTIFIER - // | IDENTIFIER funcall_suffix - // | IDENTIFIER '.' selector_suffix // | list_expression // | '(' ')' // a tuple with zero elements // | '(' expr ')' // a parenthesized expression @@ -665,14 +646,7 @@ case STRING: return parseStringLiteral(); case IDENTIFIER: - { - Identifier ident = parseIdent(); - if (token.kind == TokenKind.LPAREN) { // it's a function application - return parseFuncallSuffix(start, null, ident); - } else { - return ident; - } - } + return parseIdent(); case LBRACKET: // it's a list return parseListMaker(); case LBRACE: // it's a dictionary @@ -714,8 +688,7 @@ } } - // primary_with_suffix ::= primary selector_suffix* - // | primary substring_suffix + // primary_with_suffix ::= primary (selector_suffix | substring_suffix | funcall_suffix)* private Expression parsePrimaryWithSuffix() { int start = token.left; Expression receiver = parsePrimary(); @@ -724,6 +697,8 @@ receiver = parseSelectorSuffix(start, receiver); } else if (token.kind == TokenKind.LBRACKET) { receiver = parseSubstringSuffix(start, receiver); + } else if (token.kind == TokenKind.LPAREN) { + receiver = parseFuncallSuffix(start, receiver); } else { break; } @@ -732,6 +707,8 @@ } // substring_suffix ::= '[' expression? ':' expression? ':' expression? ']' + // | '[' expression? ':' expression? ']' + // | '[' expression ']' private Expression parseSubstringSuffix(int start, Expression receiver) { Expression startExpr;
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 becaab2..fb438a6 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
@@ -142,6 +142,12 @@ } @Test + public void testComplexFunctionCall() throws Exception { + newTest().setUp("functions = [min, max]", "l = [1,2]") + .testEval("(functions[0](l), functions[1](l))", "(1, 2)"); + } + + @Test public void testKeywordArgs() throws Exception { // This function returns the map of keyword arguments passed to it.
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java index c616b2a..a9a9e4c 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -174,10 +174,13 @@ @Test public void testFuncallExpr() throws Exception { - FuncallExpression e = (FuncallExpression) parseExpression("foo(1, 2, bar=wiz)"); + FuncallExpression e = (FuncallExpression) parseExpression("foo[0](1, 2, bar=wiz)"); - Identifier ident = (Identifier) e.getFunction(); - assertThat(ident.getName()).isEqualTo("foo"); + IndexExpression function = (IndexExpression) e.getFunction(); + Identifier functionList = (Identifier) function.getObject(); + assertThat(functionList.getName()).isEqualTo("foo"); + IntegerLiteral listIndex = (IntegerLiteral) function.getKey(); + assertThat(listIndex.getValue()).isEqualTo(0); assertThat(e.getArguments()).hasSize(3); assertThat(e.getNumPositionalArguments()).isEqualTo(2);