Optionally limits the length of the output of Printer.printList() (default = no).

--
MOS_MIGRATED_REVID=104655508

4 files changed, 359 insertions, 42 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 c4fd52e737..c1382ca0cd 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,14 +273,9 @@ public final class FuncallExpression extends Expression {
       sb.append(obj).append(".");
     }
     sb.append(func);
-    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>)";
-    }
+    Printer.printList(sb, args, "(", ", ", ")", /* singletonTerminator */ null,
+        Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
+        Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
     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 22dcec8810..cf620bd95f 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,33 +70,11 @@ 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();
-    sb.append(startChar(kind));
-    String sep = "";
-    for (Expression e : exprs) {
-      sb.append(sep);
-      sb.append(e);
-      sep = ", ";
-    }
-    sb.append(endChar(kind));
+    Printer.printList(sb, exprs, isTuple(), '"', Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
+        Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
     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 0fa29641fe..26e6fdabd6 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
@@ -14,6 +14,8 @@
 package com.google.devtools.build.lib.syntax;
 
 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;
 
@@ -27,6 +29,8 @@ 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
@@ -35,6 +39,19 @@ public final class Printer {
 
   private static final char SKYLARK_QUOTATION_MARK = '"';
 
+  /*
+   * Suggested maximum number of list elements that should be printed via printList().
+   * By default, this setting is not considered and no limitation takes place.
+   */
+  static final int SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT = 4;
+
+  /*
+   * Suggested limit for printList() to shorten the values of list elements when their combined
+   * string length reaches this value.
+   * By default, this setting is not considered and no limitation takes place.
+   */
+  static final int SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH = 32;
+
   private Printer() {
   }
 
@@ -257,21 +274,108 @@ public final class Printer {
       String after,
       String singletonTerminator,
       char quotationMark) {
+    return printList(
+        buffer, list, before, separator, after, singletonTerminator, quotationMark, -1, -1);
+  }
+
+  /**
+   * Print a list of object representations.
+   *
+   * <p>The length of the output will be limited when both {@code maxItemsToPrint} and {@code
+   * criticalItemsStringLength} have values greater than zero.
+   *
+   * @param buffer an appendable buffer onto which to write the list.
+   * @param list the list of objects to write (each as with repr)
+   * @param before a string to print before the list
+   * @param separator a separator to print between each object
+   * @param after a string to print after the list
+   * @param singletonTerminator null or a string to print after the list if it is a singleton
+   * The singleton case is notably relied upon in python syntax to distinguish
+   *    a tuple of size one such as ("foo",) from a merely parenthesized object such as ("foo").
+   * @param quotationMark The quotation mark to be used (' or ")
+   * @param maxItemsToPrint the maximum number of elements to be printed.
+   * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
+   *    'Soft' means that this limit may be exceeded because of formatting.
+   * @return the Appendable, in fluent style.
+   */
+  public static Appendable printList(Appendable buffer, Iterable<?> list, String before,
+      String separator, String after, String singletonTerminator, char quotationMark,
+      int maxItemsToPrint, int criticalItemsStringLength) {
+    append(buffer, before);
+    int len = 0;
+    // Limits the total length of the string representation of the elements, if specified.
+    if (maxItemsToPrint > 0 && criticalItemsStringLength > 0) {
+      len = appendListElements(LengthLimitedAppendable.create(buffer, criticalItemsStringLength),
+          list, separator, quotationMark, maxItemsToPrint);
+    } else {
+      len = appendListElements(buffer, list, separator, quotationMark);
+    }
+    if (singletonTerminator != null && len == 1) {
+      append(buffer, singletonTerminator);
+    }
+    return append(buffer, after);
+  }
+
+  public static Appendable printList(Appendable buffer, Iterable<?> list, String before,
+      String separator, String after, String singletonTerminator, int maxItemsToPrint,
+      int criticalItemsStringLength) {
+    return printList(buffer, list, before, separator, after, singletonTerminator,
+        SKYLARK_QUOTATION_MARK, maxItemsToPrint, criticalItemsStringLength);
+  }
+
+  /**
+   * Appends the given elements to the specified {@link Appendable} and returns the number of
+   * elements.
+   */
+  private static int appendListElements(
+      Appendable appendable, Iterable<?> list, String separator, char quotationMark) {
     boolean printSeparator = false; // don't print the separator before the first element
     int len = 0;
-    append(buffer, before);
     for (Object o : list) {
       if (printSeparator) {
-        append(buffer, separator);
+        append(appendable, separator);
       }
-      write(buffer, o, quotationMark);
+      write(appendable, o, quotationMark);
       printSeparator = true;
       len++;
     }
-    if (singletonTerminator != null && len == 1) {
-      append(buffer, singletonTerminator);
+    return len;
+  }
+
+  /**
+   * Tries to append the given elements to the specified {@link Appendable} until specific limits
+   * are reached.
+   * @return the number of appended elements.
+   */
+  private static int appendListElements(LengthLimitedAppendable appendable, Iterable<?> list,
+      String separator, char quotationMark, int maxItemsToPrint) {
+    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;
+    appendable.enforceLimit();
+    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);
+      }
+      write(appendable, o, quotationMark);
+      printSeparator = true;
+      len++;
     }
-    return append(buffer, after);
+    appendable.ignoreLimit();
+    if (skipArgs) {
+      append(appendable, separator);
+      append(appendable, String.format("<%d more arguments>", items - len));
+    }
+    return len;
   }
 
   public static Appendable printList(Appendable buffer, Iterable<?> list, String before,
@@ -286,17 +390,27 @@ public final class Printer {
    * @param list the contents of the list or tuple
    * @param isTuple is it a tuple or a list?
    * @param quotationMark The quotation mark to be used (' or ")
+   * @param maxItemsToPrint the maximum number of elements to be printed.
+   * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
+   * 'Soft' means that this limit may be exceeded because of formatting.
    * @return the Appendable, in fluent style.
    */
-  public static Appendable printList(
-      Appendable buffer, Iterable<?> list, boolean isTuple, char quotationMark) {
+  public static Appendable printList(Appendable buffer, Iterable<?> list, boolean isTuple,
+      char quotationMark, int maxItemsToPrint, int criticalItemsStringLength) {
     if (isTuple) {
-      return printList(buffer, list, "(", ", ", ")", ",", quotationMark);
+      return printList(buffer, list, "(", ", ", ")", ",", quotationMark, maxItemsToPrint,
+          criticalItemsStringLength);
     } else {
-      return printList(buffer, list, "[", ", ", "]", null, quotationMark);
+      return printList(buffer, list, "[", ", ", "]", null, quotationMark, maxItemsToPrint,
+          criticalItemsStringLength);
     }
   }
 
+  public static Appendable printList(
+      Appendable buffer, Iterable<?> list, boolean isTuple, char quotationMark) {
+    return printList(buffer, list, isTuple, quotationMark, -1, -1);
+  }
+
   /**
    * Print a list of object representations
    * @param list the list of objects to write (each as with repr)
@@ -443,4 +557,134 @@ 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 925503e050..19bd520b6f 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,6 +18,8 @@ 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.Joiner;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
 
@@ -28,6 +30,7 @@ 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;
 
@@ -166,4 +169,101 @@ public class PrinterTest {
     assertThat(Printer.repr(dict, '\''))
         .isEqualTo("{1: ('foo', 'bar'), 2: ['foo', 'bar'], 'foo': []}");
   }
+
+  @Test
+  public void testListLimitStringLength() throws Exception {
+    int lengthDivisibleByTwo = Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH;
+    if (lengthDivisibleByTwo % 2 == 1) {
+      ++lengthDivisibleByTwo;
+    }
+    String limit = Strings.repeat("x", lengthDivisibleByTwo);
+    String half = Strings.repeat("x", lengthDivisibleByTwo / 2);
+
+    List<String> list = Arrays.asList(limit + limit);
+
+    // String is way too long -> shorten.
+    assertThat(printListWithLimit(list)).isEqualTo("[\"" + limit + "...\"]");
+
+    LinkedList<List<String>> nestedList = new LinkedList<>();
+    nestedList.add(list);
+
+    // Same as above, but with one additional level of indirection.
+    assertThat(printListWithLimit(nestedList)).isEqualTo("[[\"" + limit + "...\"]]");
+
+    // The inner list alone would meet the limit, but because of the first element, it has to be
+    // shortened.
+    assertThat(printListWithLimit(Arrays.asList(half, Arrays.asList(limit))))
+        .isEqualTo("[\"" + half + "\", [\"" + half + "...\"]]");
+
+    // String is too long, but the ellipsis make it even longer.
+    assertThat(printListWithLimit(Arrays.asList(limit + "x"))).isEqualTo("[\"" + limit + "...\"]");
+
+    // We hit the limit exactly -> everything is printed.
+    assertThat(printListWithLimit(Arrays.asList(limit))).isEqualTo("[\"" + limit + "\"]");
+
+    // Exact hit, but with two arguments -> everything is printed.
+    assertThat(printListWithLimit(Arrays.asList(half, half)))
+        .isEqualTo("[\"" + half + "\", \"" + half + "\"]");
+
+    // First argument hits the limit -> remaining argument is shortened.
+    assertThat(printListWithLimit(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(printListWithLimit(Arrays.asList(limitMinusOne, limit)))
+        .isEqualTo("[\"" + limitMinusOne + "\", \"x...\"]");
+
+    // First argument hits the limit -> we skip  the remaining two arguments.
+    assertThat(printListWithLimit(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.SUGGESTED_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(printListWithLimit(maxLength)).isEqualTo("[" + builder + ", " + next + "]");
+
+    // There are two too many, hence we don't print them.
+    ++next;
+    maxLength.add(next);
+    assertThat(printListWithLimit(maxLength)).isEqualTo("[" + builder + ", <2 more arguments>]");
+  }
+
+  @Test
+  public void testPrintListDefaultNoLimit() throws Exception {
+    List<Integer> list = new LinkedList<>();
+    // Make sure that the resulting string is longer than the suggestion. This should also lead to
+    // way more items than suggested.
+    for (int i = 0; i < Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH * 2; ++i) {
+      list.add(i);
+    }
+    assertThat(Printer.str(list)).isEqualTo(String.format("[%s]", Joiner.on(", ").join(list)));
+  }
+
+  private String printListWithLimit(List<?> list) {
+    return printList(list, Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
+        Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
+  }
+
+  private String printList(List<?> list, int criticalElementsCount, int criticalStringLength) {
+    StringBuilder builder = new StringBuilder();
+    Printer.printList(
+        builder, list, "[", ", ", "]", "", '"', criticalElementsCount, criticalStringLength);
+    return builder.toString();
+  }
 }