Add start/end params to str.startswith/endswith, allow named default in dict.pop (#172)

Summary:
This PR makes three Bazel-compatible improvements to Starlark built-in methods:

1. **`str.startswith(prefix[, start[, end]])`** — Add optional `start` and `end` parameters, matching the [Starlark spec](https://github.com/bazelbuild/starlark/blob/master/spec.md#string%C2%B7startswith) and Python semantics. The substring `s[start:end]` is tested instead of the full string.

2. **`str.endswith(suffix[, start[, end]])`** — Same as above for `endswith()`.

3. **`dict.pop(key[, default])`** — Allow `default` to be passed as a named keyword argument (e.g., `d.pop("key", default=None)`), not just positional. This matches Bazel's Java Starlark implementation and Python's `dict.pop()`.

## Motivation

These changes are needed for Bazel compatibility. Real-world Bazel BUILD files and `.bzl` macros use these features extensively:

```python
# startswith with start/end (common in monorepo BUILD files)
[b for b in GLOBAL_BINARIES if b.startswith("//", 0, 2)]

# dict.pop with named default (common in rule macros)
workdir = kwargs.pop("workdir", default = None)
```

## Related Issues

- **[bazelbuild/starlark#56](bazelbuild/starlark#56 — The Starlark spec was updated to include `start`/`end` params for `startswith`/`endswith`, but `starlark-rust` never implemented them.
- **Conformance test suite inconsistency** — The [bazelbuild/starlark conformance tests](https://github.com/bazelbuild/starlark/blob/master/test_suite/testdata/go/dict.star) document this with: `# _inconsistency_: rust fails when default=None`

## Implementation Details

### `startswith` / `endswith`

Reuses the existing `convert_str_indices` infrastructure (already used by `find()`, `count()`, `index()`, `rfind()`, `rindex()`) to slice the string before testing. Supports both single string and tuple-of-strings prefix/suffix arguments.

### `dict.pop`

Removes `#[starlark(require = pos)]` from the `default` parameter, allowing it to be passed as either positional or named. The `key` parameter remains positional-only.

## Testing

- All 981 existing tests pass (828 lib + 153 doc)
- New tests added for each change:
  - `test_startswith_with_start_end` — 7 assertions covering basic, negative indices, and tuple prefix
  - `test_endswith_with_start_end` — 4 assertions covering basic, negative indices, and tuple suffix
  - `test_dict_pop_default_named` — 3 assertions covering missing key, existing key, and fallback value

Pull Request resolved: #172

Reviewed By: IanChilds

Differential Revision: D102404513

Pulled By: cjhopman

fbshipit-source-id: 61ef8f00e530a17cb8e5653aba6d7d8ef394f9ee

Author: twitchax

starlark/src/values/types/dict/methods.rs

@@ -169,7 +169,7 @@ pub(crate) fn dict_methods(registry: &mut MethodsBuilder) {
     fn pop<'v>(
         this: Value<'v>,
         #[starlark(require = pos)] key: Value<'v>,
-        #[starlark(require = pos)] default: Option<Value<'v>>,
+        default: Option<Value<'v>>,
     ) -> starlark::Result<Value<'v>> {
         let mut me = DictMut::from_value(this)?;
         match me.aref.remove_hashed(key.get_hashed()?) {
@@ -385,6 +385,14 @@ mod tests {
         assert::fail("x = {}; x.popitem()", "empty");
     }
 
+    #[test]
+    fn test_dict_pop_default_named() {
+        // Bazel compatibility: dict.pop() accepts `default` as a named kwarg
+        assert::is_true(r#"{"a": 1}.pop("b", default = None) == None"#);
+        assert::is_true(r#"{"a": 1}.pop("a", default = 99) == 1"#);
+        assert::is_true(r#"{"a": 1}.pop("b", default = 42) == 42"#);
+    }
+
     #[test]
     fn test_dict_add() {
         assert::fail("{1: 2} + {3: 4}", "not supported");

starlark/src/values/types/string/methods.rs

@@ -221,22 +221,30 @@ pub(crate) fn string_methods(builder: &mut MethodsBuilder) {
     /// https://github.com/bazelbuild/starlark/blob/master/spec.md#string·endswith
     /// ): determine if a string ends with a given suffix.
     ///
-    /// `S.endswith(suffix)` reports whether the string S has the specified
-    /// suffix.
+    /// `S.endswith(suffix[, start[, end]])` reports whether the string
+    /// `S[start:end]` has the specified suffix.
     ///
     /// ```
     /// # starlark::assert::all_true(r#"
     /// "filename.sky".endswith(".sky") == True
+    /// "hello".endswith("ell", 0, 4) == True
+    /// "hello".endswith("lo", 3) == True
     /// # "#);
     /// ```
     #[starlark(speculative_exec_safe)]
     fn endswith(
         this: &str,
         #[starlark(require = pos)] suffix: StringOrTuple,
+        #[starlark(require = pos, default = NoneOr::None)] start: NoneOr<i32>,
+        #[starlark(require = pos, default = NoneOr::None)] end: NoneOr<i32>,
     ) -> anyhow::Result<bool> {
+        let haystack = match convert_str_indices(this, start.into_option(), end.into_option()) {
+            Some(StrIndices { haystack, .. }) => haystack,
+            None => return Ok(false),
+        };
         match suffix {
-            StringOrTuple::String(x) => Ok(this.ends_with(x)),
-            StringOrTuple::Tuple(xs) => Ok(xs.items.iter().any(|x| this.ends_with(x))),
+            StringOrTuple::String(x) => Ok(haystack.ends_with(x)),
+            StringOrTuple::Tuple(xs) => Ok(xs.items.iter().any(|x| haystack.ends_with(x))),
         }
     }
 
@@ -1121,8 +1129,8 @@ pub(crate) fn string_methods(builder: &mut MethodsBuilder) {
     /// https://github.com/bazelbuild/starlark/blob/master/spec.md#string·startswith
     /// ): test whether a string starts with a given prefix.
     ///
-    /// `S.startswith(suffix)` reports whether the string S has the specified
-    /// prefix.
+    /// `S.startswith(prefix[, start[, end]])` reports whether the string
+    /// `S[start:end]` has the specified prefix.
     ///
     /// ```
     /// # starlark::assert::all_true(r#"
@@ -1131,16 +1139,26 @@ pub(crate) fn string_methods(builder: &mut MethodsBuilder) {
     /// 'abc'.startswith(('a', 'A')) == True
     /// 'ABC'.startswith(('a', 'A')) == True
     /// 'def'.startswith(('a', 'A')) == False
+    /// "//foo".startswith("//", 0, 2) == True
+    /// "//foo".startswith("foo", 2) == True
+    /// "hello".startswith("ell", 1, 4) == True
+    /// "hello".startswith("ell", 2, 4) == False
     /// # "#);
     /// ```
     #[starlark(speculative_exec_safe)]
     fn startswith(
         this: &str,
         #[starlark(require = pos)] prefix: StringOrTuple,
+        #[starlark(require = pos, default = NoneOr::None)] start: NoneOr<i32>,
+        #[starlark(require = pos, default = NoneOr::None)] end: NoneOr<i32>,
     ) -> anyhow::Result<bool> {
+        let haystack = match convert_str_indices(this, start.into_option(), end.into_option()) {
+            Some(StrIndices { haystack, .. }) => haystack,
+            None => return Ok(false),
+        };
         match prefix {
-            StringOrTuple::String(x) => Ok(this.starts_with(x)),
-            StringOrTuple::Tuple(xs) => Ok(xs.items.iter().any(|x| this.starts_with(x))),
+            StringOrTuple::String(x) => Ok(haystack.starts_with(x)),
+            StringOrTuple::Tuple(xs) => Ok(xs.items.iter().any(|x| haystack.starts_with(x))),
         }
     }
 
@@ -1309,4 +1327,30 @@ mod tests {
         assert::is_true("type('foo'.elems()) != type([])");
         assert::is_true("type('foo'.codepoints()) != type([])");
     }
+
+    #[test]
+    fn test_startswith_with_start_end() {
+        assert::all_true(
+            r#"
+"//foo".startswith("//", 0, 2) == True
+"//foo".startswith("foo", 2) == True
+"hello".startswith("ell", 1, 4) == True
+"hello".startswith("ell", 2, 4) == False
+"hello".startswith("hel", -5) == True
+"hello".startswith(("he", "wo"), 0, 3) == True
+"#,
+        );
+    }
+
+    #[test]
+    fn test_endswith_with_start_end() {
+        assert::all_true(
+            r#"
+"hello".endswith("ell", 0, 4) == True
+"hello".endswith("lo", 3) == True
+"hello".endswith("llo", 0, -1) == False
+"hello".endswith(("lo", "la"), 3) == True
+"#,
+        );
+    }
 }