Repository: tweag/nickel (rust)

Issue:
`std.string.find` doesn't properly handle optional capture groups
The regex `(a+)(b)?` should match the string "aa", and `std.string.is_match` agrees. But:

```
nickel> std.string.find "(a+)(b)?" "aa"
{ groups = [  ], index = -1, matched = "", }
```


Pull request description:
Fix empty capture groups in regexes
Fixes #2108 

Patch:
diff --git a/core/src/eval/operation.rs b/core/src/eval/operation.rs
index ce8c409e..0eeaa1ba 100644
--- a/core/src/eval/operation.rs
+++ b/core/src/eval/operation.rs
@@ -1006,7 +1006,12 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                                 "groups",
                                 RichTerm::from(Term::Array(
                                     Array::from_iter(
-                                        groups.into_iter().map(|s| Term::Str(s).into())
+                                        groups
+                                            .into_iter()
+                                            // Unmatched groups get turned into empty strings. It
+                                            // might be nicer to have a 'Some s / 'None instead,
+                                            // but that would be an API break.
+                                            .map(|s| Term::Str(s.unwrap_or_default()).into())
                                     ),
                                     ArrayAttrs::new().closurized()
                                 ))
@@ -1029,7 +1034,13 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                                     "groups",
                                     RichTerm::from(Term::Array(
                                         Array::from_iter(
-                                            found.groups.into_iter().map(|s| Term::Str(s).into())
+                                            found
+                                                .groups
+                                                .into_iter()
+                                                // Unmatched groups get turned into empty strings. It
+                                                // might be nicer to have a 'Some s / 'None instead,
+                                                // but that would be an API break.
+                                                .map(|s| Term::Str(s.unwrap_or_default()).into())
                                         ),
                                         ArrayAttrs::new().closurized()
                                     ))
diff --git a/core/src/term/string.rs b/core/src/term/string.rs
index aca416a5..5533542d 100644
--- a/core/src/term/string.rs
+++ b/core/src/term/string.rs
@@ -316,7 +316,7 @@ impl NickelString {
             let groups = capt
                 .iter()
                 .skip(1)
-                .filter_map(|s_opt| s_opt.map(|s| s.as_str().into()))
+                .map(|s_opt| s_opt.map(|s| s.as_str().into()))
                 .collect();
 
             // The indices returned by the `regex` crate are byte offsets into
@@ -357,7 +357,10 @@ impl Default for NickelString {
 pub struct RegexFindResult {
     pub matched: NickelString,
     pub index: Number,
-    pub groups: Vec<NickelString>,
+    /// If a capture group didn't match, we store a `None`. This `None` placeholders
+    /// make the indexing predictable, so it's possible to associate captures with
+    /// parenthesis groupings in the original regex.
+    pub groups: Vec<Option<NickelString>>,
 }
 
 /// Errors returned by `NickelString`'s `substring` method.
@@ -573,9 +576,7 @@ mod grapheme_cluster_preservation {
         ) -> impl Iterator<Item = regex::Captures<'a>> {
             needle.captures_iter(haystack).filter(|c| {
                 c.iter().all(|maybe_match| {
-                    maybe_match
-                        .map(|m| does_match_start_and_end_on_boundary(haystack, &m))
-                        .unwrap_or(false)
+                    maybe_match.map_or(true, |m| does_match_start_and_end_on_boundary(haystack, &m))
                 })
             })
         }


Test patch:
diff --git a/core/tests/integration/inputs/stdlib/string_find.ncl b/core/tests/integration/inputs/stdlib/string_find.ncl
index 2dd0a7a6..8324ec55 100644
--- a/core/tests/integration/inputs/stdlib/string_find.ncl
+++ b/core/tests/integration/inputs/stdlib/string_find.ncl
@@ -6,6 +6,7 @@ let { string, .. } = std in
   std.string.find "a" "aaa bbb ccc abc" == { groups = [], index = 0, matched = "a" },
   std.string.find "([a-z]+)=([0-9]+)" "one=1, two=2, three=3" == { groups = ["one", "1"], index = 0, matched = "one=1" },
   std.string.find "(\\d+)\\.(\\d+)\\.(\\d+)" "1.2.3" == { groups = ["1", "2", "3"], index = 0, matched = "1.2.3" },
-  std.string.find "(\\p{Emoji})=(\\w+)" "😀=smiling" == { groups = ["😀", "smiling"], index = 0, matched = "😀=smiling" }
+  std.string.find "(\\p{Emoji})=(\\w+)" "😀=smiling" == { groups = ["😀", "smiling"], index = 0, matched = "😀=smiling" },
+  std.string.find "a(b)?" "ac" == { groups = [""], index = 0, matched = "a" },
 ]
 |> std.test.assert_all