commit 327bd6639204c455169f779d532e7f22a1101dad
author Googler <noreply@google.com> Tue Jul 16 14:41:24 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Jul 16 14:42:22 2019 -0700
tree 83062bbe97a9a2361aec50287ee8b08375e0dcf1
parent f6408d60c724b971ce684ece89b4000b54f81ccd

bazel syntax: simplify implementation of sorted function

I failed to notice in the GitHub review
(github.com/bazelbuild/bazel/pull/8881)
that the function had redundant checks and parameters.

PiperOrigin-RevId: 258443310

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

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 557fc60..816d92f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -104,22 +104,6 @@
     }
   }
 
-  private static Object evalKeyFunc(
-      Object obj, Object key, Location loc, Environment env, FuncallExpression ast)
-      throws EvalException, InterruptedException {
-    checkValidKeyFunc(key, loc);
-    return ((StarlarkFunction) key)
-        .call(Collections.singletonList(obj), Collections.emptyMap(), ast, env);
-  }
-
-  private static void checkValidKeyFunc(Object key, Location loc) throws EvalException {
-    if (key instanceof StarlarkFunction) {
-      return;
-    }
-    throw new EvalException(
-        loc, Printer.format("%r object is not callable", EvalUtils.getDataTypeName(key)));
-  }
-
   @SkylarkCallable(
       name = "all",
       doc =
@@ -218,9 +202,8 @@
       } catch (EvalUtils.ComparisonException e) {
         throw new EvalException(loc, e);
       }
-    } else {
-      checkValidKeyFunc(key, loc);
-
+    } else if (key instanceof StarlarkFunction) {
+      final StarlarkFunction keyfn = (StarlarkFunction) key;
       final FuncallExpression ast = new FuncallExpression(Identifier.of(""), ImmutableList.of());
 
       class KeyComparator implements Comparator<Object> {
@@ -229,8 +212,7 @@
         @Override
         public int compare(Object x, Object y) {
           try {
-            return EvalUtils.SKYLARK_COMPARATOR.compare(
-                evalKeyFunc(x, key, loc, env, ast), evalKeyFunc(y, key, loc, env, ast));
+            return EvalUtils.SKYLARK_COMPARATOR.compare(callKeyFunc(x), callKeyFunc(y));
           } catch (InterruptedException | EvalException e) {
             if (this.e == null) {
               this.e = e;
@@ -238,6 +220,10 @@
             return 0;
           }
         }
+
+        Object callKeyFunc(Object x) throws EvalException, InterruptedException {
+          return keyfn.call(Collections.singletonList(x), ImmutableMap.of(), ast, env);
+        }
       }
 
       KeyComparator comp = new KeyComparator();
@@ -253,6 +239,9 @@
         }
         throw (EvalException) comp.e;
       }
+    } else {
+      throw new EvalException(
+          loc, Printer.format("%r object is not callable", EvalUtils.getDataTypeName(key)));
     }
 
     if (reverse) {