Migrate from deprecated MutableList constructor to copyOf, fixing bug in latter

PiperOrigin-RevId: 171176871

8 files changed, 23 insertions, 34 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleImplementationFunctions.java
index 6d1e73883f..f71730b10a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleImplementationFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleImplementationFunctions.java
@@ -732,8 +732,8 @@ public class SkylarkRuleImplementationFunctions {
           List<String> argv =
               helper.buildCommandLine(command, inputs, SCRIPT_SUFFIX, executionRequirements);
           return Tuple.<Object>of(
-              new MutableList(inputs, env),
-              new MutableList(argv, env),
+              MutableList.copyOf(env, inputs),
+              MutableList.copyOf(env, argv),
               helper.getToolsRunfilesSuppliers());
         }
       };
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 53c8f0af1b..ce3e1c0e67 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -592,7 +592,7 @@ public final class PackageFactory {
     }
 
     GlobList<String> globList = GlobList.captureResults(includes, excludes, matches);
-    return new MutableList<>(globList, env);
+    return MutableList.copyOf(env, globList);
   }
 
   /**
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 cef1d1d947..6f1d1ac30b 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
@@ -267,7 +267,7 @@ public class BazelLibrary {
       new BuiltinFunction("to_list") {
         @SuppressWarnings("unused")
         public MutableList<Object> invoke(SkylarkNestedSet input, Environment env) {
-          return new MutableList<>(input.toCollection(), env);
+          return MutableList.copyOf(env, input.toCollection());
         }
       };
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
index 3a457fb760..618913cd68 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
@@ -100,7 +100,7 @@ public final class ListComprehension extends AbstractComprehension {
 
     @Override
     public Object getResult(Environment env) throws EvalException {
-      return new MutableList<>(result, env);
+      return MutableList.copyOf(env, result);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index ffe702686e..bd3a880730 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -396,7 +396,7 @@ public class MethodLibrary {
       result.addFirst(input.substring(0, remainingLength));
     }
 
-    return new MutableList<>(result, env);
+    return MutableList.copyOf(env, result);
   }
 
   @SkylarkSignature(name = "partition", objectType = StringModule.class,
@@ -1134,9 +1134,9 @@ public class MethodLibrary {
         public MutableList<?> invoke(Object self, Location loc, Environment env)
             throws EvalException {
           try {
-            return new MutableList<>(
-                EvalUtils.SKYLARK_COMPARATOR.sortedCopy(EvalUtils.toCollection(self, loc, env)),
-                env);
+            return MutableList.copyOf(
+                env,
+                EvalUtils.SKYLARK_COMPARATOR.sortedCopy(EvalUtils.toCollection(self, loc, env)));
           } catch (EvalUtils.ComparisonException e) {
             throw new EvalException(loc, e);
           }
@@ -1176,7 +1176,7 @@ public class MethodLibrary {
           for (Object element : EvalUtils.toIterable(sequence, loc, env)) {
             tmpList.addFirst(element);
           }
-          return new MutableList<>(tmpList, env);
+          return MutableList.copyOf(env, tmpList);
         }
       };
 
@@ -1500,7 +1500,7 @@ public class MethodLibrary {
   private static final BuiltinFunction values =
       new BuiltinFunction("values") {
         public MutableList<?> invoke(SkylarkDict<?, ?> self, Environment env) throws EvalException {
-          return new MutableList<>(self.values(), env);
+          return MutableList.copyOf(env, self.values());
         }
       };
 
@@ -1523,7 +1523,7 @@ public class MethodLibrary {
           for (Map.Entry<?, ?> entries : self.entrySet()) {
             list.add(Tuple.of(entries.getKey(), entries.getValue()));
           }
-          return new MutableList<>(list, env);
+          return MutableList.copyOf(env, list);
         }
       };
 
@@ -1545,7 +1545,7 @@ public class MethodLibrary {
       for (Map.Entry<?, ?> entries : self.entrySet()) {
         list.add(entries.getKey());
       }
-      return new MutableList(list, env);
+      return MutableList.copyOf(env, list);
     }
   };
 
@@ -1601,7 +1601,7 @@ public class MethodLibrary {
   private static final BuiltinFunction list =
       new BuiltinFunction("list") {
         public MutableList<?> invoke(Object x, Location loc, Environment env) throws EvalException {
-          return new MutableList<>(EvalUtils.toCollection(x, loc, env), env);
+          return MutableList.copyOf(env, EvalUtils.toCollection(x, loc, env));
         }
       };
 
@@ -1872,7 +1872,7 @@ public class MethodLibrary {
             result.add(Tuple.of(count, obj));
             count++;
           }
-          return new MutableList<>(result, env);
+          return MutableList.copyOf(env, result);
         }
       };
 
@@ -1966,7 +1966,7 @@ public class MethodLibrary {
               start += step;
             }
           }
-          return new MutableList<>(result, env);
+          return MutableList.copyOf(env, result);
         }
       };
 
@@ -2078,7 +2078,7 @@ public class MethodLibrary {
           }
           fields.addAll(Runtime.getFunctionNames(object.getClass()));
           fields.addAll(FuncallExpression.getMethodNames(object.getClass()));
-          return new MutableList<>(fields, env);
+          return MutableList.copyOf(env, fields);
         }
       };
 
@@ -2204,7 +2204,7 @@ public class MethodLibrary {
               result.add(Tuple.copyOf(elem));
             }
           } while (allHasNext);
-          return new MutableList<>(result, env);
+          return MutableList.copyOf(env, result);
         }
       };
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
index 24feb126f9..56d078c5fa 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
@@ -287,18 +287,7 @@ public abstract class SkylarkList<E> extends BaseMutableList<E>
     }
 
     private MutableList(Iterable<? extends E> contents, @Nullable Mutability mutability) {
-      this(contents, 0, mutability);
-    }
-
-    /**
-     * Constructs from the given items and the {@link Mutability} belonging to the given {@link
-     * Environment}. If {@code env} is null, the list is immutable.
-     *
-     * @deprecated prefer using {@link #copyOf}
-     */
-    @Deprecated
-    public MutableList(Iterable<? extends E> contents, @Nullable Environment env) {
-      this(contents, 0, env == null ? null : env.mutability());
+      this(contents, /*capacity=*/ 0, mutability);
     }
 
     /**
@@ -333,7 +322,7 @@ public abstract class SkylarkList<E> extends BaseMutableList<E>
      */
     public static <T> MutableList<T> copyOf(
         @Nullable Environment env, Iterable<? extends T> contents) {
-      return new MutableList<>(contents, env.mutability());
+      return new MutableList<>(contents, env == null ? null : env.mutability());
     }
 
     /**
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 c176f3ca96..6d8c1a8667 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
@@ -722,7 +722,7 @@ public abstract class SkylarkType implements Serializable {
    */
   public static Object convertToSkylark(Object object, @Nullable Environment env) {
     if (object instanceof List && !(object instanceof SkylarkList)) {
-      return new MutableList<>((List<?>) object, env);
+      return MutableList.copyOf(env, (List<?>) object);
     }
     if (object instanceof SkylarkValue) {
       return object;
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 12d4ad8846..773627d9c1 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -237,7 +237,7 @@ public class ValidationTest extends EvaluationTestCase {
   public void testGetSkylarkType() throws Exception {
     Class<?> emptyTupleClass = Tuple.empty().getClass();
     Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
-    Class<?> mutableListClass = new MutableList<>(Tuple.of(1, 2, 3), env).getClass();
+    Class<?> mutableListClass = MutableList.copyOf(env, Tuple.of(1, 2, 3)).getClass();
 
     assertThat(EvalUtils.getSkylarkType(mutableListClass)).isEqualTo(MutableList.class);
     assertThat(MutableList.class.isAnnotationPresent(SkylarkModule.class)).isTrue();
@@ -261,7 +261,7 @@ public class ValidationTest extends EvaluationTestCase {
   public void testSkylarkTypeEquivalence() throws Exception {
     Class<?> emptyTupleClass = Tuple.empty().getClass();
     Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
-    Class<?> mutableListClass = new MutableList<>(Tuple.of(1, 2, 3), env).getClass();
+    Class<?> mutableListClass = MutableList.copyOf(env, Tuple.of(1, 2, 3)).getClass();
 
     assertThat(SkylarkType.of(mutableListClass)).isEqualTo(SkylarkType.LIST);
     assertThat(SkylarkType.of(emptyTupleClass)).isEqualTo(SkylarkType.TUPLE);