Reinstate mutable SkylarkDict

Add <String, Object> annotation to optionMap invocation in SkylarkAttr,
to make JDK 1.7 happy.

Give the visible name "aspect" to class SkylarkAspect.

--
MOS_MIGRATED_REVID=113394826

4 files changed, 107 insertions, 89 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 aa2fef91d5..6e22fbf269 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,7 +15,6 @@
 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;
@@ -34,6 +33,7 @@ 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,7 +42,6 @@ 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.
@@ -106,12 +105,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(Map<String, Object> arguments, String key) {
+  private static boolean containsNonNoneKey(SkylarkDict<String, Object> arguments, String key) {
     return arguments.containsKey(key) && arguments.get(key) != Runtime.NONE;
   }
 
   private static Attribute.Builder<?> createAttribute(
-      Type<?> type, Map<String, Object> arguments, FuncallExpression ast, Environment env)
+      Type<?> type, SkylarkDict<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).
@@ -192,7 +191,7 @@ public final class SkylarkAttr {
   }
 
   private static Descriptor createAttrDescriptor(
-      Map<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
+      SkylarkDict<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
       throws EvalException {
     try {
       return new Descriptor(createAttribute(type, kwargs, ast, env));
@@ -231,15 +230,15 @@ public final class SkylarkAttr {
         public Descriptor invoke(
             Integer defaultInt,
             Boolean mandatory,
-            SkylarkList values,
+            SkylarkList<?> values,
             FuncallExpression ast,
             Environment env)
             throws EvalException {
           // TODO(bazel-team): Replace literal strings with constants.
           env.checkLoadingPhase("attr.int", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
               Type.INTEGER,
               ast,
               env);
@@ -276,14 +275,14 @@ public final class SkylarkAttr {
         public Descriptor invoke(
             String defaultString,
             Boolean mandatory,
-            SkylarkList values,
+            SkylarkList<?> values,
             FuncallExpression ast,
             Environment env)
             throws EvalException {
           env.checkLoadingPhase("attr.string", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
               Type.STRING,
               ast,
               env);
@@ -361,7 +360,7 @@ public final class SkylarkAttr {
             Boolean executable,
             Object allowFiles,
             Boolean mandatory,
-            SkylarkList providers,
+            SkylarkList<?> providers,
             Object allowRules,
             Boolean singleFile,
             Object cfg,
@@ -370,7 +369,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.label", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
+              EvalUtils.<String, Object>optionMap(
+                  env,
                   DEFAULT_ARG,
                   defaultO,
                   EXECUTABLE_ARG,
@@ -417,7 +417,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction stringList =
       new BuiltinFunction("string_list") {
         public Descriptor invoke(
-            SkylarkList defaultList,
+            SkylarkList<?> defaultList,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -425,8 +425,14 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.string_list", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+              EvalUtils.<String, Object>optionMap(
+                  env,
+                  DEFAULT_ARG,
+                  defaultList,
+                  MANDATORY_ARG,
+                  mandatory,
+                  NON_EMPTY_ARG,
+                  nonEmpty),
               Type.STRING_LIST,
               ast,
               env);
@@ -457,7 +463,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction intList =
       new BuiltinFunction("int_list") {
         public Descriptor invoke(
-            SkylarkList defaultList,
+            SkylarkList<?> defaultList,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -465,8 +471,14 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.int_list", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+              EvalUtils.<String, Object>optionMap(
+                  env,
+                  DEFAULT_ARG,
+                  defaultList,
+                  MANDATORY_ARG,
+                  mandatory,
+                  NON_EMPTY_ARG,
+                  nonEmpty),
               Type.INTEGER_LIST,
               ast,
               env);
@@ -547,18 +559,20 @@ public final class SkylarkAttr {
             Object defaultList,
             Object allowFiles,
             Object allowRules,
-            SkylarkList providers,
-            SkylarkList flags,
+            SkylarkList<?> providers,
+            SkylarkList<?> flags,
             Boolean mandatory,
             Boolean nonEmpty,
             Object cfg,
-            SkylarkList aspects,
+            SkylarkList<?> aspects,
             FuncallExpression ast,
             Environment env)
             throws EvalException {
           env.checkLoadingPhase("attr.label_list", ast.getLocation());
-          ImmutableMap<String, Object> kwargs = EvalUtils.optionMap(
-              DEFAULT_ARG, defaultList,
+          SkylarkDict<String, Object> kwargs = EvalUtils.<String, Object>optionMap(
+              env,
+              DEFAULT_ARG,
+              defaultList,
               ALLOW_FILES_ARG,
               allowFiles,
               ALLOW_RULES_ARG,
@@ -576,24 +590,12 @@ public final class SkylarkAttr {
           try {
             Attribute.Builder<?> attribute = createAttribute(
                 BuildType.LABEL_LIST, kwargs, ast, env);
-
-            ImmutableList<SkylarkAspect> skylarkAspects = getSkylarkAspects(aspects);
+            ImmutableList<SkylarkAspect> skylarkAspects =
+                ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects"));
             return new Descriptor(attribute, skylarkAspects);
-          } catch (ConversionException e) {
-            throw new EvalException(ast.getLocation(), e.getMessage());
-          }
-        }
-
-        protected ImmutableList<SkylarkAspect> getSkylarkAspects(SkylarkList aspects)
-            throws ConversionException {
-          ImmutableList.Builder<SkylarkAspect> aspectBuilder = ImmutableList.builder();
-          for (Object aspect : aspects) {
-            if (!(aspect instanceof SkylarkAspect)) {
-              throw new ConversionException("Expected a list of aspects for 'aspects'");
-            }
-            aspectBuilder.add((SkylarkAspect) aspect);
+          } catch (EvalException e) {
+            throw new EvalException(ast.getLocation(), e.getMessage(), e);
           }
-          return aspectBuilder.build();
         }
       };
 
@@ -617,7 +619,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.bool", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
               Type.BOOLEAN,
               ast,
               env);
@@ -653,7 +656,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.output", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
               BuildType.OUTPUT,
               ast,
               env);
@@ -694,8 +698,14 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.output_list", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+              EvalUtils.<String, Object>optionMap(
+                  env,
+                  DEFAULT_ARG,
+                  defaultList,
+                  MANDATORY_ARG,
+                  mandatory,
+                  NON_EMPTY_ARG,
+                  nonEmpty),
               BuildType.OUTPUT_LIST,
               ast,
               env);
@@ -710,7 +720,7 @@ public final class SkylarkAttr {
     objectType = SkylarkAttr.class,
     returnType = Descriptor.class,
     optionalNamedOnly = {
-      @Param(name = DEFAULT_ARG, type = Map.class, defaultValue = "{}", doc = DEFAULT_DOC),
+      @Param(name = DEFAULT_ARG, type = SkylarkDict.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
@@ -722,7 +732,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction stringDict =
       new BuiltinFunction("string_dict") {
         public Descriptor invoke(
-            Map<?, ?> defaultO,
+            SkylarkDict<?, ?> defaultO,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -730,8 +740,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.string_dict", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.STRING_DICT,
               ast,
               env);
@@ -746,7 +756,7 @@ public final class SkylarkAttr {
     objectType = SkylarkAttr.class,
     returnType = Descriptor.class,
     optionalNamedOnly = {
-      @Param(name = DEFAULT_ARG, type = Map.class, defaultValue = "{}", doc = DEFAULT_DOC),
+      @Param(name = DEFAULT_ARG, type = SkylarkDict.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
@@ -758,7 +768,7 @@ public final class SkylarkAttr {
   private static BuiltinFunction stringListDict =
       new BuiltinFunction("string_list_dict") {
         public Descriptor invoke(
-            Map<?, ?> defaultO,
+            SkylarkDict<?, ?> defaultO,
             Boolean mandatory,
             Boolean nonEmpty,
             FuncallExpression ast,
@@ -766,8 +776,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.string_list_dict", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(
-                  DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
+              EvalUtils.<String, Object>optionMap(
+                  env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
               Type.STRING_LIST_DICT,
               ast,
               env);
@@ -796,7 +806,8 @@ public final class SkylarkAttr {
             throws EvalException {
           env.checkLoadingPhase("attr.license", ast.getLocation());
           return createAttrDescriptor(
-              EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
+              EvalUtils.<String, Object>optionMap(
+                  env, 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 bfe75ec7ae..23d6b87b43 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
@@ -69,6 +69,7 @@ import com.google.devtools.build.lib.packages.SkylarkAspectClass;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.TestSize;
 import com.google.devtools.build.lib.rules.SkylarkAttr.Descriptor;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
@@ -85,6 +86,7 @@ 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;
@@ -224,7 +226,8 @@ 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 = Map.class, noneable = true, defaultValue = "None", doc =
+        @Param(name = "attrs", type = SkylarkDict.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 "
@@ -233,7 +236,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 = Map.class, callbackEnabled = true, noneable = true,
+        @Param(name = "outputs", type = SkylarkDict.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>"
@@ -362,7 +365,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 = Map.class, noneable = true, defaultValue = "None",
+      @Param(name = "attrs", type = SkylarkDict.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). "
@@ -698,6 +701,7 @@ public class SkylarkRuleClassFunctions {
   /**
    * A Skylark value that is a result of 'aspect(..)' function call.
    */
+  @SkylarkModule(name = "aspect", doc = "", documented = false)
   public static final class SkylarkAspect implements SkylarkValue {
     private final BaseFunction implementation;
     private final ImmutableList<String> attributeAspects;
@@ -840,10 +844,12 @@ 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 c57f48e732..32f812508a 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,6 +52,7 @@ 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;
@@ -152,7 +153,7 @@ public final class SkylarkRuleContext {
 
   private final FragmentCollection hostFragments;
 
-  private final ImmutableMap<String, String> makeVariables;
+  private final SkylarkDict<String, String> makeVariables;
   private final SkylarkRuleAttributesCollection attributesCollection;
   private final SkylarkRuleAttributesCollection ruleAttributesCollection;
 
@@ -517,7 +518,7 @@ public final class SkylarkRuleContext {
 
   @SkylarkCallable(structField = true,
       doc = "Dictionary (String to String) of configuration variables")
-  public ImmutableMap<String, String> var() {
+  public SkylarkDict<String, String> var() {
     return makeVariables;
   }
 
@@ -527,7 +528,7 @@ public final class SkylarkRuleContext {
   }
 
   @SkylarkCallable(doc = "Splits a shell command to a list of tokens.", documented = false)
-  public MutableList tokenize(String optionString) throws FuncallException {
+  public MutableList<String> tokenize(String optionString) throws FuncallException {
     List<String> options = new ArrayList<>();
     try {
       ShellUtils.tokenize(options, optionString);
@@ -544,7 +545,8 @@ public final class SkylarkRuleContext {
           + "Deprecated.",
     documented = false
   )
-  public String expand(@Nullable String expression, SkylarkList artifacts, Label labelResolver)
+  public String expand(
+      @Nullable String expression, SkylarkList<Object> artifacts, Label labelResolver)
       throws EvalException, FuncallException {
     try {
       Map<Label, Iterable<Artifact>> labelMap = new HashMap<>();
@@ -596,7 +598,7 @@ public final class SkylarkRuleContext {
   }
 
   @SkylarkCallable(documented = false)
-  public boolean checkPlaceholders(String template, SkylarkList allowedPlaceholders)
+  public boolean checkPlaceholders(String template, SkylarkList<Object> 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 4103b81613..7b69e37684 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,6 +52,7 @@ 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;
@@ -143,8 +144,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",
@@ -163,14 +164,14 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "env",
-        type = Map.class,
+        type = SkylarkDict.class,
         noneable = true,
         defaultValue = "None",
         doc = "sets the dictionary of environment variables"
       ),
       @Param(
         name = "execution_requirements",
-        type = Map.class,
+        type = SkylarkDict.class,
         noneable = true,
         defaultValue = "None",
         doc =
@@ -179,7 +180,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "input_manifests",
-        type = Map.class,
+        type = SkylarkDict.class,
         noneable = true,
         defaultValue = "None",
         doc =
@@ -203,7 +204,7 @@ public class SkylarkRuleImplementationFunctions {
             Boolean useDefaultShellEnv,
             Object envO,
             Object executionRequirementsO,
-            Object inputManifestsO,
+            Object inputManifests,
             Location loc)
             throws EvalException, ConversionException {
           SpawnAction.Builder builder = new SpawnAction.Builder();
@@ -289,14 +290,11 @@ public class SkylarkRuleImplementationFunctions {
                         String.class,
                         "execution_requirements")));
           }
-          if (inputManifestsO != Runtime.NONE) {
+          if (inputManifests instanceof SkylarkDict) {
             for (Map.Entry<PathFragment, Artifact> entry :
-                castMap(
-                        inputManifestsO,
-                        PathFragment.class,
-                        Artifact.class,
-                        "input manifest file map")
-                    .entrySet()) {
+                     ((SkylarkDict<?, ?>) inputManifests)
+                     .getContents(PathFragment.class, Artifact.class, "input manifest file map")
+                     .entrySet()) {
               builder.addInputManifest(entry.getValue(), entry.getKey());
             }
           }
@@ -459,7 +457,7 @@ public class SkylarkRuleImplementationFunctions {
             doc = "the template file"),
         @Param(name = "output", type = Artifact.class,
             doc = "the output file"),
-        @Param(name = "substitutions", type = Map.class,
+        @Param(name = "substitutions", type = SkylarkDict.class,
             doc = "substitutions to make when expanding the template")},
       optionalNamedOnly = {
         @Param(name = "executable", type = Boolean.class,
@@ -467,23 +465,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, Map<?, ?> substitutionsO, Boolean executable)
+            Artifact template, Artifact output, SkylarkDict<?, ?> substitutions, Boolean executable)
             throws EvalException, ConversionException {
-          ImmutableList.Builder<Substitution> substitutions = ImmutableList.builder();
-          for (Map.Entry<String, String> substitution : castMap(
-              substitutionsO, String.class, String.class, "substitutions").entrySet()) {
+          ImmutableList.Builder<Substitution> substitutionsBuilder = ImmutableList.builder();
+          for (Map.Entry<String, String> substitution : substitutions.getContents(
+              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.
-            substitutions.add(Substitution.of(
+            substitutionsBuilder.add(Substitution.of(
                 substitution.getKey(), convertLatin1ToUtf8(substitution.getValue())));
           }
           TemplateExpansionAction action = new TemplateExpansionAction(
               ctx.getRuleContext().getActionOwner(),
               template,
               output,
-              substitutions.build(),
+              substitutionsBuilder.build(),
               executable);
           ctx.getRuleContext().registerAction(action);
           return action;
@@ -574,7 +572,8 @@ 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();
@@ -633,7 +632,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "make_variables",
-        type = Map.class, // dict(string, string)
+        type = SkylarkDict.class, // dict(string, string)
         noneable = true,
         doc = "make variables to expand, or None"
       ),
@@ -646,7 +645,7 @@ public class SkylarkRuleImplementationFunctions {
       ),
       @Param(
         name = "label_dict",
-        type = Map.class,
+        type = SkylarkDict.class,
         defaultValue = "{}",
         doc =
             "dictionary of resolved labels and the corresponding list of Files "
@@ -658,24 +657,24 @@ public class SkylarkRuleImplementationFunctions {
   private static final BuiltinFunction resolveCommand =
       new BuiltinFunction("resolve_command") {
         @SuppressWarnings("unchecked")
-        public Tuple invoke(
+        public Tuple<Object> invoke(
             SkylarkRuleContext ctx,
             String command,
             Object attributeO,
             Boolean expandLocations,
             Object makeVariablesO,
             SkylarkList tools,
-            Map<?, ?> labelDictM,
+            SkylarkDict<?, ?> labelDict,
             Location loc,
             Environment env)
             throws ConversionException, EvalException {
           Label ruleLabel = ctx.getLabel();
-          Map<Label, Iterable<Artifact>> labelDict = checkLabelDict(labelDictM, loc);
+          Map<Label, Iterable<Artifact>> labelDictM = checkLabelDict(labelDict, 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(labelDict));
+              ImmutableMap.copyOf(labelDictM));
           String attribute = Type.STRING.convertOptional(attributeO, "attribute", ruleLabel);
           if (expandLocations) {
             command = helper.resolveCommandAndExpandLabels(
@@ -689,7 +688,7 @@ public class SkylarkRuleImplementationFunctions {
           List<Artifact> inputs = new ArrayList<>();
           inputs.addAll(helper.getResolvedTools());
           List<String> argv = helper.buildCommandLine(command, inputs, SCRIPT_SUFFIX);
-          return Tuple.of(
+          return Tuple.<Object>of(
               new MutableList(inputs, env),
               new MutableList(argv, env),
               helper.getRemoteRunfileManifestMap());