Rollback of commit c0a8c58b9230a1f5d76269eb7dc6b11e18f19686.

*** Reason for rollback ***

Break Java 1.7 builds of Bazel.

See http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.7,PLATFORM_NAME=linux-x86_64/327/console

Test: 
git clone ... && git revert c0a8c58 && export JAVA_VERSION=1.7 && export BAZEL_COMPILE_TARGET=compile && bash -c "source scripts/ci/build.sh; bazel_build"

*** Original change description ***

Make Skylark dicts mutable

Represent Skylark dict using a new subclass SkylarkDict<K, V> of Map<K, V>.
Back it with a TreeMap to provide a deterministic iteration order.
Also make SkylarkList generic in its element type <E>.
Have Artifact implement Comparable<Object> so it can be used as TreeMap key.

--
MOS_MIGRATED_REVID=113359718

4 files changed, 57 insertions, 82 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 6eb1abc72c..aa2fef91d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -33,7 +34,6 @@ import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
-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.syntax.Type;
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.syntax.UserDefinedFunction;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
 import java.util.List;
+import java.util.Map;
 
 /**
  * A helper class to provide Attr module in Skylark.
@@ -105,12 +106,12 @@ public final class SkylarkAttr {
       "the list of allowed values for the attribute. An error is raised if any other "
           + "value is given.";
 
-  private static boolean containsNonNoneKey(SkylarkDict<String, Object> arguments, String key) {
+  private static boolean containsNonNoneKey(Map<String, Object> arguments, String key) {
     return arguments.containsKey(key) && arguments.get(key) != Runtime.NONE;
   }
 
   private static Attribute.Builder<?> createAttribute(
-      Type<?> type, SkylarkDict<String, Object> arguments, FuncallExpression ast, Environment env)
+      Type<?> type, Map<String, Object> arguments, FuncallExpression ast, Environment env)
       throws EvalException, ConversionException {
     // We use an empty name now so that we can set it later.
     // This trick makes sense only in the context of Skylark (builtin rules should not use it).
@@ -191,7 +192,7 @@ public final class SkylarkAttr {
   }
 
   private static Descriptor createAttrDescriptor(
-      SkylarkDict<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
+      Map<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
       throws EvalException {
     try {
       return new Descriptor(createAttribute(type, kwargs, ast, env));
@@ -238,7 +239,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.int", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env, DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
+                  DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
               Type.INTEGER,
               ast,
               env);
@@ -282,7 +283,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.string", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env, DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
+                  DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
               Type.STRING,
               ast,
               env);
@@ -370,7 +371,6 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.label", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env,
                   DEFAULT_ARG,
                   defaultO,
                   EXECUTABLE_ARG,
@@ -426,13 +426,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.string_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env,
-                  DEFAULT_ARG,
-                  defaultList,
-                  MANDATORY_ARG,
-                  mandatory,
-                  NON_EMPTY_ARG,
-                  nonEmpty),
+                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.STRING_LIST,
               ast,
               env);
@@ -472,13 +466,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.int_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env,
-                  DEFAULT_ARG,
-                  defaultList,
-                  MANDATORY_ARG,
-                  mandatory,
-                  NON_EMPTY_ARG,
-                  nonEmpty),
+                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.INTEGER_LIST,
               ast,
               env);
@@ -569,10 +557,8 @@ public final class SkylarkAttr {
             Environment env)
             throws EvalException {
           env.checkLoadingPhase("attr.label_list", ast.getLocation());
-          SkylarkDict<String, Object> kwargs = EvalUtils.optionMap(
-              env,
-              DEFAULT_ARG,
-              defaultList,
+          ImmutableMap<String, Object> kwargs = EvalUtils.optionMap(
+              DEFAULT_ARG, defaultList,
               ALLOW_FILES_ARG,
               allowFiles,
               ALLOW_RULES_ARG,
@@ -631,7 +617,7 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.bool", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
               Type.BOOLEAN,
               ast,
               env);
@@ -667,7 +653,7 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.output", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
               BuildType.OUTPUT,
               ast,
               env);
@@ -709,13 +695,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.output_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env,
-                  DEFAULT_ARG,
-                  defaultList,
-                  MANDATORY_ARG,
-                  mandatory,
-                  NON_EMPTY_ARG,
-                  nonEmpty),
+                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               BuildType.OUTPUT_LIST,
               ast,
               env);
@@ -730,7 +710,7 @@ public final class SkylarkAttr {
     objectType = SkylarkAttr.class,
     returnType = Descriptor.class,
     optionalNamedOnly = {
-      @Param(name = DEFAULT_ARG, type = SkylarkDict.class, defaultValue = "{}", doc = DEFAULT_DOC),
+      @Param(name = DEFAULT_ARG, type = Map.class, defaultValue = "{}", doc = DEFAULT_DOC),
       @Param(name = MANDATORY_ARG, type = Boolean.class, defaultValue = "False", doc = MANDATORY_DOC
       ),
       @Param(name = NON_EMPTY_ARG, type = Boolean.class, defaultValue = "False", doc = NON_EMPTY_DOC
@@ -742,7 +722,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction stringDict =
       new BuiltinFunction("string_dict") {
         public Descriptor invoke(
-            SkylarkDict<?, ?> defaultO,
+            Map<?, ?> defaultO,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -751,7 +731,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.string_dict", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+                  DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.STRING_DICT,
               ast,
               env);
@@ -766,7 +746,7 @@ public final class SkylarkAttr {
     objectType = SkylarkAttr.class,
     returnType = Descriptor.class,
     optionalNamedOnly = {
-      @Param(name = DEFAULT_ARG, type = SkylarkDict.class, defaultValue = "{}", doc = DEFAULT_DOC),
+      @Param(name = DEFAULT_ARG, type = Map.class, defaultValue = "{}", doc = DEFAULT_DOC),
       @Param(name = MANDATORY_ARG, type = Boolean.class, defaultValue = "False", doc = MANDATORY_DOC
       ),
       @Param(name = NON_EMPTY_ARG, type = Boolean.class, defaultValue = "False", doc = NON_EMPTY_DOC
@@ -778,7 +758,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction stringListDict =
       new BuiltinFunction("string_list_dict") {
         public Descriptor invoke(
-            SkylarkDict<?, ?> defaultO,
+            Map<?, ?> defaultO,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -787,7 +767,7 @@ public final class SkylarkAttr {
           env.checkLoadingPhase("attr.string_list_dict", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.optionMap(
-                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+                  DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.STRING_LIST_DICT,
               ast,
               env);
@@ -816,7 +796,7 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.license", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
               BuildType.LICENSE,
               ast,
               env);
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 2c5cc0140b..bfe75ec7ae 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
@@ -85,7 +85,6 @@ import com.google.devtools.build.lib.syntax.FunctionSignature;
 import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
-import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
@@ -225,8 +224,7 @@ public class SkylarkRuleClassFunctions {
             doc = "Whether this rule is a test rule. "
             + "If True, the rule must end with <code>_test</code> (otherwise it must not), "
             + "and there must be an action that generates <code>ctx.outputs.executable</code>."),
-        @Param(name = "attrs", type = SkylarkDict.class, noneable = true, defaultValue = "None",
-            doc =
+        @Param(name = "attrs", type = Map.class, noneable = true, defaultValue = "None", doc =
             "dictionary to declare all the attributes of the rule. It maps from an attribute name "
             + "to an attribute object (see <a href=\"attr.html\">attr</a> module). "
             + "Attributes starting with <code>_</code> are private, and can be used to add "
@@ -235,7 +233,7 @@ public class SkylarkRuleClassFunctions {
             + "<code>deprecation</code>, <code>tags</code>, <code>testonly</code>, and "
             + "<code>features</code> are implicitly added and might be overriden."),
             // TODO(bazel-team): need to give the types of these builtin attributes
-        @Param(name = "outputs", type = SkylarkDict.class, callbackEnabled = true, noneable = true,
+        @Param(name = "outputs", type = Map.class, callbackEnabled = true, noneable = true,
             defaultValue = "None", doc = "outputs of this rule. "
             + "It is a dictionary mapping from string to a template name. "
             + "For example: <code>{\"ext\": \"%{name}.ext\"}</code>. <br>"
@@ -364,7 +362,7 @@ public class SkylarkRuleClassFunctions {
         doc = "List of attribute names.  The aspect propagates along dependencies specified by "
         + " attributes of a target with this name"
       ),
-      @Param(name = "attrs", type = SkylarkDict.class, noneable = true, defaultValue = "None",
+      @Param(name = "attrs", type = Map.class, noneable = true, defaultValue = "None",
         doc = "dictionary to declare all the attributes of the aspect.  "
         + "It maps from an attribute name to an attribute object "
         + "(see <a href=\"attr.html\">attr</a> module). "
@@ -842,12 +840,10 @@ public class SkylarkRuleClassFunctions {
       return aspectDefinition;
     }
 
-    @Override
     public Label getExtensionLabel() {
       return extensionLabel;
     }
 
-    @Override
     public String getExportedName() {
       return exportedName;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index 32f812508a..c57f48e732 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -52,7 +52,6 @@ import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FuncallExpression.FuncallException;
 import com.google.devtools.build.lib.syntax.Runtime;
-import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkType;
@@ -153,7 +152,7 @@ public final class SkylarkRuleContext {
 
   private final FragmentCollection hostFragments;
 
-  private final SkylarkDict<String, String> makeVariables;
+  private final ImmutableMap<String, String> makeVariables;
   private final SkylarkRuleAttributesCollection attributesCollection;
   private final SkylarkRuleAttributesCollection ruleAttributesCollection;
 
@@ -518,7 +517,7 @@ public final class SkylarkRuleContext {
 
   @SkylarkCallable(structField = true,
       doc = "Dictionary (String to String) of configuration variables")
-  public SkylarkDict<String, String> var() {
+  public ImmutableMap<String, String> var() {
     return makeVariables;
   }
 
@@ -528,7 +527,7 @@ public final class SkylarkRuleContext {
   }
 
   @SkylarkCallable(doc = "Splits a shell command to a list of tokens.", documented = false)
-  public MutableList<String> tokenize(String optionString) throws FuncallException {
+  public MutableList tokenize(String optionString) throws FuncallException {
     List<String> options = new ArrayList<>();
     try {
       ShellUtils.tokenize(options, optionString);
@@ -545,8 +544,7 @@ public final class SkylarkRuleContext {
           + "Deprecated.",
     documented = false
   )
-  public String expand(
-      @Nullable String expression, SkylarkList<Object> artifacts, Label labelResolver)
+  public String expand(@Nullable String expression, SkylarkList artifacts, Label labelResolver)
       throws EvalException, FuncallException {
     try {
       Map<Label, Iterable<Artifact>> labelMap = new HashMap<>();
@@ -598,7 +596,7 @@ public final class SkylarkRuleContext {
   }
 
   @SkylarkCallable(documented = false)
-  public boolean checkPlaceholders(String template, SkylarkList<Object> allowedPlaceholders)
+  public boolean checkPlaceholders(String template, SkylarkList allowedPlaceholders)
       throws EvalException {
     List<String> actualPlaceHolders = new LinkedList<>();
     Set<String> allowedPlaceholderSet =
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 7b69e37684..4103b81613 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
@@ -52,7 +52,6 @@ import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.Runtime;
-import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
@@ -144,8 +143,8 @@ public class SkylarkRuleImplementationFunctions {
         defaultValue = "None",
         doc =
             "shell command to execute. It is usually preferable to "
-                + "use <code>executable</code> instead. "
-                + "Arguments are available with <code>$1</code>, <code>$2</code>, etc."
+                + "use <code>executable</code> instead. Arguments are available with <code>$1</code>, "
+                + "<code>$2</code>, etc."
       ),
       @Param(
         name = "progress_message",
@@ -164,14 +163,14 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "env",
-        type = SkylarkDict.class,
+        type = Map.class,
         noneable = true,
         defaultValue = "None",
         doc = "sets the dictionary of environment variables"
       ),
       @Param(
         name = "execution_requirements",
-        type = SkylarkDict.class,
+        type = Map.class,
         noneable = true,
         defaultValue = "None",
         doc =
@@ -180,7 +179,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "input_manifests",
-        type = SkylarkDict.class,
+        type = Map.class,
         noneable = true,
         defaultValue = "None",
         doc =
@@ -204,7 +203,7 @@ public class SkylarkRuleImplementationFunctions {
             Boolean useDefaultShellEnv,
             Object envO,
             Object executionRequirementsO,
-            Object inputManifests,
+            Object inputManifestsO,
             Location loc)
             throws EvalException, ConversionException {
           SpawnAction.Builder builder = new SpawnAction.Builder();
@@ -290,11 +289,14 @@ public class SkylarkRuleImplementationFunctions {
                         String.class,
                         "execution_requirements")));
           }
-          if (inputManifests instanceof SkylarkDict) {
+          if (inputManifestsO != Runtime.NONE) {
             for (Map.Entry<PathFragment, Artifact> entry :
-                     ((SkylarkDict<?, ?>) inputManifests)
-                     .getContents(PathFragment.class, Artifact.class, "input manifest file map")
-                     .entrySet()) {
+                castMap(
+                        inputManifestsO,
+                        PathFragment.class,
+                        Artifact.class,
+                        "input manifest file map")
+                    .entrySet()) {
               builder.addInputManifest(entry.getValue(), entry.getKey());
             }
           }
@@ -457,7 +459,7 @@ public class SkylarkRuleImplementationFunctions {
             doc = "the template file"),
         @Param(name = "output", type = Artifact.class,
             doc = "the output file"),
-        @Param(name = "substitutions", type = SkylarkDict.class,
+        @Param(name = "substitutions", type = Map.class,
             doc = "substitutions to make when expanding the template")},
       optionalNamedOnly = {
         @Param(name = "executable", type = Boolean.class,
@@ -465,23 +467,23 @@ public class SkylarkRuleImplementationFunctions {
   private static final BuiltinFunction createTemplateAction =
       new BuiltinFunction("template_action", Arrays.<Object>asList(false)) {
         public TemplateExpansionAction invoke(SkylarkRuleContext ctx,
-            Artifact template, Artifact output, SkylarkDict<?, ?> substitutions, Boolean executable)
+            Artifact template, Artifact output, Map<?, ?> substitutionsO, Boolean executable)
             throws EvalException, ConversionException {
-          ImmutableList.Builder<Substitution> substitutionsBuilder = ImmutableList.builder();
-          for (Map.Entry<String, String> substitution : substitutions.getContents(
-              String.class, String.class, "substitutions").entrySet()) {
+          ImmutableList.Builder<Substitution> substitutions = ImmutableList.builder();
+          for (Map.Entry<String, String> substitution : castMap(
+              substitutionsO, String.class, String.class, "substitutions").entrySet()) {
             // ParserInputSource.create(Path) uses Latin1 when reading BUILD files, which might
             // contain UTF-8 encoded symbols as part of template substitution.
             // As a quick fix, the substitution values are corrected before being passed on.
             // In the long term, fixing ParserInputSource.create(Path) would be a better approach.
-            substitutionsBuilder.add(Substitution.of(
+            substitutions.add(Substitution.of(
                 substitution.getKey(), convertLatin1ToUtf8(substitution.getValue())));
           }
           TemplateExpansionAction action = new TemplateExpansionAction(
               ctx.getRuleContext().getActionOwner(),
               template,
               output,
-              substitutionsBuilder.build(),
+              substitutions.build(),
               executable);
           ctx.getRuleContext().registerAction(action);
           return action;
@@ -572,8 +574,7 @@ public class SkylarkRuleImplementationFunctions {
 
   // TODO(bazel-team): find a better way to typecheck this argument.
   @SuppressWarnings("unchecked")
-  private static Map<Label, Iterable<Artifact>> checkLabelDict(
-      Map<?, ?> labelDict, Location loc)
+  private static Map<Label, Iterable<Artifact>> checkLabelDict(Map<?, ?> labelDict, Location loc)
       throws EvalException {
     for (Map.Entry<?, ?> entry : labelDict.entrySet()) {
       Object key = entry.getKey();
@@ -632,7 +633,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "make_variables",
-        type = SkylarkDict.class, // dict(string, string)
+        type = Map.class, // dict(string, string)
         noneable = true,
         doc = "make variables to expand, or None"
       ),
@@ -645,7 +646,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "label_dict",
-        type = SkylarkDict.class,
+        type = Map.class,
         defaultValue = "{}",
         doc =
             "dictionary of resolved labels and the corresponding list of Files "
@@ -657,24 +658,24 @@ public class SkylarkRuleImplementationFunctions {
   private static final BuiltinFunction resolveCommand =
       new BuiltinFunction("resolve_command") {
         @SuppressWarnings("unchecked")
-        public Tuple<Object> invoke(
+        public Tuple invoke(
             SkylarkRuleContext ctx,
             String command,
             Object attributeO,
             Boolean expandLocations,
             Object makeVariablesO,
             SkylarkList tools,
-            SkylarkDict<?, ?> labelDict,
+            Map<?, ?> labelDictM,
             Location loc,
             Environment env)
             throws ConversionException, EvalException {
           Label ruleLabel = ctx.getLabel();
-          Map<Label, Iterable<Artifact>> labelDictM = checkLabelDict(labelDict, loc);
+          Map<Label, Iterable<Artifact>> labelDict = checkLabelDict(labelDictM, loc);
           // The best way to fix this probably is to convert CommandHelper to Skylark.
           CommandHelper helper = new CommandHelper(
               ctx.getRuleContext(),
               tools.getContents(TransitiveInfoCollection.class, "tools"),
-              ImmutableMap.copyOf(labelDictM));
+              ImmutableMap.copyOf(labelDict));
           String attribute = Type.STRING.convertOptional(attributeO, "attribute", ruleLabel);
           if (expandLocations) {
             command = helper.resolveCommandAndExpandLabels(
@@ -688,7 +689,7 @@ public class SkylarkRuleImplementationFunctions {
           List<Artifact> inputs = new ArrayList<>();
           inputs.addAll(helper.getResolvedTools());
           List<String> argv = helper.buildCommandLine(command, inputs, SCRIPT_SUFFIX);
-          return Tuple.<Object>of(
+          return Tuple.of(
               new MutableList(inputs, env),
               new MutableList(argv, env),
               helper.getRemoteRunfileManifestMap());