@AutoCodec some expressions that show up in Java configured targets.

PiperOrigin-RevId: 189419493

12 files changed, 136 insertions, 114 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
index 8e999b5518..f23146b3e1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -487,8 +487,9 @@ public final class CustomCommandLine extends CommandLine {
     }
   }
 
-  private static class FormatArg implements ArgvFragment {
-    private static final FormatArg INSTANCE = new FormatArg();
+  @AutoCodec.VisibleForSerialization
+  static class FormatArg implements ArgvFragment {
+    @AutoCodec @AutoCodec.VisibleForSerialization static final FormatArg INSTANCE = new FormatArg();
     private static final UUID FORMAT_UUID = UUID.fromString("377cee34-e947-49e0-94a2-6ab95b396ec4");
 
     private static void push(List<Object> arguments, String formatStr, Object... args) {
@@ -526,8 +527,9 @@ public final class CustomCommandLine extends CommandLine {
     }
   }
 
-  private static class PrefixArg implements ArgvFragment {
-    private static final PrefixArg INSTANCE = new PrefixArg();
+  @AutoCodec.VisibleForSerialization
+  static class PrefixArg implements ArgvFragment {
+    @AutoCodec @AutoCodec.VisibleForSerialization static final PrefixArg INSTANCE = new PrefixArg();
     private static final UUID PREFIX_UUID = UUID.fromString("a95eccdf-4f54-46fc-b925-c8c7e1f50c95");
 
     private static void push(List<Object> arguments, String before, Object arg) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
index a49d827091..e710f83a8a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
@@ -21,10 +21,12 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.util.FileType;
 
 /** A container of Java compilation artifacts. */
 @AutoValue
+@AutoCodec
 public abstract class JavaCompilationArgs {
   // TODO(bazel-team): It would be desirable to use LinkOrderNestedSet here so that
   // parents-before-deps is preserved for graphs that are not trees. However, the legacy
@@ -45,7 +47,9 @@ public abstract class JavaCompilationArgs {
           NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER),
           NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER));
 
-  private static JavaCompilationArgs create(
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec.Instantiator
+  static JavaCompilationArgs create(
       NestedSet<Artifact> runtimeJars,
       NestedSet<Artifact> compileTimeJars,
       NestedSet<Artifact> fullCompileTimeJars,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
index 34b0ddb17c..2c3816b58f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
@@ -22,26 +22,26 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.Collection;
 
-/**
- * An interface for objects that provide information on how to include them in
- * Java builds.
- */
+/** An interface for objects that provide information on how to include them in Java builds. */
 @AutoValue
 @Immutable
+@AutoCodec
 public abstract class JavaCompilationArgsProvider implements TransitiveInfoProvider {
 
+  @AutoCodec.Instantiator
   public static JavaCompilationArgsProvider create(
       JavaCompilationArgs javaCompilationArgs,
       JavaCompilationArgs recursiveJavaCompilationArgs,
-      NestedSet<Artifact> compileTimeJavaDepArtifacts,
-      NestedSet<Artifact> runTimeJavaDepArtifacts) {
+      NestedSet<Artifact> compileTimeJavaDependencyArtifacts,
+      NestedSet<Artifact> runTimeJavaDependencyArtifacts) {
     return new AutoValue_JavaCompilationArgsProvider(
         javaCompilationArgs,
         recursiveJavaCompilationArgs,
-        compileTimeJavaDepArtifacts,
-        runTimeJavaDepArtifacts);
+        compileTimeJavaDependencyArtifacts,
+        runTimeJavaDependencyArtifacts);
   }
 
   public static JavaCompilationArgsProvider create(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java
index a6d346a11e..89f3066ed5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java
@@ -20,31 +20,27 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.LinkedHashSet;
 import java.util.Set;
-
 import javax.annotation.Nullable;
 
 /**
- * A collection of artifacts for java compilations. It concisely describes the
- * outputs of a java-related rule, with runtime jars, compile-time jars,
- * unfiltered compile-time jars (these are run through ijar if they are
- * dependent upon by another target), source ijars, and instrumentation
- * manifests. Not all rules generate all kinds of artifacts. Each java-related
- * rule should add both a runtime jar and either a compile-time jar or an
- * unfiltered compile-time jar.
+ * A collection of artifacts for java compilations. It concisely describes the outputs of a
+ * java-related rule, with runtime jars, compile-time jars, unfiltered compile-time jars (these are
+ * run through ijar if they are dependent upon by another target), source ijars, and instrumentation
+ * manifests. Not all rules generate all kinds of artifacts. Each java-related rule should add both
+ * a runtime jar and either a compile-time jar or an unfiltered compile-time jar.
  *
- * <p>An instance of this class only collects the data for the current target,
- * not for the transitive closure of targets, so these still need to be
- * collected using some other mechanism, such as the {@link
- * JavaCompilationArgsProvider}.
+ * <p>An instance of this class only collects the data for the current target, not for the
+ * transitive closure of targets, so these still need to be collected using some other mechanism,
+ * such as the {@link JavaCompilationArgsProvider}.
  */
+@AutoCodec
 @Immutable
 @AutoValue
 public abstract class JavaCompilationArtifacts {
-
-  public static final JavaCompilationArtifacts EMPTY = new Builder().build();
+  @AutoCodec public static final JavaCompilationArtifacts EMPTY = new Builder().build();
 
   public abstract ImmutableList<Artifact> getRuntimeJars();
   public abstract ImmutableList<Artifact> getCompileTimeJars();
@@ -58,6 +54,24 @@ public abstract class JavaCompilationArtifacts {
     return new Builder();
   }
 
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec.Instantiator
+  static JavaCompilationArtifacts create(
+      ImmutableList<Artifact> runtimeJars,
+      ImmutableList<Artifact> compileTimeJars,
+      ImmutableList<Artifact> fullCompileTimeJars,
+      ImmutableList<Artifact> instrumentationMetadata,
+      Artifact compileTimeDependencyArtifact,
+      Artifact instrumentedJar) {
+    return new AutoValue_JavaCompilationArtifacts(
+        ImmutableList.copyOf(runtimeJars),
+        ImmutableList.copyOf(compileTimeJars),
+        ImmutableList.copyOf(fullCompileTimeJars),
+        ImmutableList.copyOf(instrumentationMetadata),
+        compileTimeDependencyArtifact,
+        instrumentedJar);
+  }
+
   /**
    * A builder for {@link JavaCompilationArtifacts}.
    */
@@ -71,7 +85,7 @@ public abstract class JavaCompilationArtifacts {
 
     public JavaCompilationArtifacts build() {
       Preconditions.checkState(fullCompileTimeJars.size() == compileTimeJars.size());
-      return new AutoValue_JavaCompilationArtifacts(
+      return create(
           ImmutableList.copyOf(runtimeJars),
           ImmutableList.copyOf(compileTimeJars),
           ImmutableList.copyOf(fullCompileTimeJars),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfoProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfoProvider.java
index 826cd7b954..59ed632abc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfoProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfoProvider.java
@@ -20,10 +20,10 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 
-/**
- * Provider for users of Java plugins.
- */
+/** Provider for users of Java plugins. */
+@AutoCodec
 @Immutable
 public final class JavaPluginInfoProvider implements TransitiveInfoProvider {
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSourceJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSourceJarsProvider.java
index 2907e26fb7..fc01eb1cf1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSourceJarsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSourceJarsProvider.java
@@ -22,21 +22,22 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.List;
 
 /** The collection of source jars from the transitive closure. */
 @AutoValue
 @Immutable
+@AutoCodec
 public abstract class JavaSourceJarsProvider implements TransitiveInfoProvider {
 
+  @AutoCodec.Instantiator
   public static JavaSourceJarsProvider create(
       NestedSet<Artifact> transitiveSourceJars, Iterable<Artifact> sourceJars) {
     return new AutoValue_JavaSourceJarsProvider(
         transitiveSourceJars, ImmutableList.copyOf(sourceJars));
   }
 
-  JavaSourceJarsProvider() {}
-
   /**
    * Returns all the source jars in the transitive closure, that can be reached by a chain of
    * JavaSourceJarsProvider instances.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
index 90f9252524..8a928da8c4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorAr
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.function.Consumer;
 
 /**
@@ -174,7 +175,9 @@ public final class SingleJarActionBuilder {
     return args.build();
   }
 
-  private static class ResourceArgMapFn extends CommandLineItem.ParametrizedMapFn<Artifact> {
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec
+  static class ResourceArgMapFn extends CommandLineItem.ParametrizedMapFn<Artifact> {
     private final JavaSemantics semantics;
 
     ResourceArgMapFn(JavaSemantics semantics) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
index cc5da78da8..3a3a507203 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
@@ -81,10 +81,11 @@ public class BazelLibrary {
         name = "items",
         type = Object.class,
         defaultValue = "[]",
-        doc = "Deprecated: Either an iterable whose items become the direct elements of "
-            + "the new depset, in left-to-right order, or else a depset that becomes "
-            + "a transitive element of the new depset. In the latter case, <code>transitive</code> "
-            + "cannot be specified."
+        doc =
+            "Deprecated: Either an iterable whose items become the direct elements of "
+                + "the new depset, in left-to-right order, or else a depset that becomes "
+                + "a transitive element of the new depset. In the latter case, "
+                + "<code>transitive</code> cannot be specified."
       ),
       @Param(
         name = "order",
@@ -95,13 +96,13 @@ public class BazelLibrary {
                 + "the possible values."
       ),
       @Param(
-          name = "direct",
-          type = SkylarkList.class,
-          defaultValue = "None",
-          positional = false,
-          named = true,
-          noneable = true,
-          doc =  "A list of <i>direct</i> elements of a depset."
+        name = "direct",
+        type = SkylarkList.class,
+        defaultValue = "None",
+        positional = false,
+        named = true,
+        noneable = true,
+        doc = "A list of <i>direct</i> elements of a depset."
       ),
       @Param(
         name = "transitive",
@@ -119,11 +120,7 @@ public class BazelLibrary {
   private static final BuiltinFunction depset =
       new BuiltinFunction("depset") {
         public SkylarkNestedSet invoke(
-            Object items,
-            String orderString,
-            Object direct,
-            Object transitive,
-            Location loc)
+            Object items, String orderString, Object direct, Object transitive, Location loc)
             throws EvalException {
           Order order;
           try {
@@ -134,7 +131,7 @@ public class BazelLibrary {
 
           if (transitive == Runtime.NONE && direct == Runtime.NONE) {
             // Legacy behavior.
-            return new SkylarkNestedSet(order, items, loc);
+            return SkylarkNestedSet.of(order, items, loc);
           }
 
           if (direct != Runtime.NONE && !isEmptySkylarkList(items)) {
@@ -154,8 +151,8 @@ public class BazelLibrary {
           Iterable<SkylarkNestedSet> transitiveList;
           if (transitive != Runtime.NONE) {
             SkylarkType.checkType(transitive, SkylarkList.class, "transitive");
-            transitiveList = ((SkylarkList<?>) transitive).getContents(
-                SkylarkNestedSet.class, "transitive");
+            transitiveList =
+                ((SkylarkList<?>) transitive).getContents(SkylarkNestedSet.class, "transitive");
           } else {
             transitiveList = ImmutableList.of();
           }
@@ -206,7 +203,7 @@ public class BazelLibrary {
           // newElements' type is Object because of the polymorphism on unioning two
           // SkylarkNestedSets versus a set and another kind of iterable.
           // Can't use EvalUtils#toIterable since that would discard this information.
-          return new SkylarkNestedSet(input, newElements, loc);
+          return SkylarkNestedSet.of(input, newElements, loc);
         }
       };
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index ab5badcb92..b56c9ebc90 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -331,7 +331,7 @@ public final class BinaryOperatorExpression extends Expression {
                 + "recommendations. Use --incompatible_depset_union=false "
                 + "to temporarily disable this check.");
       }
-      return new SkylarkNestedSet((SkylarkNestedSet) lval, rval, location);
+      return SkylarkNestedSet.of((SkylarkNestedSet) lval, rval, location);
     }
     throw typeException(lval, rval, Operator.PLUS, location);
   }
@@ -348,7 +348,7 @@ public final class BinaryOperatorExpression extends Expression {
                 + "recommendations. Use --incompatible_depset_union=false "
                 + "to temporarily disable this check.");
       }
-      return new SkylarkNestedSet((SkylarkNestedSet) lval, rval, location);
+      return SkylarkNestedSet.of((SkylarkNestedSet) lval, rval, location);
     }
     throw typeException(lval, rval, Operator.PIPE, location);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 86d9c369ee..c882743e78 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -557,15 +557,24 @@ public final class Environment implements Freezable {
       for (String name : names) {
         Object value = bindings.get(name);
         Object otherValue = otherBindings.get(name);
-        if (!value.equals(otherValue)) {
-          badEntries.add(String.format(
-              "%s: this one has %s (class %s), but given one has %s (class %s)",
-              name,
-              Printer.repr(value),
-              value.getClass().getName(),
-              Printer.repr(otherValue),
-              otherValue.getClass().getName()));
+        if (value.equals(otherValue)) {
+          continue;
         }
+        if (value instanceof SkylarkNestedSet
+            && otherValue instanceof SkylarkNestedSet
+            && (((SkylarkNestedSet) value)
+                .toCollection()
+                .equals(((SkylarkNestedSet) otherValue).toCollection()))) {
+          continue;
+        }
+        badEntries.add(
+            String.format(
+                "%s: this one has %s (class %s), but given one has %s (class %s)",
+                name,
+                Printer.repr(value),
+                value.getClass().getName(),
+                Printer.repr(otherValue),
+                otherValue.getClass().getName()));
       }
       if (!badEntries.isEmpty()) {
         throw new IllegalStateException(
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
index ce6dabc661..1ab3d1370a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
@@ -20,12 +20,12 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import java.util.Collection;
-import java.util.List;
 import javax.annotation.Nullable;
 
 /**
@@ -81,30 +81,34 @@ import javax.annotation.Nullable;
           + "(using <code>to_list()</code>). Duplicates may interfere with the ordering semantics."
 )
 @Immutable
+@AutoCodec
 public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
-
   private final SkylarkType contentType;
   private final NestedSet<?> set;
-  @Nullable
-  private final List<Object> items;
-  @Nullable
-  private final List<NestedSet> transitiveItems;
-
-  public SkylarkNestedSet(Order order, Object item, Location loc) throws EvalException {
-    this(order, SkylarkType.TOP, item, loc, null);
-  }
-
-  public SkylarkNestedSet(SkylarkNestedSet left, Object right, Location loc) throws EvalException {
-    this(left.set.getOrder(), left.contentType, right, loc, left);
+  @Nullable private final ImmutableList<Object> items;
+  @Nullable private final ImmutableList<NestedSet<?>> transitiveItems;
+
+  @AutoCodec.VisibleForSerialization
+  SkylarkNestedSet(
+      SkylarkType contentType,
+      NestedSet<?> set,
+      ImmutableList<Object> items,
+      ImmutableList<NestedSet<?>> transitiveItems) {
+    this.contentType = Preconditions.checkNotNull(contentType, "type cannot be null");
+    this.set = set;
+    this.items = items;
+    this.transitiveItems = transitiveItems;
   }
 
-  // This is safe because of the type checking
-  @SuppressWarnings("unchecked")
-  private SkylarkNestedSet(Order order, SkylarkType contentType, Object item, Location loc,
-      @Nullable SkylarkNestedSet left) throws EvalException {
-
+  static SkylarkNestedSet of(
+      Order order,
+      SkylarkType contentType,
+      Object item,
+      Location loc,
+      @Nullable SkylarkNestedSet left)
+      throws EvalException {
     ImmutableList.Builder<Object> itemsBuilder = ImmutableList.builder();
-    ImmutableList.Builder<NestedSet> transitiveItemsBuilder = ImmutableList.builder();
+    ImmutableList.Builder<NestedSet<?>> transitiveItemsBuilder = ImmutableList.builder();
     if (left != null) {
       if (left.items == null) { // SkylarkSet created from native NestedSet
         transitiveItemsBuilder.add(left.set);
@@ -137,29 +141,36 @@ public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
           String.format(
               "cannot union value of type '%s' to a depset", EvalUtils.getDataTypeName(item)));
     }
-    this.contentType = Preconditions.checkNotNull(contentType, "type cannot be null");
-    this.items = itemsBuilder.build();
-    this.transitiveItems = transitiveItemsBuilder.build();
-
+    ImmutableList<Object> items = itemsBuilder.build();
+    ImmutableList<NestedSet<?>> transitiveItems = transitiveItemsBuilder.build();
     // Initializing the real nested set
     NestedSetBuilder<Object> builder = new NestedSetBuilder<>(order);
-    builder.addAll(this.items);
+    builder.addAll(items);
     try {
-      for (NestedSet<?> nestedSet : this.transitiveItems) {
+      for (NestedSet<?> nestedSet : transitiveItems) {
         builder.addTransitive(nestedSet);
       }
     } catch (IllegalArgumentException e) {
       // Order mismatch between item and builder.
       throw new EvalException(loc, e.getMessage());
     }
-    this.set = builder.build();
+    return new SkylarkNestedSet(contentType, builder.build(), items, transitiveItems);
+  }
+
+  public static SkylarkNestedSet of(Order order, Object item, Location loc) throws EvalException {
+    return of(order, SkylarkType.TOP, item, loc, null);
+  }
+
+  public static SkylarkNestedSet of(SkylarkNestedSet left, Object right, Location loc)
+      throws EvalException {
+    return of(left.set.getOrder(), left.contentType, right, loc, left);
   }
 
   /**
    * Returns a type safe SkylarkNestedSet. Use this instead of the constructor if possible.
    */
   public static <T> SkylarkNestedSet of(SkylarkType contentType, NestedSet<T> set) {
-    return new SkylarkNestedSet(contentType, set);
+    return new SkylarkNestedSet(contentType, set, null, null);
   }
 
   /**
@@ -169,26 +180,6 @@ public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
     return of(SkylarkType.of(contentType), set);
   }
 
-  /**
-   * A not type safe constructor for SkylarkNestedSet. It's discouraged to use it unless type
-   * generic safety is guaranteed from the caller side.
-   */
-  SkylarkNestedSet(SkylarkType contentType, NestedSet<?> set) {
-    // This is here for the sake of FuncallExpression.
-    this.contentType = Preconditions.checkNotNull(contentType, "type cannot be null");
-    this.set = Preconditions.checkNotNull(set, "depset cannot be null");
-    this.items = null;
-    this.transitiveItems = null;
-  }
-
-  /**
-   * A not type safe constructor for SkylarkNestedSet, specifying type as a Java class.
-   * It's discouraged to use it unless type generic safety is guaranteed from the caller side.
-   */
-  public SkylarkNestedSet(Class<?> contentType, NestedSet<?> set) {
-    this(SkylarkType.of(contentType), set);
-  }
-
   private static final SkylarkType DICT_LIST_UNION =
       SkylarkType.Union.of(SkylarkType.DICT, SkylarkType.LIST);
 
@@ -384,7 +375,7 @@ public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
     }
 
     public SkylarkNestedSet build() {
-      return new SkylarkNestedSet(contentType, builder.build());
+      return new SkylarkNestedSet(contentType, builder.build(), null, null);
     }
   }
 }
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 f8213412de..b4729ace56 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
@@ -733,7 +733,8 @@ public abstract class SkylarkType implements Serializable {
    */
   static Object convertToSkylark(Object object, Method method, @Nullable Environment env) {
     if (object instanceof NestedSet<?>) {
-      return new SkylarkNestedSet(getGenericTypeFromMethod(method), (NestedSet<?>) object);
+      return SkylarkNestedSet.of(
+          SkylarkType.of(getGenericTypeFromMethod(method)), (NestedSet<?>) object);
     }
     return convertToSkylark(object, env);
   }