Refactor FuncallExpression to allow for complex function terms later.
RELNOTES: None.
PiperOrigin-RevId: 164981071
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
index f64781b..9d123ca 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
+import com.google.devtools.build.lib.util.Preconditions;
import java.util.Map;
/**
@@ -155,7 +156,7 @@
public Object call(
Object[] args, FuncallExpression ast, com.google.devtools.build.lib.syntax.Environment env)
throws EvalException, InterruptedException {
- String ruleClassName = ast.getFunction().getName();
+ String ruleClassName = Preconditions.checkNotNull(ast.getFunctionNameIfPossible());
try {
RuleClass ruleClass = builder.build(ruleClassName);
PackageContext context = PackageFactory.getContext(env, ast);
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 ab68d65..6bfa55a 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
@@ -186,24 +186,48 @@
}
}
- @Nullable private final Expression object;
-
- private final Identifier function;
+ private final Expression function;
private final List<Argument.Passed> arguments;
private final int numPositionalArgs;
- public FuncallExpression(@Nullable Expression object, Identifier function,
- List<Argument.Passed> arguments) {
- this.object = object;
- this.function = function;
- this.arguments = arguments;
+ public FuncallExpression(Expression function, List<Argument.Passed> arguments) {
+ this.function = Preconditions.checkNotNull(function);
+ this.arguments = Preconditions.checkNotNull(arguments);
this.numPositionalArgs = countPositionalArguments();
}
- public FuncallExpression(Identifier function, List<Argument.Passed> arguments) {
- this(null, function, arguments);
+ /** Returns the function that is called. */
+ public Expression getFunction() {
+ return this.function;
+ }
+
+ /**
+ * Returns the name of the called function if it's available in the AST.
+ *
+ * <p>It may not be available in cases like `list[0](arg1, arg2)`.
+ */
+ @Nullable
+ public String getFunctionNameIfPossible() {
+ Identifier ident = getFunctionIdentifierIfPossible();
+ return ident == null ? null : ident.getName();
+ }
+
+ /**
+ * Returns the identifier of the called function if it's available in the AST.
+ *
+ * <p>It may not be available in cases like `list[0](arg1, arg2)`.
+ */
+ @Nullable
+ public Identifier getFunctionIdentifierIfPossible() {
+ if (function instanceof Identifier) {
+ return (Identifier) function;
+ }
+ if (function instanceof DotExpression) {
+ return ((DotExpression) function).getField();
+ }
+ return null;
}
/**
@@ -220,22 +244,6 @@
}
/**
- * Returns the function expression.
- */
- public Identifier getFunction() {
- return function;
- }
-
- /**
- * Returns the object the function called on.
- * It's null if the function is not called on an object.
- */
- @Nullable
- public Expression getObject() {
- return object;
- }
-
- /**
* Returns an (immutable, ordered) list of function arguments. The first n are
* positional and the remaining ones are keyword args, where n =
* getNumPositionalArguments().
@@ -254,10 +262,6 @@
@Override
public void prettyPrint(Appendable buffer) throws IOException {
- if (object != null) {
- object.prettyPrint(buffer);
- buffer.append('.');
- }
function.prettyPrint(buffer);
buffer.append('(');
String sep = "";
@@ -272,9 +276,6 @@
@Override
public String toString() {
Printer.LengthLimitedPrinter printer = new Printer.LengthLimitedPrinter();
- if (object != null) {
- printer.append(object.toString()).append(".");
- }
printer.append(function.toString());
printer.printAbbreviatedList(arguments, "(", ", ", ")", null,
Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
@@ -381,7 +382,8 @@
getLocation(),
String.format(
"type '%s' has multiple matches for function %s",
- EvalUtils.getDataTypeNameFromClass(objClass), formatMethod(args, kwargs)));
+ EvalUtils.getDataTypeNameFromClass(objClass),
+ formatMethod(methodName, args, kwargs)));
}
}
}
@@ -396,14 +398,15 @@
errorMessage =
String.format(
"type '%s' has no method %s",
- EvalUtils.getDataTypeNameFromClass(objClass), formatMethod(args, kwargs));
+ EvalUtils.getDataTypeNameFromClass(objClass),
+ formatMethod(methodName, args, kwargs));
} else {
errorMessage =
String.format(
"%s, in method %s of '%s'",
argumentListConversionResult.getError(),
- formatMethod(args, kwargs),
+ formatMethod(methodName, args, kwargs),
EvalUtils.getDataTypeNameFromClass(objClass));
}
throw new EvalException(getLocation(), errorMessage);
@@ -513,9 +516,9 @@
return ArgumentListConversionResult.fromArgumentList(builder.build());
}
- private String formatMethod(List<Object> args, Map<String, Object> kwargs) {
+ private static String formatMethod(String name, List<Object> args, Map<String, Object> kwargs) {
StringBuilder sb = new StringBuilder();
- sb.append(function.getName()).append("(");
+ sb.append(name).append("(");
boolean first = true;
for (Object obj : args) {
if (!first) {
@@ -713,7 +716,7 @@
addKeywordArg(kwargs, arg.getName(), value, duplicates);
}
}
- checkDuplicates(duplicates, function.getName(), getLocation());
+ checkDuplicates(duplicates, function.toString(), getLocation());
}
@VisibleForTesting
@@ -724,14 +727,20 @@
@Override
Object doEval(Environment env) throws EvalException, InterruptedException {
- return (object != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env);
+ 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));
}
- /**
- * Invokes object.function() and returns the result.
- */
- private Object invokeObjectMethod(Environment env) throws EvalException, InterruptedException {
- Object objValue = object.eval(env);
+ /** Invokes object.function() and returns the result. */
+ private Object invokeObjectMethod(Environment env, DotExpression dot)
+ throws EvalException, InterruptedException {
+ Object objValue = dot.getObject().eval(env);
ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
posargs.add(objValue);
// We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
@@ -739,7 +748,7 @@
Map<String, Object> kwargs = new LinkedHashMap<>();
evalArguments(posargs, kwargs, env);
return invokeObjectMethod(
- function.getName(), posargs.build(), ImmutableMap.copyOf(kwargs), this, env);
+ dot.getField().getName(), posargs.build(), ImmutableMap.copyOf(kwargs), this, env);
}
/**
@@ -789,11 +798,7 @@
@Override
void validate(ValidationEnvironment env) throws EvalException {
- if (object != null) {
- object.validate(env);
- } else {
- function.validate(env);
- }
+ function.validate(env);
for (Argument.Passed arg : arguments) {
arg.getValue().validate(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 e3b14aa..8f5e301 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
@@ -447,7 +447,8 @@
if (function.getLocation() == null) {
function = setLocation(function, start, end);
}
- return setLocation(new FuncallExpression(receiver, function, args), start, end);
+ Expression fun = receiver == null ? function : new DotExpression(receiver, function);
+ return setLocation(new FuncallExpression(fun, args), start, end);
}
// arg ::= IDENTIFIER '=' nontupleexpr
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index 345b4bb..b23b048 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -56,9 +56,6 @@
}
public void visit(FuncallExpression node) {
- if (node.getObject() != null) {
- visit(node.getObject());
- }
visit(node.getFunction());
visitAll(node.getArguments());
}
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 dd30a41..becaab2 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
@@ -694,7 +694,7 @@
@Test
public void testDictKeysDuplicateKeyArgs() throws Exception {
- newTest().testIfExactError("duplicate keywords 'arg', 'k' in call to keys",
+ newTest().testIfExactError("duplicate keywords 'arg', 'k' in call to {\"a\": 1}.keys",
"{'a': 1}.keys(arg='abc', arg='def', k=1, k=2)");
}
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 fc5e586..c616b2a 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
@@ -176,7 +176,7 @@
public void testFuncallExpr() throws Exception {
FuncallExpression e = (FuncallExpression) parseExpression("foo(1, 2, bar=wiz)");
- Identifier ident = e.getFunction();
+ Identifier ident = (Identifier) e.getFunction();
assertThat(ident.getName()).isEqualTo("foo");
assertThat(e.getArguments()).hasSize(3);
@@ -199,8 +199,8 @@
FuncallExpression e =
(FuncallExpression) parseExpression("foo.foo(1, 2, bar=wiz)");
- Identifier ident = e.getFunction();
- assertThat(ident.getName()).isEqualTo("foo");
+ DotExpression dotExpression = (DotExpression) e.getFunction();
+ assertThat(dotExpression.getField().getName()).isEqualTo("foo");
assertThat(e.getArguments()).hasSize(3);
assertThat(e.getNumPositionalArguments()).isEqualTo(2);
@@ -222,8 +222,8 @@
FuncallExpression e =
(FuncallExpression) parseExpression("foo.replace().split(1)");
- Identifier ident = e.getFunction();
- assertThat(ident.getName()).isEqualTo("split");
+ DotExpression dotExpr = (DotExpression) e.getFunction();
+ assertThat(dotExpr.getField().getName()).isEqualTo("split");
assertThat(e.getArguments()).hasSize(1);
assertThat(e.getNumPositionalArguments()).isEqualTo(1);
@@ -244,8 +244,8 @@
public void testStringMethExpr() throws Exception {
FuncallExpression e = (FuncallExpression) parseExpression("'foo'.foo()");
- Identifier ident = e.getFunction();
- assertThat(ident.getName()).isEqualTo("foo");
+ DotExpression dotExpression = (DotExpression) e.getFunction();
+ assertThat(dotExpression.getField().getName()).isEqualTo("foo");
assertThat(e.getArguments()).isEmpty();
}
@@ -291,7 +291,8 @@
FuncallExpression e = (FuncallExpression) parseExpression(
"'FOO.CC'.lower()[1:].startswith('oo')");
- assertThat(e.getFunction().getName()).isEqualTo("startswith");
+ DotExpression dotExpression = (DotExpression) e.getFunction();
+ assertThat(dotExpression.getField().getName()).isEqualTo("startswith");
assertThat(e.getArguments()).hasSize(1);
s = (SliceExpression) parseExpression("'FOO.CC'[1:][:2]");
@@ -336,7 +337,7 @@
// Test that the actual parameters are: (1, $error$, 3):
- Identifier ident = e.getFunction();
+ Identifier ident = (Identifier) e.getFunction();
assertThat(ident.getName()).isEqualTo("f");
assertThat(e.getArguments()).hasSize(3);
