Rollback of commit 5972bee6ebfa53cf165befab9fa7962e19d5f620.

*** Reason for rollback ***

Break Bazel bootstrap on JDK7

*** Original change description ***

Skylark: Give more detailed errors when parsing the arguments
of a SkylarkCallable.

--
MOS_MIGRATED_REVID=134309621

1 files changed, 21 insertions, 72 deletions

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 b347e5c9eb..4397ae74f9 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
@@ -197,32 +197,6 @@ public final class FuncallExpression extends Expression {
     return null;
   }
 
-  private static class ArgumentListConversionResult {
-    private final ImmutableList<Object> arguments;
-    private final String error;
-
-    private ArgumentListConversionResult(ImmutableList<Object> arguments, String error) {
-      this.arguments = arguments;
-      this.error = error;
-    }
-
-    public static ArgumentListConversionResult fromArgumentList(ImmutableList<Object> arguments) {
-      return new ArgumentListConversionResult(arguments, null);
-    }
-
-    public static ArgumentListConversionResult fromError(String error) {
-      return new ArgumentListConversionResult(null, error);
-    }
-
-    public String getError() {
-      return error;
-    }
-
-    public ImmutableList<Object> getArguments() {
-      return arguments;
-    }
-  }
-
   /**
    * An exception class to handle exceptions in direct Java API calls.
    */
@@ -399,20 +373,18 @@ public final class FuncallExpression extends Expression {
   // TODO(bazel-team): If there's exactly one usable method, this works. If there are multiple
   // matching methods, it still can be a problem. Figure out how the Java compiler does it
   // exactly and copy that behaviour.
-  // Throws an EvalException when it cannot find a matching function.
   private Pair<MethodDescriptor, List<Object>> findJavaMethod(
       Class<?> objClass, String methodName, List<Object> args, Map<String, Object> kwargs)
       throws EvalException {
     Pair<MethodDescriptor, List<Object>> matchingMethod = null;
     List<MethodDescriptor> methods = getMethods(objClass, methodName, getLocation());
-    ArgumentListConversionResult argumentListConversionResult = null;
     if (methods != null) {
       for (MethodDescriptor method : methods) {
         if (!method.getAnnotation().structField()) {
-          argumentListConversionResult = convertArgumentList(args, kwargs, method);
-          if (argumentListConversionResult.getArguments() != null) {
+          List<Object> resultArgs = convertArgumentList(args, kwargs, method);
+          if (resultArgs != null) {
             if (matchingMethod == null) {
-              matchingMethod = new Pair<>(method, argumentListConversionResult.getArguments());
+              matchingMethod = new Pair<>(method, resultArgs);
             } else {
               throw new EvalException(
                   getLocation(),
@@ -425,22 +397,11 @@ public final class FuncallExpression extends Expression {
       }
     }
     if (matchingMethod == null) {
-      String errorMessage;
-      if (argumentListConversionResult == null || argumentListConversionResult.getError() == null) {
-        errorMessage =
-            String.format(
-                "Type %s has no %s",
-                EvalUtils.getDataTypeNameFromClass(objClass), formatMethod(args, kwargs));
-
-      } else {
-        errorMessage =
-            String.format(
-                "%s (in %s of %s).",
-                argumentListConversionResult.getError(),
-                formatMethod(args, kwargs),
-                EvalUtils.getDataTypeNameFromClass(objClass));
-      }
-      throw new EvalException(getLocation(), errorMessage);
+      throw new EvalException(
+          getLocation(),
+          String.format(
+              "Type %s has no %s",
+              EvalUtils.getDataTypeNameFromClass(objClass), formatMethod(args, kwargs)));
     }
     return matchingMethod;
   }
@@ -455,10 +416,10 @@ public final class FuncallExpression extends Expression {
 
   /**
    * Constructs the parameters list to actually pass to the method, filling with default values if
-   * any. If the type does not match, returns null and fills in methodTypeMatchError with the
-   * appropriate message.
+   * any. If the type does not match, returns null.
    */
-  private ArgumentListConversionResult convertArgumentList(
+  @Nullable
+  private ImmutableList<Object> convertArgumentList(
       List<Object> args, Map<String, Object> kwargs, MethodDescriptor method) {
     ImmutableList.Builder<Object> builder = ImmutableList.builder();
     Class<?>[] params = method.getMethod().getParameterTypes();
@@ -473,17 +434,15 @@ public final class FuncallExpression extends Expression {
     }
     if (mandatoryPositionals > args.size()
         || args.size() > mandatoryPositionals + callable.parameters().length) {
-      return ArgumentListConversionResult.fromError("Too many arguments");
+      return null;
     }
-    // First process the legacy positional parameters.
+    // First process the legacy positional parameters
     int i = 0;
     if (mandatoryPositionals > 0) {
       for (Class<?> param : params) {
         Object value = args.get(i);
         if (!param.isAssignableFrom(value.getClass())) {
-          return ArgumentListConversionResult.fromError(
-              String.format(
-                  "Cannot convert parameter at position %d to type %s", i, param.toString()));
+          return null;
         }
         builder.add(value);
         i++;
@@ -493,7 +452,6 @@ public final class FuncallExpression extends Expression {
         }
       }
     }
-
     // Then the parameters specified in callable.parameters()
     Set<String> keys = new HashSet<>(kwargs.keySet());
     for (Param param : callable.parameters()) {
@@ -504,41 +462,32 @@ public final class FuncallExpression extends Expression {
       Object value = null;
       if (i < args.size()) {
         value = args.get(i);
-        if (!param.positional()) {
-          return ArgumentListConversionResult.fromError(
-              String.format("Parameter '%s' is not positional", param.name()));
-        } else if (!type.contains(value)) {
-          return ArgumentListConversionResult.fromError(
-              String.format(
-                  "Cannot convert parameter '%s' to type %s", param.name(), type.toString()));
+        if (!param.positional() || !type.contains(value)) {
+          return null; // next positional argument is not of the good type
         }
         i++;
       } else if (param.named() && keys.remove(param.name())) {
         // Named parameters
         value = kwargs.get(param.name());
         if (!type.contains(value)) {
-          return ArgumentListConversionResult.fromError(
-              String.format(
-                  "Cannot convert parameter '%s' to type %s", param.name(), type.toString()));
+          return null; // type does not match
         }
       } else {
         // Use default value
         if (param.defaultValue().isEmpty()) {
-          return ArgumentListConversionResult.fromError(
-              String.format("Parameter '%s' has no default value", param.name()));
+          return null; // no default value
         }
         value = SkylarkSignatureProcessor.getDefaultValue(param, null);
       }
       builder.add(value);
       if (!param.noneable() && value instanceof NoneType) {
-        return ArgumentListConversionResult.fromError(
-            String.format("Parameter '%s' cannot be None", param.name()));
+        return null; // cannot be None
       }
     }
     if (i < args.size() || !keys.isEmpty()) {
-      return ArgumentListConversionResult.fromError("Too many arguments");
+      return null; // too many arguments
     }
-    return ArgumentListConversionResult.fromArgumentList(builder.build());
+    return builder.build();
   }
 
   private String formatMethod(List<Object> args, Map<String, Object> kwargs) {