Rollback of commit d7b64bd03100300b79cd33d04904ce9b0e6a5332.

*** Reason for rollback ***

Some rules write the toString representation of lists of files to disk and then read it back again. This breaks that.

--
MOS_MIGRATED_REVID=104524336

4 files changed, 37 insertions, 259 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 56da7abe9b..c4fd52e737 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -273,7 +273,14 @@ public final class FuncallExpression extends Expression {
       sb.append(obj).append(".");
     }
     sb.append(func);
-    Printer.printList(sb, args, "(", ", ", ")", /* singletonTerminator */ null);
+    String backup = sb.toString();
+    try {
+      Printer.printList(sb, args, "(", ", ", ")", /* singletonTerminator */ null);
+    } catch (OutOfMemoryError ex) {
+      // export_files might lead to an OOM error (e.g. when deserializing very large packages).
+      // TODO(bazel-dev): make the Printer limit its own output.
+      return backup + "(<too long>)";
+    }
     return sb.toString();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index 55e855611c..22dcec8810 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -70,10 +70,33 @@ public final class ListLiteral extends Expression {
     return kind == Kind.TUPLE;
   }
 
+  private static char startChar(Kind kind) {
+    switch(kind) {
+    case LIST:  return '[';
+    case TUPLE: return '(';
+    }
+    return '[';
+  }
+
+  private static char endChar(Kind kind) {
+    switch(kind) {
+    case LIST:  return ']';
+    case TUPLE: return ')';
+    }
+    return ']';
+  }
+
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
-    Printer.printList(sb, exprs, isTuple(), '"');
+    sb.append(startChar(kind));
+    String sep = "";
+    for (Expression e : exprs) {
+      sb.append(sep);
+      sb.append(e);
+      sep = ", ";
+    }
+    sb.append(endChar(kind));
     return sb.toString();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index f607b11253..0fa29641fe 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -13,10 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -30,8 +27,6 @@ import java.util.MissingFormatWidthException;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 /**
  * (Pretty) Printing of Skylark values
@@ -40,19 +35,6 @@ public final class Printer {
 
   private static final char SKYLARK_QUOTATION_MARK = '"';
 
-  /*
-   * Maximum number of list elements that should be printed via printList().
-   */
-  @VisibleForTesting
-  static final int CRITICAL_LIST_ELEMENTS_COUNT = 10;
-
-  /*
-   * printList() will start to shorten the values of list elements when their string length reaches
-   * this value.
-   */
-  @VisibleForTesting
-  static final int CRITICAL_LIST_ELEMENTS_STRING_LENGTH = 128;
-
   private Printer() {
   }
 
@@ -275,54 +257,21 @@ public final class Printer {
       String after,
       String singletonTerminator,
       char quotationMark) {
-    return printList(buffer, list, before, separator, after, singletonTerminator, quotationMark,
-        CRITICAL_LIST_ELEMENTS_COUNT, CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
-  }
-
-  private static Appendable printList(
-      Appendable buffer,
-      Iterable<?> list,
-      String before,
-      String separator,
-      String after,
-      String singletonTerminator,
-      char quotationMark,
-      int maxItemsToPrint,
-      int criticalItemsStringLength) {
     boolean printSeparator = false; // don't print the separator before the first element
-    boolean skipArgs = false;
-    int items = Iterables.size(list);
     int len = 0;
-    // We don't want to print "1 more arguments", hence we don't skip arguments if there is only one
-    // above the limit.
-    int itemsToPrint = (items - maxItemsToPrint == 1) ? items : maxItemsToPrint;
-
-    LengthLimitedAppendable appendable =
-        LengthLimitedAppendable.create(buffer, criticalItemsStringLength);
-    append(appendable, before);
-    appendable.enforceLimit();
+    append(buffer, before);
     for (Object o : list) {
-      // We don't want to print "1 more arguments", even if we hit the string limit.
-      if (len == itemsToPrint || (appendable.hasHitLimit() && len < items - 1)) {
-        skipArgs = true;
-        break;
-      }
       if (printSeparator) {
-        append(appendable, separator);
+        append(buffer, separator);
       }
-      write(appendable, o, quotationMark);
+      write(buffer, o, quotationMark);
       printSeparator = true;
       len++;
     }
-    appendable.ignoreLimit();
-    if (skipArgs) {
-      append(appendable, separator);
-      append(appendable, String.format("<%d more arguments>", items - len));
-    }
     if (singletonTerminator != null && len == 1) {
-      append(appendable, singletonTerminator);
+      append(buffer, singletonTerminator);
     }
-    return append(appendable, after);
+    return append(buffer, after);
   }
 
   public static Appendable printList(Appendable buffer, Iterable<?> list, String before,
@@ -494,134 +443,4 @@ public final class Printer {
     }
     return buffer;
   }
-
-  /**
-   * Helper class for {@code Appendable}s that want to limit the length of their input.
-   *
-   * <p>Instances of this class act as a proxy for one {@code Appendable} object and decide whether
-   * the given input (or parts of it) can be written to the underlying {@code Appendable}, depending
-   * on whether the specified maximum length has been met or not.
-   */
-  private static final class LengthLimitedAppendable implements Appendable {
-
-    private static final ImmutableSet<Character> SPECIAL_CHARS =
-        ImmutableSet.of(',', ' ', '"', '\'', ':', '(', ')', '[', ']', '{', '}');
-
-    private static final Pattern ARGS_PATTERN = Pattern.compile("<\\d+ more arguments>");
-
-    private final Appendable original;
-    private int limit;
-    private boolean ignoreLimit;
-    private boolean previouslyShortened;
-    
-    private LengthLimitedAppendable(Appendable original, int limit) {
-      this.original = original;
-      this.limit = limit;
-    }
-
-    public static LengthLimitedAppendable create(Appendable original, int limit) {
-      // We don't want to overwrite the limit if original is already an instance of this class.
-      return (original instanceof LengthLimitedAppendable)
-          ? (LengthLimitedAppendable) original : new LengthLimitedAppendable(original, limit);
-    }
-
-    @Override
-    public Appendable append(CharSequence csq) throws IOException {
-      if (ignoreLimit || hasOnlySpecialChars(csq)) {
-        // Don't update limit.
-        original.append(csq);
-        previouslyShortened = false;
-      } else {
-        int length = csq.length();
-        if (length <= limit) {
-          limit -= length;
-          original.append(csq);
-        } else {
-          original.append(csq, 0, limit);
-          // We don't want to append multiple ellipses.
-          if (!previouslyShortened) {
-            original.append("...");
-          }
-          appendTrailingSpecialChars(csq, limit);
-          previouslyShortened = true;
-          limit = 0;
-        }
-      }
-      return this;
-    }
-
-    /**
-     * Appends any trailing "special characters" (e.g. brackets, quotation marks) in the given
-     * sequence to the output buffer, regardless of the limit.
-     *
-     * <p>For example, let's look at foo(['too long']). Without this method, the shortened result
-     * would be foo(['too...) instead of the prettier foo(['too...']).
-     *
-     * <p>If the input string was already shortened and contains "<x more arguments>", this part
-     * will also be appended.
-     */
-    private void appendTrailingSpecialChars(CharSequence csq, int limit) throws IOException {
-      int length = csq.length();
-      Matcher matcher = ARGS_PATTERN.matcher(csq);
-      // We assume that everything following the "x more arguments" part has to be copied, too.
-      int start = matcher.find() ? matcher.start() : length;
-      // Find the left-most non-arg char that has to be copied.
-      for (int i = start - 1; i > limit; --i) {
-        if (isSpecialChar(csq.charAt(i))) {
-          start = i;
-        } else {
-          break;
-        }
-      }
-      if (start < length) {
-        original.append(csq, start, csq.length());
-      }
-    }
-
-    /**
-     * Returns whether the given sequence denotes characters that are not part of the value of an
-     * argument.
-     *
-     * <p>Examples are brackets, braces and quotation marks.
-     */
-    private boolean hasOnlySpecialChars(CharSequence csq) {
-      for (int i = 0; i < csq.length(); ++i) {
-        if (!isSpecialChar(csq.charAt(i))) {
-          return false;
-        }
-      }
-      return true;
-    }
-
-    private boolean isSpecialChar(char c)    {
-      return SPECIAL_CHARS.contains(c);
-    }
-
-    @Override
-    public Appendable append(CharSequence csq, int start, int end) throws IOException {
-      return append(csq.subSequence(start, end));
-    }
-
-    @Override
-    public Appendable append(char c) throws IOException {
-      return append(String.valueOf(c));
-    }
-    
-    public boolean hasHitLimit()  {
-      return limit <= 0;
-    }
-
-    public void enforceLimit()  {
-      ignoreLimit = false;
-    }
-    
-    public void ignoreLimit() {
-      ignoreLimit = true;
-    }
-
-    @Override
-    public String toString() {
-      return original.toString();
-    }
-  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
index 92e3d5ec0f..925503e050 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
 
@@ -29,7 +28,6 @@ import org.junit.runners.JUnit4;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.IllegalFormatException;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
@@ -168,73 +166,4 @@ public class PrinterTest {
     assertThat(Printer.repr(dict, '\''))
         .isEqualTo("{1: ('foo', 'bar'), 2: ['foo', 'bar'], 'foo': []}");
   }
-
-  @Test
-  public void testListLimitStringLength() throws Exception {
-    String limit = Strings.repeat("x", Printer.CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
-    String half = Strings.repeat("x", Printer.CRITICAL_LIST_ELEMENTS_STRING_LENGTH / 2);
-
-    List<String> list = Arrays.asList(limit + limit);
-
-    // String is way too long -> shorten.
-    assertThat(Printer.str(list)).isEqualTo("[\"" + limit + "...\"]");
-
-    LinkedList<List<String>> nestedList = new LinkedList<>();
-    nestedList.add(list);
-
-    // Same as above, but with one additional level of indirection.
-    assertThat(Printer.str(nestedList)).isEqualTo("[[\"" + limit + "...\"]]");
-
-    // The inner list alone would meet the limit, but because of the first element, it has to be
-    // shortened.
-    assertThat(Printer.str(Arrays.asList(half, Arrays.asList(limit))))
-        .isEqualTo("[\"" + half + "\", [\"" + half + "...\"]]");
-
-    // String is too long, but the ellipsis make it even longer.
-    assertThat(Printer.str(Arrays.asList(limit + "x"))).isEqualTo("[\"" + limit + "...\"]");
-
-    // We hit the limit exactly -> everything is printed.
-    assertThat(Printer.str(Arrays.asList(limit))).isEqualTo("[\"" + limit + "\"]");
-
-    // Exact hit, but with two arguments -> everything is printed.
-    assertThat(Printer.str(Arrays.asList(half, half)))
-        .isEqualTo("[\"" + half + "\", \"" + half + "\"]");
-
-    // First argument hits the limit -> remaining argument is shortened.
-    assertThat(Printer.str(Arrays.asList(limit, limit))).isEqualTo("[\"" + limit + "\", \"...\"]");
-
-    String limitMinusOne = limit.substring(0, limit.length() - 1);
-
-    // First arguments is one below the limit -> print first character of remaining argument.
-    assertThat(Printer.str(Arrays.asList(limitMinusOne, limit)))
-        .isEqualTo("[\"" + limitMinusOne + "\", \"x...\"]");
-
-    // First argument hits the limit -> we skip  the remaining two arguments.
-    assertThat(Printer.str(Arrays.asList(limit, limit, limit)))
-        .isEqualTo("[\"" + limit + "\", <2 more arguments>]");
-  }
-
-  @Test
-  public void testListLimitTooManyArgs() throws Exception {
-    StringBuilder builder = new StringBuilder();
-    List<Integer> maxLength = new LinkedList<>();
-
-    int next;
-    for (next = 0; next < Printer.CRITICAL_LIST_ELEMENTS_COUNT; ++next) {
-      maxLength.add(next);
-      if (next > 0) {
-        builder.append(", ");
-      }
-      builder.append(next);
-    }
-
-    // There is one too many, but we print every argument nonetheless.
-    maxLength.add(next);
-    assertThat(Printer.str(maxLength)).isEqualTo("[" + builder + ", " + next + "]");
-
-    // There are two too many, hence we don't print them.
-    ++next;
-    maxLength.add(next);
-    assertThat(Printer.str(maxLength)).isEqualTo("[" + builder + ", <2 more arguments>]");
-  }
 }