Refactor Printer

It's now easier to customize Printer if in different situations objects should
be printed differently. Also its API is cleaner now. Names of methods of SkylarkValue objects now reflect names of Skylark functions: SkylarkValue#repr and SkylarkPrintableValue#str.

PiperOrigin-RevId: 160635154

5 files changed, 42 insertions, 52 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
index 97a38b25e8..de6d81a257 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
@@ -530,12 +530,6 @@ public class BuildTypeTest {
   }
 
   @Test
-  public void testSingleQuotes() throws Exception {
-    assertThat(Printer.repr(createTestFilesetEntry(), '\''))
-        .isEqualTo(createExpectedFilesetEntryString('\''));
-  }
-
-  @Test
   public void testFilesetEntrySymlinkAttr() throws Exception {
     FilesetEntry entryDereference =
       createTestFilesetEntry(FilesetEntry.SymlinkBehavior.DEREFERENCE);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index b4334436a9..888047d871 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -2728,7 +2728,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
     assertThat(getConfiguredTarget("//java/com/foo")).isNull();
     assertContainsEvent(
         "in config_feature_flag rule //java/com/foo:flag1: "
-            + "value must be one of ['off', 'on'], but was 'invalid'");
+            + "value must be one of [\"off\", \"on\"], but was \"invalid\"");
   }
 
   @Test
@@ -2757,7 +2757,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
     assertThat(getConfiguredTarget("//java/com/foo")).isNull();
     assertContainsEvent(
         "in config_feature_flag rule //java/com/foo:flag1: "
-            + "value must be one of ['off', 'on'], but was 'invalid'");
+            + "value must be one of [\"off\", \"on\"], but was \"invalid\"");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
index b649872487..0d87489daf 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
@@ -237,7 +237,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
     assertThat(getConfiguredTarget("//test:flag")).isNull();
     assertContainsEvent(
         "in allowed_values attribute of config_feature_flag rule //test:flag: "
-            + "cannot contain duplicates, but contained multiple of ['double']");
+            + "cannot contain duplicates, but contained multiple of [\"double\"]");
   }
 
   @Test
@@ -260,7 +260,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
     assertThat(getConfiguredTarget("//test:top")).isNull();
     assertContainsEvent(
         "in default_value attribute of config_feature_flag rule //test:flag: "
-            + "must be one of ['eagle', 'legal'], but was 'beagle'");
+            + "must be one of [\"eagle\", \"legal\"], but was \"beagle\"");
   }
 
   @Test
@@ -284,7 +284,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
     // TODO(mstaib): when configurationError is implemented, switch to testing for that
     assertContainsEvent(
         "in config_feature_flag rule //test:flag: "
-            + "value must be one of ['configured', 'default', 'other'], but was 'invalid'");
+            + "value must be one of [\"configured\", \"default\", \"other\"], but was \"invalid\"");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index afaf9aaab7..2ae31358cb 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -16,6 +16,8 @@ package com.google.devtools.build.lib.syntax;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
@@ -583,30 +585,54 @@ public class EvaluationTest extends EvaluationTestCase {
     newTest().testStatement("not 'a' in ['a'] or 0", 0);
   }
 
-  private Object createObjWithStr() {
+  private SkylarkValue createObjWithStr() {
+    return new SkylarkValue() {
+      @Override
+      public void repr(SkylarkPrinter printer) {
+        printer.append("str marker");
+      }
+
+      @Override
+      public boolean isImmutable() {
+        return false;
+      }
+    };
+  }
+
+  private Object createUnknownObj() {
     return new Object() {
       @Override
       public String toString() {
-        return "str marker";
+        return "unknown object";
       }
     };
   }
 
   @Test
   public void testPercOnObject() throws Exception {
-    newTest().update("obj", createObjWithStr()).testStatement("'%s' % obj", "str marker");
+    newTest()
+        .update("obj", createObjWithStr())
+        .testStatement("'%s' % obj", "str marker");
+    newTest()
+        .update("unknown", createUnknownObj())
+        .testStatement("'%s' % unknown", "unknown object");
   }
 
   @Test
   public void testPercOnObjectList() throws Exception {
-    newTest().update("obj", createObjWithStr()).testStatement("'%s %s' % (obj, obj)",
-        "str marker str marker");
+    newTest()
+        .update("obj", createObjWithStr())
+        .testStatement("'%s %s' % (obj, obj)", "str marker str marker");
+    newTest()
+        .update("unknown", createUnknownObj())
+        .testStatement("'%s %s' % (unknown, unknown)", "unknown object unknown object");
   }
 
   @Test
   public void testPercOnObjectInvalidFormat() throws Exception {
-    newTest().update("obj", createObjWithStr()).testIfExactError(
-        "invalid argument str marker for format pattern %d", "'%d' % obj");
+    newTest()
+        .update("obj", createObjWithStr())
+        .testIfExactError("invalid argument str marker for format pattern %d", "'%d' % obj");
   }
 
   @Test
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 0a7f8410a8..fdf43271bb 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
@@ -98,12 +98,12 @@ public class PrinterTest {
 
   @Test
   public void testFormatPositional() throws Exception {
-    assertThat(Printer.formatToString("%s %d", Tuple.of("foo", 3))).isEqualTo("foo 3");
+    assertThat(Printer.formatWithList("%s %d", Tuple.of("foo", 3))).isEqualTo("foo 3");
     assertThat(Printer.format("%s %d", "foo", 3)).isEqualTo("foo 3");
 
     // Note: formatToString doesn't perform scalar x -> (x) conversion;
     // The %-operator is responsible for that.
-    assertThat(Printer.formatToString("", Tuple.of())).isEmpty();
+    assertThat(Printer.formatWithList("", Tuple.of())).isEmpty();
     assertThat(Printer.format("%s", "foo")).isEqualTo("foo");
     assertThat(Printer.format("%s", 3.14159)).isEqualTo("3.14159");
     checkFormatPositionalFails("not all arguments converted during string formatting",
@@ -131,34 +131,6 @@ public class PrinterTest {
   }
 
   @Test
-  public void testSingleQuotes() throws Exception {
-    assertThat(Printer.str("test", '\'')).isEqualTo("test");
-    assertThat(Printer.repr("test", '\'')).isEqualTo("'test'");
-
-    assertThat(Printer.repr("'", '\'')).isEqualTo("'\\''");
-    assertThat(Printer.str("\"", '\'')).isEqualTo("\"");
-    assertThat(Printer.repr("\"", '\'')).isEqualTo("'\"'");
-
-    List<?> list = MutableList.of(null, "foo", "bar");
-    List<?> tuple = Tuple.of("foo", "bar");
-
-    assertThat(Printer.str(Tuple.of(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
-    assertThat(Printer.repr(Tuple.of(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
-    assertThat(Printer.str(MutableList.of(null, 1, tuple, 3), '\''))
-        .isEqualTo("[1, ('foo', 'bar'), 3]");
-    assertThat(Printer.repr(MutableList.of(null, 1, tuple, 3), '\''))
-        .isEqualTo("[1, ('foo', 'bar'), 3]");
-
-    Map<Object, Object> dict =
-        ImmutableMap.<Object, Object>of(1, tuple, 2, list, "foo", MutableList.of(null));
-
-    assertThat(Printer.str(dict, '\''))
-        .isEqualTo("{1: ('foo', 'bar'), 2: ['foo', 'bar'], 'foo': []}");
-    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) {
@@ -249,9 +221,7 @@ public class PrinterTest {
   }
 
   private String printList(List<?> list, int criticalElementsCount, int criticalStringLength) {
-    StringBuilder builder = new StringBuilder();
-    Printer.printList(
-        builder, list, "[", ", ", "]", "", '"', criticalElementsCount, criticalStringLength);
-    return builder.toString();
+    return Printer.printAbbreviatedList(
+        list, "[", ", ", "]", "", criticalElementsCount, criticalStringLength);
   }
 }