[Skylark] Control whether range type is used with --incompatible_rang…

…e_type.

src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java

@@ -54,6 +54,7 @@ public void serialize(
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowSlashOperator());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
     codedOut.writeBoolNoTag(semantics.incompatiblePackageNameIsAFunction());
+    codedOut.writeBoolNoTag(semantics.incompatibleRangeType());
     codedOut.writeBoolNoTag(semantics.incompatibleRemoveNativeGitRepository());
     codedOut.writeBoolNoTag(semantics.incompatibleRemoveNativeHttpArchive());
     codedOut.writeBoolNoTag(semantics.incompatibleStringIsNotIterable());
@@ -77,6 +78,7 @@ public SkylarkSemantics deserialize(DeserializationContext context, CodedInputSt
     builder.incompatibleDisallowSlashOperator(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());
     builder.incompatiblePackageNameIsAFunction(codedIn.readBool());
+    builder.incompatibleRangeType(codedIn.readBool());
     builder.incompatibleRemoveNativeGitRepository(codedIn.readBool());
     builder.incompatibleRemoveNativeHttpArchive(codedIn.readBool());
     builder.incompatibleStringIsNotIterable(codedIn.readBool());

src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java

@@ -214,6 +214,19 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatiblePackageNameIsAFunction;
 
+  @Option(
+      name = "incompatible_range_type",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      metadataTags = {
+          OptionMetadataTag.INCOMPATIBLE_CHANGE,
+          OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help = "If set to true, range() will use the 'range' type instead of 'list'."
+  )
+  public boolean incompatibleRangeType;
+
   @Option(
     name = "incompatible_remove_native_git_repository",
     defaultValue = "false",
@@ -283,6 +296,7 @@ public SkylarkSemantics toSkylarkSemantics() {
         .incompatibleDisallowSlashOperator(incompatibleDisallowSlashOperator)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)
         .incompatiblePackageNameIsAFunction(incompatiblePackageNameIsAFunction)
+        .incompatibleRangeType(incompatibleRangeType)
         .incompatibleRemoveNativeGitRepository(incompatibleRemoveNativeGitRepository)
         .incompatibleRemoveNativeHttpArchive(incompatibleRemoveNativeHttpArchive)
         .incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)

src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java

@@ -17,9 +17,7 @@
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.Concatable.Concatter;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
-import com.google.devtools.build.lib.syntax.SkylarkList.MutableListLike;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
-import com.google.devtools.build.lib.syntax.SkylarkList.TupleLike;
 import java.io.IOException;
 import java.util.IllegalFormatException;
 
@@ -308,15 +306,6 @@ private static Object plus(
       }
     }
 
-    if (lval instanceof MutableListLike && rval instanceof MutableListLike) {
-      return MutableList.concat(((MutableListLike<?>) lval).toMutableList(),
-          ((MutableListLike<?>) rval).toMutableList(), env.mutability());
-    }
-
-    if (lval instanceof TupleLike && rval instanceof TupleLike) {
-      return Tuple.concat(((TupleLike<?>) lval).toTuple(), ((TupleLike<?>) rval).toTuple());
-    }
-
     if (lval instanceof SkylarkDict && rval instanceof SkylarkDict) {
       if (env.getSemantics().incompatibleDisallowDictPlus()) {
         throw new EvalException(

src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java

@@ -623,7 +623,7 @@ public Integer invoke(String value) throws EvalException {
 
   @SkylarkSignature(
     name = "range",
-    returnType = RangeList.class,
+    returnType = SkylarkList.class,
     doc =
         "Creates a list where items go from <code>start</code> to <code>stop</code>, using a "
             + "<code>step</code> increment. If a single argument is provided, items will "
@@ -660,7 +660,7 @@ public Integer invoke(String value) throws EvalException {
   )
   private static final BuiltinFunction range =
       new BuiltinFunction("range") {
-        public RangeList invoke(
+        public SkylarkList<Integer> invoke(
             Integer startOrStop, Object stopOrNone, Integer step, Location loc, Environment env)
             throws EvalException {
           int start;
@@ -680,7 +680,8 @@ public RangeList invoke(
             // by turning it into range(0, 0)
             stop = Math.max(start, stop);
           }
-          return RangeList.of(start, stop, step);
+          RangeList range = RangeList.of(start, stop, step);
+          return env.getSemantics().incompatibleRangeType() ? range : range.toMutableList(env);
         }
       };
 

src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java

@@ -120,10 +120,9 @@ public String toString() {
   @Override
   public boolean equals(Object object) {
     return (this == object)
-        || ((object != null) && (this.getClass() == object.getClass()
-        || this instanceof MutableListLike && object instanceof MutableListLike
-        || this instanceof TupleLike && object instanceof TupleLike)
-        && getContentsUnsafe().equals(((SkylarkList) object).getContentsUnsafe()));
+        || ((object != null)
+            && (this.getClass() == object.getClass())
+            && getContentsUnsafe().equals(((SkylarkList) object).getContentsUnsafe()));
   }
 
   @Override
@@ -207,25 +206,32 @@ public static <E> SkylarkList<E> createImmutable(Iterable<? extends E> contents)
     return MutableList.copyOf(Mutability.IMMUTABLE, contents);
   }
 
-  /** An interface for classes that can be converted to {@link MutableList}. */
-  public interface MutableListLike<E> {
-    MutableList<E> toMutableList();
-  }
-
-  /** An interface for classes that can be converted to {@link Tuple}. */
-  public interface TupleLike<E> {
-    Tuple<E> toTuple();
-  }
-
   /**
    * A sequence returned by the {@code range} function invocation.
    *
-   * Instead of eagerly allocating an array with all elements of the sequence, this class uses
-   * simple math to compute a value at each index. This is particularly useful when range is huge
-   * or only a few elements from it are used.
+   * <p>Instead of eagerly allocating an array with all elements of the sequence, this class uses
+   * simple math to compute a value at each index. This is particularly useful when range is huge or
+   * only a few elements from it are used.
+   *
+   * <p>Eventually {@code range} function should produce an instance of the {@code range} type as is
+   * the case in Python 3, but for now to preserve backwards compatibility with Python 2, {@code
+   * list} is returned.
    */
-  public static final class RangeList extends SkylarkList<Integer> implements
-      MutableListLike<Integer>, TupleLike<Integer> {
+  @SkylarkModule(
+      name = "range",
+      category = SkylarkModuleCategory.BUILTIN,
+      doc =
+          "A language built-in type to support ranges. Example of range literal:<br>"
+              + "<pre class=language-python>x = range(1, 10, 3)</pre>"
+              + "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+              + "<pre class=language-python>e = x[1]   # e == 2</pre>"
+              + "Ranges do not support the <code>+</code> operator for concatenation."
+              + "Similar to strings, ranges support slice operations:"
+              + "<pre class=language-python>range(10)[1:3]   # range(1, 3)\n"
+              + "range(10)[::2]  # range(0, 10, 2)\n"
+              + "range(10)[3:0:-1]  # range(3, 0, -1)</pre>"
+              + "Ranges are immutable, as in Python 3.")
+  public static final class RangeList extends SkylarkList<Integer> {
 
     /** Provides access to range elements based on their index. */
     private static class RangeListView extends AbstractList<Integer> {
@@ -333,9 +339,24 @@ public Iterator<Integer> iterator() {
           return new DecreasingIterator(start, stop, step);
         }
       }
+
+      /** @return the start of the range. */
+      public int getStart() {
+        return start;
+      }
+
+      /** @return the stop element (next afer the last one) of the range. */
+      public int getStop() {
+        return stop;
+      }
+
+      /** @return the step between each element of the range. */
+      public int getStep() {
+        return step;
+      }
     }
 
-    private final AbstractList<Integer> contents;
+    private final RangeListView contents;
 
     private RangeList(int start, int stop, int step) {
       this.contents = new RangeListView(start, stop, step);
@@ -352,11 +373,13 @@ public ImmutableList<Integer> getImmutableList() {
     }
 
     @Override
-    public SkylarkList<Integer> getSlice(Object start, Object end, Object step, Location loc,
-        Mutability mutability) throws EvalException {
+    public SkylarkList<Integer> getSlice(
+        Object start, Object end, Object step, Location loc, Mutability mutability)
+        throws EvalException {
       // TODO: use lazy slice implementation
       List<Integer> sliceIndices = EvalUtils.getSliceIndices(start, end, step, this.size(), loc);
-      ImmutableList.Builder<Integer> builder = ImmutableList.builderWithExpectedSize(sliceIndices.size());
+      ImmutableList.Builder<Integer> builder =
+          ImmutableList.builderWithExpectedSize(sliceIndices.size());
       for (int pos : sliceIndices) {
         builder.add(this.get(pos));
       }
@@ -365,7 +388,8 @@ public SkylarkList<Integer> getSlice(Object start, Object end, Object step, Loca
 
     @Override
     public SkylarkList<Integer> repeat(int times, Mutability mutability) {
-      ImmutableList.Builder<Integer> builder = ImmutableList.builderWithExpectedSize(this.size() * times);
+      ImmutableList.Builder<Integer> builder =
+          ImmutableList.builderWithExpectedSize(this.size() * times);
       for (int i = 0; i < times; i++) {
         builder.addAll(this);
       }
@@ -383,13 +407,17 @@ public Mutability mutability() {
     }
 
     @Override
-    public MutableList<Integer> toMutableList() {
-      return MutableList.copyOf(Mutability.IMMUTABLE, contents);
+    public void repr(SkylarkPrinter printer) {
+      if (contents.getStep() == 1) {
+        printer.format("range(%d, %d)", contents.getStart(), contents.getStop());
+      } else {
+        printer.format(
+            "range(%d, %d, %d)", contents.getStart(), contents.getStop(), contents.getStep());
+      }
     }
 
-    @Override
-    public Tuple<Integer> toTuple() {
-      return Tuple.copyOf(contents);
+    public MutableList<Integer> toMutableList(Environment env) {
+      return MutableList.copyOf(env, contents);
     }
 
     public static RangeList of(int start, int stop, int step) {
@@ -419,7 +447,7 @@ public static RangeList of(int start, int stop, int step) {
             + "['a', 'b', 'c', 'd'][3:0:-1]  # ['d', 'c', 'b']</pre>"
             + "Lists are mutable, as in Python."
   )
-  public static final class MutableList<E> extends SkylarkList<E> implements MutableListLike<E> {
+  public static final class MutableList<E> extends SkylarkList<E> {
 
     private final ArrayList<E> contents;
 
@@ -762,11 +790,6 @@ public Object pop(Object i, Location loc, Environment env)
       remove(index, loc, env.mutability());
       return result;
     }
-
-    @Override
-    public MutableList<E> toMutableList() {
-      return this;
-    }
   }
 
   /**
@@ -791,7 +814,7 @@ public MutableList<E> toMutableList() {
             + "('a', 'b', 'c', 'd')[3:0:-1]  # ('d', 'c', 'b')</pre>"
             + "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported."
   )
-  public static final class Tuple<E> extends SkylarkList<E> implements TupleLike<E> {
+  public static final class Tuple<E> extends SkylarkList<E> {
 
     private final ImmutableList<E> contents;
 
@@ -900,10 +923,5 @@ public Tuple<E> repeat(int times, Mutability mutability) {
       }
       return copyOf(builder.build());
     }
-
-    @Override
-    public Tuple<E> toTuple() {
-      return this;
-    }
   }
 }

src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java

@@ -61,6 +61,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean incompatiblePackageNameIsAFunction();
 
+  public abstract boolean incompatibleRangeType();
+
   public abstract boolean incompatibleRemoveNativeGitRepository();
 
   public abstract boolean incompatibleRemoveNativeHttpArchive();
@@ -95,6 +97,7 @@ public static Builder builderWithDefaults() {
           .incompatibleDisallowSlashOperator(false)
           .incompatibleNewActionsApi(false)
           .incompatiblePackageNameIsAFunction(false)
+          .incompatibleRangeType(false)
           .incompatibleRemoveNativeGitRepository(false)
           .incompatibleRemoveNativeHttpArchive(false)
           .incompatibleStringIsNotIterable(false)
@@ -128,6 +131,8 @@ public abstract static class Builder {
 
     public abstract Builder incompatiblePackageNameIsAFunction(boolean value);
 
+    public abstract Builder incompatibleRangeType(boolean value);
+
     public abstract Builder incompatibleRemoveNativeGitRepository(boolean value);
 
     public abstract Builder incompatibleRemoveNativeHttpArchive(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -130,6 +130,7 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
         "--incompatible_disallow_slash_operator=" + rand.nextBoolean(),
         "--incompatible_new_actions_api=" + rand.nextBoolean(),
         "--incompatible_package_name_is_a_function=" + rand.nextBoolean(),
+        "--incompatible_range_type=" + rand.nextBoolean(),
         "--incompatible_remove_native_git_repository=" + rand.nextBoolean(),
         "--incompatible_remove_native_http_archive=" + rand.nextBoolean(),
         "--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
@@ -154,6 +155,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisallowSlashOperator(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())
         .incompatiblePackageNameIsAFunction(rand.nextBoolean())
+        .incompatibleRangeType(rand.nextBoolean())
         .incompatibleRemoveNativeGitRepository(rand.nextBoolean())
         .incompatibleRemoveNativeHttpArchive(rand.nextBoolean())
         .incompatibleStringIsNotIterable(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java

@@ -507,6 +507,44 @@ public void testRangeIsList() throws Exception {
     runRangeIsListAssertions("range(4)[:3]");
   }
 
+  @Test
+  public void testRangeType() throws Exception {
+    new BothModesTest("--incompatible_range_type=true")
+        .setUp("a = range(3)")
+        .testStatement("len(a)", 3)
+        .testStatement("str(a)", "range(0, 3)")
+        .testStatement("str(range(1,2,3))", "range(1, 2, 3)")
+        .testStatement("repr(a)", "range(0, 3)")
+        .testStatement("repr(range(1,2,3))", "range(1, 2, 3)")
+        .testStatement("type(a)", "range")
+        .testIfErrorContains("unsupported operand type(s) for +: 'range' and 'range'", "a + a")
+        .testIfErrorContains("type 'range' has no method append(int)", "a.append(3)")
+        .testStatement("str(list(range(5)))", "[0, 1, 2, 3, 4]")
+        .testStatement("str(list(range(0)))", "[]")
+        .testStatement("str(list(range(1)))", "[0]")
+        .testStatement("str(list(range(-2)))", "[]")
+        .testStatement("str(list(range(-3, 2)))", "[-3, -2, -1, 0, 1]")
+        .testStatement("str(list(range(3, 2)))", "[]")
+        .testStatement("str(list(range(3, 3)))", "[]")
+        .testStatement("str(list(range(3, 4)))", "[3]")
+        .testStatement("str(list(range(3, 5)))", "[3, 4]")
+        .testStatement("str(list(range(-3, 5, 2)))", "[-3, -1, 1, 3]")
+        .testStatement("str(list(range(-3, 6, 2)))", "[-3, -1, 1, 3, 5]")
+        .testStatement("str(list(range(5, 0, -1)))", "[5, 4, 3, 2, 1]")
+        .testStatement("str(list(range(5, 0, -10)))", "[5]")
+        .testStatement("str(list(range(0, -3, -2)))", "[0, -2]")
+        .testStatement("str(range(5)[1:])", "[1, 2, 3, 4]")
+        .testStatement("len(range(5)[1:])", 4)
+        .testStatement("str(range(5)[:2])", "[0, 1]")
+        .testStatement("str(range(10)[1:9:2])", "[1, 3, 5, 7]")
+        .testStatement("str(range(10)[1:10:2])", "[1, 3, 5, 7, 9]")
+        .testStatement("str(range(10)[1:11:2])", "[1, 3, 5, 7, 9]")
+        .testStatement("str(range(0, 10, 2)[::2])", "[0, 4, 8]")
+        .testStatement("str(range(0, 10, 2)[::-2])", "[8, 4, 0]")
+        .testIfErrorContains("step cannot be 0", "range(2, 3, 0)");
+    ;
+  }
+
   /**
    * Helper function for testRangeIsList that expects a range or range slice expression producing
    * the range value containing [0, 1, 2].