Implement Skylark function repr

Move printing code from EvalUtils to Printer.
Rename functions in Printer:
printValue becomes str or print,
prettyPrintValue becomes repr or write,
formatString becomes format,
makeFormattable becomes strFormattable,
prettyPrintValues becomes listString.

write being self-sufficient is made the reference,
and print is the one that is a wrapper around write,
rather than the other way around, avoiding mutual recursion.

--
MOS_MIGRATED_REVID=95897834

5 files changed, 225 insertions, 171 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
index acd9fb03e4..da709a002b 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
@@ -142,15 +142,15 @@ public class BaseFunctionTest extends EvaluationTestCase {
         + "b1 = bar(name='foo', type='jpg', version=42)\n"
         + "b2 = bar()\n");
 
-    assertThat(EvalUtils.prettyPrintValue(lookup("v1")))
+    assertThat(Printer.repr(lookup("v1")))
         .isEqualTo("(1, 2, 3, 4, 5, 6, 7, 8, (), {})");
-    assertThat(EvalUtils.prettyPrintValue(lookup("v2")))
+    assertThat(Printer.repr(lookup("v2")))
         .isEqualTo("(1, \"x\", \"y\", \"z\", 5, 6, 7, 9, (\"t\",), {\"i\": 0})");
 
     // NB: the conversion to a TreeMap below ensures the keys are sorted.
-    assertThat(EvalUtils.prettyPrintValue(
+    assertThat(Printer.repr(
         new TreeMap<String, Object>((Map<String, Object>) lookup("b1"))))
         .isEqualTo("{\"name\": \"foo\", \"type\": \"jpg\", \"version\": 42}");
-    assertThat(EvalUtils.prettyPrintValue(lookup("b2"))).isEqualTo("{}");
+    assertThat(Printer.repr(lookup("b2"))).isEqualTo("{}");
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
index 13c0987364..fa98c70e3d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
@@ -14,11 +14,9 @@
 
 package com.google.devtools.build.lib.syntax;
 
-import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import com.google.common.collect.Lists;
 
@@ -27,7 +25,6 @@ import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 import java.util.Arrays;
-import java.util.IllegalFormatException;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -39,15 +36,18 @@ import java.util.Map;
 @RunWith(JUnit4.class)
 public class EvalUtilsTest {
 
-  private static List<?> makeList(Object ...args) {
+  private static List<?> makeList(Object... args) {
     return EvalUtils.makeSequence(Arrays.<Object>asList(args), false);
   }
-  private static List<?> makeTuple(Object ...args) {
+
+  private static List<?> makeTuple(Object... args) {
     return EvalUtils.makeSequence(Arrays.<Object>asList(args), true);
   }
+
   private static Map<Object, Object> makeDict() {
     return new LinkedHashMap<>();
   }
+
   private static FilesetEntry makeFilesetEntry() {
     try {
       return new FilesetEntry(Label.parseAbsolute("//foo:bar"),
@@ -78,164 +78,4 @@ public class EvalUtilsTest {
     assertFalse(EvalUtils.isImmutable(makeDict()));
     assertFalse(EvalUtils.isImmutable(makeFilesetEntry()));
   }
-
-  @Test
-  public void testPrintValue() throws Exception {
-    // Note that prettyPrintValue and printValue only differ on behaviour of
-    // labels and strings at toplevel.
-    assertEquals("foo\nbar", EvalUtils.printValue("foo\nbar"));
-    assertEquals("\"foo\\nbar\"", EvalUtils.prettyPrintValue("foo\nbar"));
-    assertEquals("foo\nbar", EvalUtils.printValue("foo\nbar"));
-    assertEquals("'", EvalUtils.printValue("'"));
-    assertEquals("\"'\"", EvalUtils.prettyPrintValue("'"));
-    assertEquals("\"", EvalUtils.printValue("\""));
-    assertEquals("\"\\\"\"", EvalUtils.prettyPrintValue("\""));
-    assertEquals("a\\b", EvalUtils.printValue("a\\b"));
-    assertEquals("\"a\\\\b\"", EvalUtils.prettyPrintValue("a\\b"));
-    assertEquals("3", EvalUtils.printValue(3));
-    assertEquals("3", EvalUtils.prettyPrintValue(3));
-    assertEquals("None", EvalUtils.printValue(Environment.NONE));
-    assertEquals("None", EvalUtils.prettyPrintValue(Environment.NONE));
-
-    assertEquals("//x:x", EvalUtils.printValue(Label.parseAbsolute("//x")));
-    assertEquals("\"//x:x\"", EvalUtils.prettyPrintValue(Label.parseAbsolute("//x")));
-
-    List<?> list = makeList("foo", "bar");
-    List<?> tuple = makeTuple("foo", "bar");
-
-    assertEquals("(1, [\"foo\", \"bar\"], 3)",
-                 EvalUtils.printValue(makeTuple(1, list, 3)));
-    assertEquals("(1, [\"foo\", \"bar\"], 3)",
-                 EvalUtils.prettyPrintValue(makeTuple(1, list, 3)));
-    assertEquals("[1, (\"foo\", \"bar\"), 3]",
-                 EvalUtils.printValue(makeList(1, tuple, 3)));
-    assertEquals("[1, (\"foo\", \"bar\"), 3]",
-                 EvalUtils.prettyPrintValue(makeList(1, tuple, 3)));
-
-    Map<Object, Object> dict = makeDict();
-    dict.put(1, tuple);
-    dict.put(2, list);
-    dict.put("foo", makeList());
-    assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
-                EvalUtils.printValue(dict));
-    assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
-                EvalUtils.prettyPrintValue(dict));
-    assertEquals("FilesetEntry(srcdir = \"//foo:bar\", files = [], "
-               + "excludes = [\"xyz\"], destdir = \"\", "
-               + "strip_prefix = \".\", symlinks = \"copy\")",
-                 EvalUtils.prettyPrintValue(makeFilesetEntry()));
-  }
-
-  private void checkFormatPositionalFails(String format, List<?> tuple,
-                                          String errorMessage) {
-    try {
-      EvalUtils.formatString(format, tuple);
-      fail();
-    } catch (IllegalFormatException e) {
-      assertThat(e).hasMessage(errorMessage);
-    }
-  }
-
-  @Test
-  public void testFormatPositional() throws Exception {
-    assertEquals("foo 3", EvalUtils.formatString("%s %d", makeTuple("foo", 3)));
-
-    // Note: formatString doesn't perform scalar x -> (x) conversion;
-    // The %-operator is responsible for that.
-    assertThat(EvalUtils.formatString("", makeTuple())).isEmpty();
-    assertEquals("foo", EvalUtils.formatString("%s", makeTuple("foo")));
-    assertEquals("3.14159", EvalUtils.formatString("%s", makeTuple(3.14159)));
-    checkFormatPositionalFails("%s", makeTuple(1, 2, 3),
-        "not all arguments converted during string formatting");
-    assertEquals("%foo", EvalUtils.formatString("%%%s", makeTuple("foo")));
-    checkFormatPositionalFails("%%s", makeTuple("foo"),
-        "not all arguments converted during string formatting");
-    checkFormatPositionalFails("% %s", makeTuple("foo"),
-        "invalid arguments for format string");
-    assertEquals("[1, 2, 3]", EvalUtils.formatString("%s", makeTuple(makeList(1, 2, 3))));
-    assertEquals("(1, 2, 3)", EvalUtils.formatString("%s", makeTuple(makeTuple(1, 2, 3))));
-    assertEquals("[]", EvalUtils.formatString("%s", makeTuple(makeList())));
-    assertEquals("()", EvalUtils.formatString("%s", makeTuple(makeTuple())));
-
-    checkFormatPositionalFails("%.3g", makeTuple(), "invalid arguments for format string");
-    checkFormatPositionalFails("%.3g", makeTuple(1, 2), "invalid arguments for format string");
-    checkFormatPositionalFails("%.s", makeTuple(), "invalid arguments for format string");
-  }
-
-  private String createExpectedFilesetEntryString(FilesetEntry.SymlinkBehavior symlinkBehavior) {
-    return "FilesetEntry(srcdir = \"//x:x\","
-           + " files = [\"//x:x\"],"
-           + " excludes = [],"
-           + " destdir = \"\","
-           + " strip_prefix = \".\","
-           + " symlinks = \"" + symlinkBehavior.toString().toLowerCase() + "\")";
-  }
-
-  private FilesetEntry createTestFilesetEntry(FilesetEntry.SymlinkBehavior symlinkBehavior)
-    throws Exception {
-    Label label = Label.parseAbsolute("//x");
-    return new FilesetEntry(label,
-                            Arrays.asList(label),
-                            Arrays.<String>asList(),
-                            "",
-                            symlinkBehavior,
-                            ".");
-  }
-
-  @Test
-  public void testFilesetEntrySymlinkAttr() throws Exception {
-    FilesetEntry entryDereference =
-      createTestFilesetEntry(FilesetEntry.SymlinkBehavior.DEREFERENCE);
-
-    assertEquals(createExpectedFilesetEntryString(FilesetEntry.SymlinkBehavior.DEREFERENCE),
-                 EvalUtils.prettyPrintValue(entryDereference));
-  }
-
-  private FilesetEntry createStripPrefixFilesetEntry(String stripPrefix)  throws Exception {
-    Label label = Label.parseAbsolute("//x");
-    return new FilesetEntry(
-        label,
-        Arrays.asList(label),
-        Arrays.<String>asList(),
-        "",
-        FilesetEntry.SymlinkBehavior.DEREFERENCE,
-        stripPrefix);
-  }
-
-  @Test
-  public void testFilesetEntryStripPrefixAttr() throws Exception {
-    FilesetEntry withoutStripPrefix = createStripPrefixFilesetEntry(".");
-    FilesetEntry withStripPrefix = createStripPrefixFilesetEntry("orange");
-
-    String prettyWithout = EvalUtils.prettyPrintValue(withoutStripPrefix);
-    String prettyWith = EvalUtils.prettyPrintValue(withStripPrefix);
-
-    assertThat(prettyWithout).contains("strip_prefix = \".\"");
-    assertThat(prettyWith).contains("strip_prefix = \"orange\"");
-  }
-
-  @Test
-  public void testRegressionCrashInPrettyPrintValue() throws Exception {
-    // Would cause crash in code such as this:
-    //  Fileset(name='x', entries=[], out=[FilesetEntry(files=['a'])])
-    // While formatting the "expected x, got y" message for the 'out'
-    // attribute, prettyPrintValue(FilesetEntry) would be recursively called
-    // with a List<Label> even though this isn't a valid datatype in the
-    // interpreter.
-    // Fileset isn't part of bazel, even though FilesetEntry is.
-    Label label = Label.parseAbsolute("//x");
-    assertEquals("FilesetEntry(srcdir = \"//x:x\","
-                 + " files = [\"//x:x\"],"
-                 + " excludes = [],"
-                 + " destdir = \"\","
-                 + " strip_prefix = \".\","
-                 + " symlinks = \"copy\")",
-                 EvalUtils.prettyPrintValue(
-                     new FilesetEntry(label,
-                                      Arrays.asList(label),
-                                      Arrays.<String>asList(),
-                                      "",
-                                      FilesetEntry.SymlinkBehavior.COPY,
-                                      ".")));
-  }
 }
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 b3e5bcff48..7177c7f1d0 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
@@ -542,7 +542,7 @@ public class EvaluationTest extends EvaluationTestCase {
   @Test
   public void testListComprehensionOnDictionaryCompositeExpression() throws Exception {
     eval("d = {1:'a',2:'b'}\n" + "l = [d[x] for x in d]");
-    assertEquals("[\"a\", \"b\"]", EvalUtils.prettyPrintValue(lookup("l")));
+    assertEquals("[\"a\", \"b\"]", Printer.repr(lookup("l")));
   }
 
   @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
new file mode 100644
index 0000000000..d69c76f681
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -0,0 +1,214 @@
+// Copyright 2006-2015 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.syntax;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.util.Arrays;
+import java.util.IllegalFormatException;
+import java.util.List;
+import java.util.Map;
+
+/**
+ *  Test properties of the evaluator's datatypes and utility functions
+ *  without actually creating any parse trees.
+ */
+@RunWith(JUnit4.class)
+public class PrinterTest {
+
+  private static List<?> makeList(Object... args) {
+    return EvalUtils.makeSequence(Arrays.<Object>asList(args), false);
+  }
+
+  private static List<?> makeTuple(Object... args) {
+    return EvalUtils.makeSequence(Arrays.<Object>asList(args), true);
+  }
+
+  private static FilesetEntry makeFilesetEntry() {
+    try {
+      return new FilesetEntry(Label.parseAbsolute("//foo:bar"),
+                              Lists.<Label>newArrayList(), Lists.newArrayList("xyz"), "",
+                              FilesetEntry.SymlinkBehavior.COPY, ".");
+    } catch (Label.SyntaxException e) {
+      throw new RuntimeException("Bad label: ", e);
+    }
+  }
+
+  @Test
+  public void testPrinter() throws Exception {
+    // Note that prettyPrintValue and printValue only differ on behaviour of
+    // labels and strings at toplevel.
+    assertEquals("foo\nbar", Printer.str("foo\nbar"));
+    assertEquals("\"foo\\nbar\"", Printer.repr("foo\nbar"));
+    assertEquals("'", Printer.str("'"));
+    assertEquals("\"'\"", Printer.repr("'"));
+    assertEquals("\"", Printer.str("\""));
+    assertEquals("\"\\\"\"", Printer.repr("\""));
+    assertEquals("3", Printer.str(3));
+    assertEquals("3", Printer.repr(3));
+    assertEquals("None", Printer.repr(Environment.NONE));
+
+    assertEquals("//x:x", Printer.str(Label.parseAbsolute("//x")));
+    assertEquals("\"//x:x\"", Printer.repr(Label.parseAbsolute("//x")));
+
+    List<?> list = makeList("foo", "bar");
+    List<?> tuple = makeTuple("foo", "bar");
+
+    assertEquals("(1, [\"foo\", \"bar\"], 3)",
+                 Printer.str(makeTuple(1, list, 3)));
+    assertEquals("(1, [\"foo\", \"bar\"], 3)",
+                 Printer.repr(makeTuple(1, list, 3)));
+    assertEquals("[1, (\"foo\", \"bar\"), 3]",
+                 Printer.str(makeList(1, tuple, 3)));
+    assertEquals("[1, (\"foo\", \"bar\"), 3]",
+                 Printer.repr(makeList(1, tuple, 3)));
+
+    Map<Object, Object> dict = ImmutableMap.of(
+        1, tuple,
+        2, list,
+        "foo", makeList());
+    assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
+                Printer.str(dict));
+    assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
+                Printer.repr(dict));
+    assertEquals("FilesetEntry(srcdir = \"//foo:bar\", files = [], "
+               + "excludes = [\"xyz\"], destdir = \"\", "
+               + "strip_prefix = \".\", symlinks = \"copy\")",
+                 Printer.repr(makeFilesetEntry()));
+  }
+
+  private void checkFormatPositionalFails(String format, List<?> tuple, String errorMessage) {
+    try {
+      Printer.format(format, tuple);
+      fail();
+    } catch (IllegalFormatException e) {
+      assertThat(e).hasMessage(errorMessage);
+    }
+  }
+
+  @Test
+  public void testFormatPositional() throws Exception {
+    assertEquals("foo 3", Printer.format("%s %d", makeTuple("foo", 3)));
+
+    // Note: formatString doesn't perform scalar x -> (x) conversion;
+    // The %-operator is responsible for that.
+    assertThat(Printer.format("", makeTuple())).isEmpty();
+    assertEquals("foo", Printer.format("%s", makeTuple("foo")));
+    assertEquals("3.14159", Printer.format("%s", makeTuple(3.14159)));
+    checkFormatPositionalFails("%s", makeTuple(1, 2, 3),
+        "not all arguments converted during string formatting");
+    assertEquals("%foo", Printer.format("%%%s", makeTuple("foo")));
+    checkFormatPositionalFails("%%s", makeTuple("foo"),
+        "not all arguments converted during string formatting");
+    checkFormatPositionalFails("% %s", makeTuple("foo"),
+        "invalid arguments for format string");
+    assertEquals("[1, 2, 3]", Printer.format("%s", makeTuple(makeList(1, 2, 3))));
+    assertEquals("(1, 2, 3)", Printer.format("%s", makeTuple(makeTuple(1, 2, 3))));
+    assertEquals("[]", Printer.format("%s", makeTuple(makeList())));
+    assertEquals("()", Printer.format("%s", makeTuple(makeTuple())));
+
+    checkFormatPositionalFails("%.3g", makeTuple(), "invalid arguments for format string");
+    checkFormatPositionalFails("%.3g", makeTuple(1, 2), "invalid arguments for format string");
+    checkFormatPositionalFails("%.s", makeTuple(), "invalid arguments for format string");
+  }
+
+  private String createExpectedFilesetEntryString(FilesetEntry.SymlinkBehavior symlinkBehavior) {
+    return "FilesetEntry(srcdir = \"//x:x\","
+           + " files = [\"//x:x\"],"
+           + " excludes = [],"
+           + " destdir = \"\","
+           + " strip_prefix = \".\","
+           + " symlinks = \"" + symlinkBehavior.toString().toLowerCase() + "\")";
+  }
+
+  private FilesetEntry createTestFilesetEntry(FilesetEntry.SymlinkBehavior symlinkBehavior)
+    throws Exception {
+    Label label = Label.parseAbsolute("//x");
+    return new FilesetEntry(
+        label,
+        Arrays.asList(label),
+        Arrays.<String>asList(),
+        "",
+        symlinkBehavior,
+        ".");
+  }
+
+  @Test
+  public void testFilesetEntrySymlinkAttr() throws Exception {
+    FilesetEntry entryDereference =
+      createTestFilesetEntry(FilesetEntry.SymlinkBehavior.DEREFERENCE);
+
+    assertEquals(createExpectedFilesetEntryString(FilesetEntry.SymlinkBehavior.DEREFERENCE),
+                 Printer.repr(entryDereference));
+  }
+
+  private FilesetEntry createStripPrefixFilesetEntry(String stripPrefix)  throws Exception {
+    Label label = Label.parseAbsolute("//x");
+    return new FilesetEntry(
+        label,
+        Arrays.asList(label),
+        Arrays.<String>asList(),
+        "",
+        FilesetEntry.SymlinkBehavior.DEREFERENCE,
+        stripPrefix);
+  }
+
+  @Test
+  public void testFilesetEntryStripPrefixAttr() throws Exception {
+    FilesetEntry withoutStripPrefix = createStripPrefixFilesetEntry(".");
+    FilesetEntry withStripPrefix = createStripPrefixFilesetEntry("orange");
+
+    String prettyWithout = Printer.repr(withoutStripPrefix);
+    String prettyWith = Printer.repr(withStripPrefix);
+
+    assertThat(prettyWithout).contains("strip_prefix = \".\"");
+    assertThat(prettyWith).contains("strip_prefix = \"orange\"");
+  }
+
+  @Test
+  public void testRegressionCrashInPrettyPrintValue() throws Exception {
+    // Would cause crash in code such as this:
+    //  Fileset(name='x', entries=[], out=[FilesetEntry(files=['a'])])
+    // While formatting the "expected x, got y" message for the 'out'
+    // attribute, prettyPrintValue(FilesetEntry) would be recursively called
+    // with a List<Label> even though this isn't a valid datatype in the
+    // interpreter.
+    // Fileset isn't part of bazel, even though FilesetEntry is.
+    Label label = Label.parseAbsolute("//x");
+    assertEquals("FilesetEntry(srcdir = \"//x:x\","
+                 + " files = [\"//x:x\"],"
+                 + " excludes = [],"
+                 + " destdir = \"\","
+                 + " strip_prefix = \".\","
+                 + " symlinks = \"copy\")",
+                 Printer.repr(
+                     new FilesetEntry(
+                         label,
+                         Arrays.asList(label),
+                         Arrays.<String>asList(),
+                         "",
+                         FilesetEntry.SymlinkBehavior.COPY,
+                         ".")));
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
index f00d8f5ab7..c4bb7d84a9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
@@ -72,7 +72,7 @@ class SkylarkShell {
       try {
         Object result = ev.eval(input);
         if (result != null) {
-          System.out.println(EvalUtils.prettyPrintValue(result));
+          System.out.println(Printer.repr(result));
         }
       } catch (Exception e) {
         e.printStackTrace();