Add some strip/rstrip/lstrip tests

and fix the bugs that showed up as a result.

--
MOS_MIGRATED_REVID=105896905

2 files changed, 20 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 62cb8df6b6..e14fbad18a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -166,7 +166,7 @@ public class MethodLibrary {
         name = "chars",
         type = String.class,
         doc = "The characters to remove",
-        defaultValue = "' \t'"
+        defaultValue = "' \\t\\n\\r'"  // \f \v are illegal in Skylark
       )
     }
   )
@@ -195,7 +195,7 @@ public class MethodLibrary {
         name = "chars",
         type = String.class,
         doc = "The characters to remove",
-        defaultValue = "' \t'"
+        defaultValue = "' \\t\\n\\r'"  // \f \v are illegal in Skylark
       )
     }
   )
@@ -224,7 +224,7 @@ public class MethodLibrary {
         name = "chars",
         type = String.class,
         doc = "The characters to remove",
-        defaultValue = "' \t'"
+        defaultValue = "' \\t\\n\\r'"  // \f \v are illegal in Skylark
       )
     }
   )
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 86e8d8fbdf..c78fba035c 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
@@ -1093,29 +1093,38 @@ public class MethodLibraryTest extends EvaluationTestCase {
   @Test
   public void testLStrip() throws Exception {
     new BothModesTest()
-        .testStatement("'abc'.lstrip('')", "abc")
+        .testStatement("'a b c'.lstrip('')", "a b c")
         .testStatement("'abcba'.lstrip('ba')", "cba")
         .testStatement("'abc'.lstrip('xyz')", "abc")
-        .testStatement("'  abc  '.lstrip()", "abc  ");
+        .testStatement("'  a b c  '.lstrip()", "a b c  ")
+        // the "\\"s are because Java absorbs one level of "\"s
+        .testStatement("' \\t\\na b c '.lstrip()", "a b c ")
+        .testStatement("' a b c '.lstrip('')", " a b c ");
   }
 
   @Test
   public void testRStrip() throws Exception {
     new BothModesTest()
-        .testStatement("'abc'.rstrip('')", "abc")
+        .testStatement("'a b c'.rstrip('')", "a b c")
         .testStatement("'abcba'.rstrip('ba')", "abc")
         .testStatement("'abc'.rstrip('xyz')", "abc")
-        .testStatement("'  abc  '.rstrip()", "  abc");
+        .testStatement("'  a b c  '.rstrip()", "  a b c")
+        // the "\\"s are because Java absorbs one level of "\"s
+        .testStatement("' a b c \\t \\n'.rstrip()", " a b c")
+        .testStatement("' a b c '.rstrip('')", " a b c ");
   }
 
   @Test
   public void testStrip() throws Exception {
     new BothModesTest()
-        .testStatement("'abc'.strip('')", "abc")
+        .testStatement("'a b c'.strip('')", "a b c")
         .testStatement("'abcba'.strip('ba')", "c")
         .testStatement("'abc'.strip('xyz')", "abc")
-        .testStatement("'  abc  '.strip()", "abc")
-        .testStatement("' abc\t'.strip()", "abc")
-        .testStatement("'abc'.strip('.')", "abc");
+        .testStatement("'  a b c  '.strip()", "a b c")
+        .testStatement("' a b c\\t'.strip()", "a b c")
+        .testStatement("'a b c'.strip('.')", "a b c")
+        // the "\\"s are because Java absorbs one level of "\"s
+        .testStatement("' \\t\\n\\ra b c \\t\\n\\r'.strip()", "a b c")
+        .testStatement("' a b c '.strip('')", " a b c ");
   }
 }