Eliminate SkylarkType.toMap() in favor of ImmutableMap.copyOf() and simplify

SkylarkType.castMap() a little.

--
MOS_MIGRATED_REVID=90988088

4 files changed, 31 insertions, 47 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
index bd54ca0bc6..1777a7e4ca 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
@@ -98,7 +98,7 @@ public abstract class ImplicitOutputsFunction {
       try {
         ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
         for (Map.Entry<String, String> entry : castMap(callback.call(attrs),
-            String.class, String.class, "implicit outputs function return value")) {
+            String.class, String.class, "implicit outputs function return value").entrySet()) {
           Iterable<String> substitutions = fromTemplates(entry.getValue()).getImplicitOutputs(map);
           if (!Iterables.isEmpty(substitutions)) {
             builder.put(entry.getKey(), Iterables.getOnlyElement(substitutions));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 803d3dad67..071185ccba 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -24,13 +24,13 @@ import static com.google.devtools.build.lib.packages.Type.LABEL_LIST;
 import static com.google.devtools.build.lib.packages.Type.STRING;
 import static com.google.devtools.build.lib.syntax.SkylarkType.castList;
 import static com.google.devtools.build.lib.syntax.SkylarkType.castMap;
-import static com.google.devtools.build.lib.syntax.SkylarkType.toMap;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
@@ -227,8 +227,8 @@ public class SkylarkRuleClassFunctions {
               ? new RuleClass.Builder("", type, true, testBaseRule)
               : new RuleClass.Builder("", type, true, baseRule);
 
-          for (Map.Entry<String, Attribute.Builder> attr : castMap(
-              arguments.get("attrs"), String.class, Attribute.Builder.class, "attrs")) {
+          for (Map.Entry<String, Attribute.Builder> attr : castMap(arguments.get("attrs"),
+              String.class, Attribute.Builder.class, "attrs").entrySet()) {
             Attribute.Builder<?> attrBuilder = attr.getValue();
             String attrName = attributeToNative(attr.getKey(), loc,
                 attrBuilder.hasLateBoundValue());
@@ -252,8 +252,8 @@ public class SkylarkRuleClassFunctions {
                   new SkylarkImplicitOutputsFunctionWithCallback(callback, loc));
             } else {
               builder.setImplicitOutputsFunction(new SkylarkImplicitOutputsFunctionWithMap(
-                  toMap(castMap(arguments.get("outputs"), String.class, String.class,
-                  "implicit outputs of the rule class"))));
+                  ImmutableMap.copyOf(castMap(arguments.get("outputs"), String.class, String.class,
+                      "implicit outputs of the rule class"))));
             }
           }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
index 9c5187fdf0..cc582ab17e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules;
 
 import static com.google.devtools.build.lib.syntax.SkylarkType.castList;
 import static com.google.devtools.build.lib.syntax.SkylarkType.castMap;
-import static com.google.devtools.build.lib.syntax.SkylarkType.toMap;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -148,8 +147,8 @@ public class SkylarkRuleImplementationFunctions {
         builder.setMnemonic((String) params.get("mnemonic"));
       }
       if (params.containsKey("env")) {
-        builder.setEnvironment(
-            toMap(castMap(params.get("env"), String.class, String.class, "env")));
+        builder.setEnvironment(ImmutableMap.copyOf(
+            castMap(params.get("env"), String.class, String.class, "env")));
       }
       if (params.containsKey("progress_message")) {
         builder.setProgressMessage((String) params.get("progress_message"));
@@ -159,12 +158,13 @@ public class SkylarkRuleImplementationFunctions {
         builder.useDefaultShellEnvironment();
       }
       if (params.containsKey("execution_requirements")) {
-        builder.setExecutionInfo(toMap(castMap(params.get("execution_requirements"),
-                    String.class, String.class, "execution_requirements")));
+        builder.setExecutionInfo(ImmutableMap.copyOf(castMap(
+            params.get("execution_requirements"),
+            String.class, String.class, "execution_requirements")));
       }
       if (params.containsKey("input_manifests")) {
         for (Map.Entry<PathFragment, Artifact> entry : castMap(params.get("input_manifests"),
-            PathFragment.class, Artifact.class, "input manifest file map")) {
+            PathFragment.class, Artifact.class, "input manifest file map").entrySet()) {
           builder.addInputManifest(entry.getValue(), entry.getKey());
         }
       }
@@ -224,8 +224,8 @@ public class SkylarkRuleImplementationFunctions {
         ConversionException {
       SkylarkRuleContext ctx = (SkylarkRuleContext) params.get("self");
       ImmutableList.Builder<Substitution> substitutions = ImmutableList.builder();
-      for (Map.Entry<String, String> substitution
-          : castMap(params.get("substitutions"), String.class, String.class, "substitutions")) {
+      for (Map.Entry<String, String> substitution : castMap(
+          params.get("substitutions"), String.class, String.class, "substitutions").entrySet()) {
         substitutions.add(Substitution.of(substitution.getKey(), substitution.getValue()));
       }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
index d12c141aef..0d3fae76e1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
@@ -775,17 +775,6 @@ public abstract class SkylarkType {
     });
   }
 
-  /** Build a map of the given key, value types from an Iterable of Map.Entry-s */
-  public static <KEY_TYPE, VALUE_TYPE> ImmutableMap<KEY_TYPE, VALUE_TYPE> toMap(
-      Iterable<Map.Entry<KEY_TYPE, VALUE_TYPE>> obj) {
-    ImmutableMap.Builder<KEY_TYPE, VALUE_TYPE> result = ImmutableMap.builder();
-    for (Map.Entry<KEY_TYPE, VALUE_TYPE> entry : obj) {
-      result.put(entry);
-    }
-
-    return result.build();
-  }
-
   /**
    * Cast a Map object into an Iterable of Map entries of the given key, value types.
    * @param obj the Map object, where null designates an empty map
@@ -793,35 +782,30 @@ public abstract class SkylarkType {
    * @param valueType the class of map values
    * @param what a string indicating what this is about, to include in case of error
    */
-  public static <KEY_TYPE, VALUE_TYPE> Iterable<Map.Entry<KEY_TYPE, VALUE_TYPE>> castMap(Object obj,
-      final Class<KEY_TYPE> keyType, final Class<VALUE_TYPE> valueType, final String what) {
+  @SuppressWarnings("unchecked")
+  public static <KEY_TYPE, VALUE_TYPE> Map<KEY_TYPE, VALUE_TYPE> castMap(Object obj,
+      Class<KEY_TYPE> keyType, Class<VALUE_TYPE> valueType, String what) {
     if (obj == null) {
-      return ImmutableList.of();
+      return ImmutableMap.of();
     }
     if (!(obj instanceof Map<?, ?>)) {
       throw new IllegalArgumentException(String.format(
           "expected a dictionary for %s but got %s instead",
           what, EvalUtils.getDataTypeName(obj)));
     }
-    return Iterables.transform(((Map<?, ?>) obj).entrySet(),
-        new com.google.common.base.Function<Map.Entry<?, ?>, Map.Entry<KEY_TYPE, VALUE_TYPE>>() {
-          // This is safe. We check the type of the key-value pairs for every entry in the Map.
-          // In Map.Entry the key always has the type of the first generic parameter, the
-          // value has the second.
-          @SuppressWarnings("unchecked")
-            @Override
-            public Map.Entry<KEY_TYPE, VALUE_TYPE> apply(Map.Entry<?, ?> input) {
-            if (keyType.isAssignableFrom(input.getKey().getClass())
-                && valueType.isAssignableFrom(input.getValue().getClass())) {
-              return (Map.Entry<KEY_TYPE, VALUE_TYPE>) input;
-            }
-            throw new IllegalArgumentException(String.format(
-                "expected <%s, %s> type for '%s' but got <%s, %s> instead",
-                keyType.getSimpleName(), valueType.getSimpleName(), what,
-                EvalUtils.getDataTypeName(input.getKey()),
-                EvalUtils.getDataTypeName(input.getValue())));
-          }
-        });
+
+    for (Map.Entry<?, ?> input : ((Map<?, ?>) obj).entrySet()) {
+      if (!keyType.isAssignableFrom(input.getKey().getClass())
+          || !valueType.isAssignableFrom(input.getValue().getClass())) {
+        throw new IllegalArgumentException(String.format(
+            "expected <%s, %s> type for '%s' but got <%s, %s> instead",
+            keyType.getSimpleName(), valueType.getSimpleName(), what,
+            EvalUtils.getDataTypeName(input.getKey()),
+            EvalUtils.getDataTypeName(input.getValue())));
+      }
+    }
+
+    return (Map<KEY_TYPE, VALUE_TYPE>) obj;
   }
 
   private static Class<?> getGenericTypeFromMethod(Method method) {