Remove deprecated legacy string representations of Skylark objects

RELNOTES[INC]: The flag --incompatible_descriptive_string_representations is no
longer available, old style string representations of objects are not supported
anymore.

PiperOrigin-RevId: 171952621

6 files changed, 7 insertions, 157 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 65e00eebcc..3e1708f3dd 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -109,7 +109,6 @@ public class SkylarkSemanticsConsistencyTest {
         "--incompatible_checked_arithmetic=" + rand.nextBoolean(),
         "--incompatible_comprehension_variables_do_not_leak=" + rand.nextBoolean(),
         "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
-        "--incompatible_descriptive_string_representations=" + rand.nextBoolean(),
         "--incompatible_dict_literal_has_no_duplicates=" + rand.nextBoolean(),
         "--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
         "--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(),
@@ -135,7 +134,6 @@ public class SkylarkSemanticsConsistencyTest {
         .incompatibleCheckedArithmetic(rand.nextBoolean())
         .incompatibleComprehensionVariablesDoNotLeak(rand.nextBoolean())
         .incompatibleDepsetIsNotIterable(rand.nextBoolean())
-        .incompatibleDescriptiveStringRepresentations(rand.nextBoolean())
         .incompatibleDictLiteralHasNoDuplicates(rand.nextBoolean())
         .incompatibleDisallowDictPlus(rand.nextBoolean())
         .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index bf62065844..4a196f710b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -568,7 +568,6 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
 
   @Test
   public void existingRuleWithSelect() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
     scratch.file(
         "test/existing_rule.bzl",
         "def macro():",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
index 78618b5fd5..72f657df91 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
@@ -223,8 +223,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Strings() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertThat(skylarkLoadingEval("str('foo')")).isEqualTo("foo");
     assertThat(skylarkLoadingEval("'%s' % 'foo'")).isEqualTo("foo");
     assertThat(skylarkLoadingEval("'{}'.format('foo')")).isEqualTo("foo");
@@ -234,8 +232,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Labels() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertThat(skylarkLoadingEval("str(Label('//foo:bar'))")).isEqualTo("//foo:bar");
     assertThat(skylarkLoadingEval("'%s' % Label('//foo:bar')")).isEqualTo("//foo:bar");
     assertThat(skylarkLoadingEval("'{}'.format(Label('//foo:bar'))")).isEqualTo("//foo:bar");
@@ -248,8 +244,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
   @Test
   public void testStringRepresentations_Primitives() throws Exception {
     // Strings are tested in a separate test case as they have different str and repr values.
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("1543", "1543");
     assertStringRepresentation("True", "True");
     assertStringRepresentation("False", "False");
@@ -257,8 +251,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Containers() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("['a', 'b']", "[\"a\", \"b\"]");
     assertStringRepresentation("('a', 'b')", "(\"a\", \"b\")");
     assertStringRepresentation("{'a': 'b', 'c': 'd'}", "{\"a\": \"b\", \"c\": \"d\"}");
@@ -267,31 +259,23 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Functions() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("all", "<built-in function all>");
     assertStringRepresentation("def f(): pass", "f", "<function f from //eval:eval.bzl>");
   }
 
   @Test
   public void testStringRepresentations_Rules() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("native.cc_library", "<built-in rule cc_library>");
     assertStringRepresentation("rule(implementation=str)", "<rule>");
   }
 
   @Test
   public void testStringRepresentations_Aspects() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("aspect(implementation=str)", "<aspect>");
   }
 
   @Test
   public void testStringRepresentations_Providers() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("provider()", "<provider>");
     assertStringRepresentation(
         "p = provider()", "p(b = 'foo', a = 1)", "struct(a = 1, b = \"foo\")");
@@ -299,8 +283,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Select() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation(
         "select({'//foo': ['//bar']}) + select({'//foo2': ['//bar2']})",
         "select({\"//foo\": [\"//bar\"]}) + select({\"//foo2\": [\"//bar2\"]})");
@@ -308,8 +290,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_RuleContext() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     generateFilesToTestStrings();
     ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
 
@@ -325,8 +305,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Files() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     generateFilesToTestStrings();
     ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
 
@@ -340,8 +318,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Root() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     generateFilesToTestStrings();
     ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
 
@@ -353,8 +329,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Glob() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     scratch.file("eval/one.txt");
     scratch.file("eval/two.txt");
     scratch.file("eval/three.txt");
@@ -366,8 +340,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Attr() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     assertStringRepresentation("attr", "<attr>");
     assertStringRepresentation("attr.int()", "<attr.int>");
     assertStringRepresentation("attr.string()", "<attr.string>");
@@ -386,8 +358,6 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
 
   @Test
   public void testStringRepresentations_Targets() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
-
     generateFilesToTestStrings();
     ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
 
@@ -415,92 +385,4 @@ public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
     assertThat(eval("'%s' % mock")).isEqualTo("<unknown object java.lang.Object>");
     assertThat(eval("'%r' % mock")).isEqualTo("<unknown object java.lang.Object>");
   }
-
-  @Test
-  public void testLegacyStringRepresentations_Labels() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertThat(skylarkLoadingEval("str(Label('//foo:bar'))")).isEqualTo("//foo:bar");
-    assertThat(skylarkLoadingEval("'%s' % Label('//foo:bar')")).isEqualTo("//foo:bar");
-    assertThat(skylarkLoadingEval("'{}'.format(Label('//foo:bar'))")).isEqualTo("//foo:bar");
-    assertThat(skylarkLoadingEval("repr(Label('//foo:bar'))")).isEqualTo("\"//foo:bar\"");
-    assertThat(skylarkLoadingEval("'%r' % Label('//foo:bar')")).isEqualTo("\"//foo:bar\"");
-
-    // Also test that str representations (as opposed to repr) also use legacy formatting
-    // They are equivalent for labels, but not equivalent for lists of labels, because
-    // containers always render their items with repr
-    assertThat(skylarkLoadingEval("str([Label('//foo:bar')])")).isEqualTo("[\"//foo:bar\"]");
-    assertThat(skylarkLoadingEval("'{}'.format([Label('//foo:bar')])")).isEqualTo("[\"//foo:bar\"]");
-    assertThat(skylarkLoadingEval("'%s' % [Label('//foo:bar')]")).isEqualTo("[\"//foo:bar\"]");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Functions() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertStringRepresentation("all", "<function all>");
-    assertStringRepresentation("def f(): pass", "f", "<function f>");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Rules() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertStringRepresentation("native.cc_library", "<function cc_library>");
-    assertStringRepresentation("rule(implementation=str)", "<function rule>");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Aspects() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertStringRepresentation("aspect(implementation=str)", "Aspect:<function str>");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Providers() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertStringRepresentation("provider()", "<function <no name>>");
-    assertStringRepresentation("p = provider()", "p(b = 'foo', a = 1)", "p(a = 1, b = \"foo\")");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Select() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    assertStringRepresentation(
-        "select({'//foo': ['//bar']}) + select({'//foo2': ['//bar2']})",
-        "selector({\"//foo\": [\"//bar\"]}) + selector({\"//foo2\": [\"//bar2\"]})");
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_RuleContext() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    generateFilesToTestStrings();
-    ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
-
-    for (String suffix : SUFFIXES) {
-      assertThat(target.get("rule_ctx" + suffix)).isEqualTo("//test/skylark:check");
-      assertThat(target.get("aspect_ctx" + suffix)).isEqualTo("//test/skylark:bar");
-      assertThat(target.get("aspect_ctx.rule" + suffix))
-          .isEqualTo("rule_collection://test/skylark:bar");
-    }
-  }
-
-  @Test
-  public void testLegacyStringRepresentations_Files() throws Exception {
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    generateFilesToTestStrings();
-    ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
-
-    for (String suffix : SUFFIXES) {
-      assertThat(target.get("source_file" + suffix))
-          .isEqualTo("File:[/workspace[source]]test/skylark/input.txt");
-      assertThat((String) target.get("generated_file" + suffix))
-          .endsWith("test/skylark/output.txt");
-    }
-  }
 }
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 ae21bee88a..5ac5a2ff57 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
@@ -633,22 +633,14 @@ public class EvaluationTest extends EvaluationTestCase {
       public void repr(SkylarkPrinter printer) {
         printer.append("<str marker>");
       }
-
-      @Override
-      public void reprLegacy(SkylarkPrinter printer) {
-        printer.append("<str legacy marker>");
-      }
     };
   }
 
   @Test
   public void testPercOnObject() throws Exception {
-    newTest("--incompatible_descriptive_string_representations=true")
+    newTest()
         .update("obj", createObjWithStr())
         .testStatement("'%s' % obj", "<str marker>");
-    newTest("--incompatible_descriptive_string_representations=false")
-        .update("obj", createObjWithStr())
-        .testStatement("'%s' % obj", "<str legacy marker>");
     newTest()
         .update("unknown", new Object())
         .testStatement("'%s' % unknown", "<unknown object java.lang.Object>");
@@ -656,12 +648,9 @@ public class EvaluationTest extends EvaluationTestCase {
 
   @Test
   public void testPercOnObjectList() throws Exception {
-    newTest("--incompatible_descriptive_string_representations=true")
+    newTest()
         .update("obj", createObjWithStr())
         .testStatement("'%s %s' % (obj, obj)", "<str marker> <str marker>");
-    newTest("--incompatible_descriptive_string_representations=false")
-        .update("obj", createObjWithStr())
-        .testStatement("'%s %s' % (obj, obj)", "<str legacy marker> <str legacy marker>");
     newTest()
         .update("unknown", new Object())
         .testStatement(
@@ -671,10 +660,7 @@ public class EvaluationTest extends EvaluationTestCase {
 
   @Test
   public void testPercOnObjectInvalidFormat() throws Exception {
-    newTest("--incompatible_descriptive_string_representations=true")
-        .update("obj", createObjWithStr())
-        .testIfExactError("invalid argument <str marker> for format pattern %d", "'%d' % obj");
-    newTest("--incompatible_descriptive_string_representations=false")
+    newTest()
         .update("obj", createObjWithStr())
         .testIfExactError("invalid argument <str marker> for format pattern %d", "'%d' % obj");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 081f1ad478..0bc19519c6 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -1621,7 +1621,7 @@ public class MethodLibraryTest extends EvaluationTestCase {
 
   @Test
   public void testStr() throws Exception {
-    new BothModesTest("--incompatible_descriptive_string_representations=true")
+    new BothModesTest()
         .testStatement("str(1)", "1")
         .testStatement("str(-2)", "-2")
         .testStatement("str([1, 2])", "[1, 2]")
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 84386b6a5f..5adbd10b4e 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
@@ -46,6 +46,9 @@ public class PrinterTest {
   public void testPrinter() throws Exception {
     // Note that prettyPrintValue and printValue only differ on behaviour of
     // labels and strings at toplevel.
+    assertThat(Printer.str(createObjWithStr())).isEqualTo("<str marker>");
+    assertThat(Printer.repr(createObjWithStr())).isEqualTo("<repr marker>");
+
     assertThat(Printer.str("foo\nbar")).isEqualTo("foo\nbar");
     assertThat(Printer.repr("foo\nbar")).isEqualTo("\"foo\\nbar\"");
     assertThat(Printer.str("'")).isEqualTo("'");
@@ -219,14 +222,6 @@ public class PrinterTest {
     assertThat(Printer.str(list)).isEqualTo(String.format("[%s]", Joiner.on(", ").join(list)));
   }
 
-  @Test
-  public void testLegacyPrinter() throws Exception {
-    assertThat(new Printer.LegacyPrinter().str(createObjWithStr()).toString())
-        .isEqualTo("<str legacy marker>");
-    assertThat(new Printer.LegacyPrinter().repr(createObjWithStr()).toString())
-        .isEqualTo("<repr legacy marker>");
-  }
-
   private String printListWithLimit(List<?> list) {
     return printList(list, Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
         Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
@@ -245,19 +240,9 @@ public class PrinterTest {
       }
 
       @Override
-      public void reprLegacy(SkylarkPrinter printer) {
-        printer.append("<repr legacy marker>");
-      }
-
-      @Override
       public void str(SkylarkPrinter printer) {
         printer.append("<str marker>");
       }
-
-      @Override
-      public void strLegacy(SkylarkPrinter printer) {
-        printer.append("<str legacy marker>");
-      }
     };
   }
 }