Repository: bazelbuild/bazel
Issue: support passing tuples to str.startswith and str.endswith
Labels: type: feature request
good first issue

Python supports passing a tuple to the first argument of `str.startswith` and `str.endswith` to allow testing multiple prefixes or suffixes in one go. I've now wanted this feature in Skylark a few times, so I'm formally submitting a request.

Issue comments:
This is a reasonable feature request.
Should be easy to implement.

Pull request review comments:
Why are we allocating a copy of the list here, only to throw it away?
Python allows only string or tuple-of-string here, not list of string, and so should we.
Done
Done
I believe we don't check the element type, that the `generic1` part of `ParamType` is just for documentation generation purposes. This should be replaced by

    for (String s : subs.getContents(String.class, "string")) {
        ...
    }

Same for `startsWith` below.
Can we add cases for:
- singleton tuple
- empty tuple
- tuple with element of wrong type
Could be a good idea to document that.
Done
Indeed, I'll add that caveat to Param's javadoc now.

Note that the var `subs` and the `@SuppressWarnings` are no longer needed, since the cast is done by `getContents`. `ConversionException` extends `EvalException` so this can just be `EvalException`. (Same below, of course.)
Thanks.

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java b/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
index e18c833097e384..2f2287911f31ff 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StringModule.java
@@ -19,6 +19,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.ParamType;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -893,7 +894,13 @@ public SkylarkList<String> elems(String self) throws ConversionException {
               + "and <code>end</code> being exclusive.",
       parameters = {
         @Param(name = "self", type = String.class, doc = "This string."),
-        @Param(name = "sub", type = String.class, legacyNamed = true,
+        @Param(
+            name = "sub",
+            allowedTypes = {
+              @ParamType(type = String.class),
+              @ParamType(type = Tuple.class, generic1 = String.class),
+            },
+            legacyNamed = true,
             doc = "The substring to check."),
         @Param(
             name = "start",
@@ -909,9 +916,21 @@ public SkylarkList<String> elems(String self) throws ConversionException {
             defaultValue = "None",
             doc = "optional position at which to stop comparing.")
       })
-  public Boolean endsWith(String self, String sub, Integer start, Object end)
-      throws ConversionException {
-    return pythonSubstring(self, start, end, "'end' operand of 'endswith'").endsWith(sub);
+  public Boolean endsWith(String self, Object sub, Integer start, Object end)
+      throws ConversionException, EvalException {
+    String str = pythonSubstring(self, start, end, "'end' operand of 'endswith'");
+    if (sub instanceof String) {
+      return str.endsWith((String)sub);
+    }
+
+    @SuppressWarnings("unchecked")
+    Tuple<Object> subs = (Tuple<Object>) sub;
+    for (String s : subs.getContents(String.class, "string")) {
+      if (str.endsWith(s)) {
+        return true;
+      }
+    }
+    return false;
   }
 
   // In Python, formatting is very complex.
@@ -966,7 +985,13 @@ public String format(String self, SkylarkList<?> args, SkylarkDict<?, ?> kwargs,
               + "<code>end</code> being exclusive.",
       parameters = {
         @Param(name = "self", type = String.class, doc = "This string."),
-        @Param(name = "sub", type = String.class, legacyNamed = true,
+        @Param(
+            name = "sub",
+            allowedTypes = {
+              @ParamType(type = String.class),
+              @ParamType(type = Tuple.class, generic1 = String.class),
+            },
+            legacyNamed = true,
             doc = "The substring to check."),
         @Param(
             name = "start",
@@ -982,9 +1007,21 @@ public String format(String self, SkylarkList<?> args, SkylarkDict<?, ?> kwargs,
             defaultValue = "None",
             doc = "Stop comparing at this position.")
       })
-  public Boolean startsWith(String self, String sub, Integer start, Object end)
-      throws ConversionException {
-    return pythonSubstring(self, start, end, "'end' operand of 'startswith'").startsWith(sub);
+  public Boolean startsWith(String self, Object sub, Integer start, Object end)
+      throws ConversionException, EvalException {
+    String str = pythonSubstring(self, start, end, "'end' operand of 'startswith'");
+    if (sub instanceof String) {
+      return str.startsWith((String)sub);
+    }
+
+    @SuppressWarnings("unchecked")
+    Tuple<Object> subs = (Tuple<Object>) sub;
+    for (String s : subs.getContents(String.class, "string")) {
+      if (str.startsWith(s)) {
+        return true;
+      }
+    }
+    return false;
   }
 
   public static final StringModule INSTANCE = new StringModule();


Test patch:
diff --git a/src/test/skylark/testdata/string_misc.sky b/src/test/skylark/testdata/string_misc.sky
index 901d7f3091ca7f..87708f9fae3b64 100644
--- a/src/test/skylark/testdata/string_misc.sky
+++ b/src/test/skylark/testdata/string_misc.sky
@@ -62,6 +62,26 @@ assert_eq('Apricot'.endswith('co', -1), False)
 assert_eq('abcd'.endswith('c', -2, -1), True)
 assert_eq('abcd'.endswith('c', 1, 8), False)
 assert_eq('abcd'.endswith('d', 1, 8), True)
+assert_eq('Apricot'.endswith(('cot', 'toc')), True)
+assert_eq('Apricot'.endswith(('toc', 'cot')), True)
+assert_eq('a'.endswith(('', '')), True)
+assert_eq('a'.endswith(('', 'a')), True)
+assert_eq('a'.endswith(('a', 'a')), True)
+assert_eq(''.endswith(('a', '')), True)
+assert_eq(''.endswith(('', '')), True)
+assert_eq(''.endswith(('a', 'a')), False)
+assert_eq('a'.endswith(('a')), True)
+assert_eq('a'.endswith(('a',)), True)
+assert_eq('a'.endswith(('b',)), False)
+assert_eq('a'.endswith(()), False)
+assert_eq(''.endswith(()), False)
+---
+'a'.endswith(['a']) ### expected value of type 'string or tuple of strings' for parameter 'sub', in method call endswith(list) of 'string'
+---
+'1'.endswith((1,)) ### expected type 'string' for 'string' element but got type 'int' instead
+---
+'a'.endswith(('1', 1)) ### expected type 'string' for 'string' element but got type 'int' instead
+---
 
 # startswith
 assert_eq('Apricot'.startswith('Apr'), True)
@@ -70,6 +90,26 @@ assert_eq('Apricot'.startswith(''), True)
 assert_eq('Apricot'.startswith('z'), False)
 assert_eq(''.startswith(''), True)
 assert_eq(''.startswith('a'), False)
+assert_eq('Apricot'.startswith(('Apr', 'rpA')), True)
+assert_eq('Apricot'.startswith(('rpA', 'Apr')), True)
+assert_eq('a'.startswith(('', '')), True)
+assert_eq('a'.startswith(('', 'a')), True)
+assert_eq('a'.startswith(('a', 'a')), True)
+assert_eq(''.startswith(('a', '')), True)
+assert_eq(''.startswith(('', '')), True)
+assert_eq(''.startswith(('a', 'a')), False)
+assert_eq('a'.startswith(('a')), True)
+assert_eq('a'.startswith(('a',)), True)
+assert_eq('a'.startswith(('b',)), False)
+assert_eq('a'.startswith(()), False)
+assert_eq(''.startswith(()), False)
+---
+'a'.startswith(['a']) ### expected value of type 'string or tuple of strings' for parameter 'sub', in method call startswith(list) of 'string'
+---
+'1'.startswith((1,)) ### expected type 'string' for 'string' element but got type 'int' instead
+---
+'a'.startswith(('1', 1)) ### expected type 'string' for 'string' element but got type 'int' instead
+---
 
 # substring
 assert_eq('012345678'[0:-1], "01234567")