fix: make URLPatternComponentResult::groups values optional (#35)

crowlKats · web-flow · commit 783c76bc233d · 2024-07-26T03:03:27.000-07:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -8,8 +8,8 @@ repository = "https://github.com/denoland/rust-urlpattern"
 license = "MIT"
 
 [dependencies]
-url = "2.2.2"
-regex = "1.4.3"
+url = "2.4.1"
+regex = "1.10.5"
 serde = { version = "1.0.127", features = ["derive"] }
 unic-ucd-ident = { version = "0.9.0", features = ["id"] }
 
diff --git a/src/component.rs b/src/component.rs
@@ -67,13 +67,13 @@ impl<R: RegExp> Component<R> {
   pub(crate) fn create_match_result(
     &self,
     input: String,
-    exec_result: Vec<&str>,
+    exec_result: Vec<Option<&str>>,
   ) -> crate::UrlPatternComponentResult {
     let groups = self
       .group_name_list
       .clone()
       .into_iter()
-      .zip(exec_result.into_iter().map(str::to_owned))
+      .zip(exec_result.into_iter().map(|s| s.map(str::to_owned)))
       .collect();
     crate::UrlPatternComponentResult { input, groups }
   }
diff --git a/src/lib.rs b/src/lib.rs
@@ -202,7 +202,7 @@ fn is_absolute_pathname(
 /// // Match the pattern against a URL.
 /// let url = "https://example.com/users/123".parse().unwrap();
 /// let result = pattern.exec(UrlPatternMatchInput::Url(url)).unwrap().unwrap();
-/// assert_eq!(result.pathname.groups.get("id").unwrap(), "123");
+/// assert_eq!(result.pathname.groups.get("id").unwrap().as_ref().unwrap(), "123");
 ///# }
 /// ```
 #[derive(Debug)]
@@ -496,7 +496,7 @@ pub struct UrlPatternComponentResult {
   /// The matched input for this component.
   pub input: String,
   /// The values for all named groups in the pattern.
-  pub groups: std::collections::HashMap<String, String>,
+  pub groups: std::collections::HashMap<String, Option<String>>,
 }
 
 #[cfg(test)]
@@ -525,7 +525,7 @@ mod tests {
   #[derive(Debug, Deserialize)]
   struct ComponentResult {
     input: String,
-    groups: HashMap<String, String>,
+    groups: HashMap<String, Option<String>>,
   }
 
   #[derive(Deserialize)]
@@ -787,7 +787,7 @@ mod tests {
             if !exactly_empty_components
               .contains(&stringify!($component).to_owned())
             {
-              groups.insert("0".to_owned(), "".to_owned());
+              groups.insert("0".to_owned(), Some("".to_owned()));
             }
             UrlPatternComponentResult {
               input: "".to_owned(),
diff --git a/src/matcher.rs b/src/matcher.rs
@@ -49,7 +49,10 @@ impl<R: RegExp> Matcher<R> {
     }
   }
 
-  pub fn matches<'a>(&self, mut input: &'a str) -> Option<Vec<&'a str>> {
+  pub fn matches<'a>(
+    &self,
+    mut input: &'a str,
+  ) -> Option<Vec<Option<&'a str>>> {
     let prefix_len = self.prefix.len();
     let suffix_len = self.suffix.len();
     let input_len = input.len();
@@ -82,7 +85,7 @@ impl<R: RegExp> Matcher<R> {
             return None;
           }
         }
-        Some(vec![input])
+        Some(vec![Some(input)])
       }
       InnerMatcher::RegExp { regexp, .. } => {
         regexp.as_ref().unwrap().matches(input)
diff --git a/src/parser.rs b/src/parser.rs
@@ -386,7 +386,7 @@ where
     if name_token.is_some() || regexp_or_wildcard_token.is_some() {
       let mut prefix = String::new();
       if let Some(char_token) = char_token {
-        prefix = char_token.value.to_owned();
+        char_token.value.clone_into(&mut prefix);
       }
       if !prefix.is_empty() && prefix != options.prefix_code_point {
         parser.pending_fixed_value.push_str(&prefix);
diff --git a/src/quirks.rs b/src/quirks.rs
@@ -176,7 +176,7 @@ impl RegExp for EcmaRegexp {
     Ok(EcmaRegexp(pattern.to_string()))
   }
 
-  fn matches<'a>(&self, text: &'a str) -> Option<Vec<&'a str>> {
+  fn matches<'a>(&self, text: &'a str) -> Option<Vec<Option<&'a str>>> {
     let regexp = regex::Regex::parse(&self.0).ok()?;
     regexp.matches(text)
   }
diff --git a/src/regexp.rs b/src/regexp.rs
@@ -11,10 +11,10 @@ pub trait RegExp: Sized {
   /// of captures. The matches are returned in the order they appear in the
   /// regular expression. It is **not** prefixed with the full match. For groups
   /// that occur in the regular expression, but did not match, the corresponding
-  /// capture should be the empty string ("").
+  /// capture should be `None`.
   ///
   /// Returns `None` if the text does not match the regular expression.
-  fn matches<'a>(&self, text: &'a str) -> Option<Vec<&'a str>>;
+  fn matches<'a>(&self, text: &'a str) -> Option<Vec<Option<&'a str>>>;
 }
 
 impl RegExp for regex::Regex {
@@ -26,13 +26,13 @@ impl RegExp for regex::Regex {
     regex::Regex::new(pattern).map_err(|_| ())
   }
 
-  fn matches<'a>(&self, text: &'a str) -> Option<Vec<&'a str>> {
+  fn matches<'a>(&self, text: &'a str) -> Option<Vec<Option<&'a str>>> {
     let captures = self.captures(text)?;
 
     let captures = captures
       .iter()
       .skip(1)
-      .map(|c| c.map(|m| m.as_str()).unwrap_or(""))
+      .map(|c| c.map(|m| m.as_str()))
       .collect();
 
     Some(captures)
diff --git a/src/testdata/urlpatterntestdata.json b/src/testdata/urlpatterntestdata.json
@@ -354,8 +354,9 @@
   {
     "pattern": [{ "pathname": "/foo/:bar?" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "bar": "" } }
+      "pathname": { "input": "/foo", "groups": { "bar": null } }
     }
   },
   {
@@ -419,8 +420,9 @@
   {
     "pattern": [{ "pathname": "/foo/:bar*" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "bar": "" } }
+      "pathname": { "input": "/foo", "groups": { "bar": null } }
     }
   },
   {
@@ -473,15 +475,17 @@
     "expected_obj": {
       "pathname": "/foo/*?"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
     "pattern": [{ "pathname": "/foo/*?" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
@@ -657,15 +661,17 @@
     "expected_obj": {
       "pathname": "/foo/**"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
     "pattern": [{ "pathname": "/foo/**" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
@@ -1812,9 +1818,10 @@
       "hostname": "(sub.)?example.com",
       "pathname": "/foo"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "https", "groups": {} },
-      "hostname": { "input": "example.com", "groups": { "0": "" } },
+      "hostname": { "input": "example.com", "groups": { "0": null } },
       "pathname": { "input": "/foo", "groups": {} }
     }
   },
@@ -1850,9 +1857,10 @@
       "hostname": "(sub(?:.))?example.com",
       "pathname": "/foo"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "https", "groups": {} },
-      "hostname": { "input": "example.com", "groups": { "0": "" } },
+      "hostname": { "input": "example.com", "groups": { "0": null } },
       "pathname": { "input": "/foo", "groups": {} }
     }
   },
@@ -2299,9 +2307,10 @@
       "protocol": "data",
       "pathname": "text/javascript,let x = 100/:tens?5;"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "data", "groups": {} },
-      "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": "" } }
+      "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": null } }
     }
   },
   {
@@ -2416,7 +2425,6 @@
     "expected_obj": "error"
   },
   {
-    "skip": "bug in rust-url: https://github.com/servo/rust-url/pull/718",
     "pattern": [{ "hostname": "bad|hostname" }],
     "expected_obj": "error"
   },
@@ -2618,13 +2626,15 @@
     }
   },
   {
+    "skip": "only works in ecmascript variety of regex",
     "pattern": [{ "pathname": "*{}**?" }],
     "inputs": [{ "pathname": "foobar" }],
     "expected_obj": {
       "pathname": "*(.*)?"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": "" }}
+      "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": null }}
     }
   },
   {

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,10 @@ impl<R: RegExp> Matcher<R> {`
`49`	`49`	`}`
`50`	`50`	`}`
`51`	`51`
`52`		`- pub fn matches<'a>(&self, mut input: &'a str) -> Option<Vec<&'a str>> {`
	`52`	`+ pub fn matches<'a>(`
	`53`	`+ &self,`
	`54`	`+ mut input: &'a str,`
	`55`	`+ ) -> Option<Vec<Option<&'a str>>> {`
`53`	`56`	`let prefix_len = self.prefix.len();`
`54`	`57`	`let suffix_len = self.suffix.len();`
`55`	`58`	`let input_len = input.len();`
`@@ -82,7 +85,7 @@ impl<R: RegExp> Matcher<R> {`
`82`	`85`	`return None;`
`83`	`86`	`}`
`84`	`87`	`}`
`85`		`- Some(vec![input])`
	`88`	`+ Some(vec![Some(input)])`
`86`	`89`	`}`
`87`	`90`	`InnerMatcher::RegExp { regexp, .. } => {`
`88`	`91`	`regexp.as_ref().unwrap().matches(input)`
Original file line number	Diff line number	Diff line change
`@@ -386,7 +386,7 @@ where`
`386`	`386`	`if name_token.is_some() \|\| regexp_or_wildcard_token.is_some() {`
`387`	`387`	`let mut prefix = String::new();`
`388`	`388`	`if let Some(char_token) = char_token {`
`389`		`- prefix = char_token.value.to_owned();`
	`389`	`+ char_token.value.clone_into(&mut prefix);`
`390`	`390`	`}`
`391`	`391`	`if !prefix.is_empty() && prefix != options.prefix_code_point {`
`392`	`392`	`parser.pending_fixed_value.push_str(&prefix);`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ impl RegExp for EcmaRegexp {`
`176`	`176`	`Ok(EcmaRegexp(pattern.to_string()))`
`177`	`177`	`}`
`178`	`178`
`179`		`- fn matches<'a>(&self, text: &'a str) -> Option<Vec<&'a str>> {`
	`179`	`+ fn matches<'a>(&self, text: &'a str) -> Option<Vec<Option<&'a str>>> {`
`180`	`180`	`let regexp = regex::Regex::parse(&self.0).ok()?;`
`181`	`181`	`regexp.matches(text)`
`182`	`182`	`}`
Original file line number	Diff line number	Diff line change
`@@ -354,8 +354,9 @@`
`354`	`354`	`{`
`355`	`355`	`"pattern": [{ "pathname": "/foo/:bar?" }],`
`356`	`356`	`"inputs": [{ "pathname": "/foo" }],`
	`357`	+ "//": "The `null` below is translated to undefined in the test harness.",
`357`	`358`	`"expected_match": {`
`358`		`- "pathname": { "input": "/foo", "groups": { "bar": "" } }`
	`359`	`+ "pathname": { "input": "/foo", "groups": { "bar": null } }`
`359`	`360`	`}`
`360`	`361`	`},`
`361`	`362`	`{`
`@@ -419,8 +420,9 @@`
`419`	`420`	`{`
`420`	`421`	`"pattern": [{ "pathname": "/foo/:bar*" }],`
`421`	`422`	`"inputs": [{ "pathname": "/foo" }],`
	`423`	+ "//": "The `null` below is translated to undefined in the test harness.",
`422`	`424`	`"expected_match": {`
`423`		`- "pathname": { "input": "/foo", "groups": { "bar": "" } }`
	`425`	`+ "pathname": { "input": "/foo", "groups": { "bar": null } }`
`424`	`426`	`}`
`425`	`427`	`},`
`426`	`428`	`{`
`@@ -473,15 +475,17 @@`
`473`	`475`	`"expected_obj": {`
`474`	`476`	`"pathname": "/foo/*?"`
`475`	`477`	`},`
	`478`	+ "//": "The `null` below is translated to undefined in the test harness.",
`476`	`479`	`"expected_match": {`
`477`		`- "pathname": { "input": "/foo", "groups": { "0": "" } }`
	`480`	`+ "pathname": { "input": "/foo", "groups": { "0": null } }`
`478`	`481`	`}`
`479`	`482`	`},`
`480`	`483`	`{`
`481`	`484`	`"pattern": [{ "pathname": "/foo/*?" }],`
`482`	`485`	`"inputs": [{ "pathname": "/foo" }],`
	`486`	+ "//": "The `null` below is translated to undefined in the test harness.",
`483`	`487`	`"expected_match": {`
`484`		`- "pathname": { "input": "/foo", "groups": { "0": "" } }`
	`488`	`+ "pathname": { "input": "/foo", "groups": { "0": null } }`
`485`	`489`	`}`
`486`	`490`	`},`
`487`	`491`	`{`
`@@ -657,15 +661,17 @@`
`657`	`661`	`"expected_obj": {`
`658`	`662`	`"pathname": "/foo/**"`
`659`	`663`	`},`
	`664`	+ "//": "The `null` below is translated to undefined in the test harness.",
`660`	`665`	`"expected_match": {`
`661`		`- "pathname": { "input": "/foo", "groups": { "0": "" } }`
	`666`	`+ "pathname": { "input": "/foo", "groups": { "0": null } }`
`662`	`667`	`}`
`663`	`668`	`},`
`664`	`669`	`{`
`665`	`670`	`"pattern": [{ "pathname": "/foo/**" }],`
`666`	`671`	`"inputs": [{ "pathname": "/foo" }],`
	`672`	+ "//": "The `null` below is translated to undefined in the test harness.",
`667`	`673`	`"expected_match": {`
`668`		`- "pathname": { "input": "/foo", "groups": { "0": "" } }`
	`674`	`+ "pathname": { "input": "/foo", "groups": { "0": null } }`
`669`	`675`	`}`
`670`	`676`	`},`
`671`	`677`	`{`
`@@ -1812,9 +1818,10 @@`
`1812`	`1818`	`"hostname": "(sub.)?example.com",`
`1813`	`1819`	`"pathname": "/foo"`
`1814`	`1820`	`},`
	`1821`	+ "//": "The `null` below is translated to undefined in the test harness.",
`1815`	`1822`	`"expected_match": {`
`1816`	`1823`	`"protocol": { "input": "https", "groups": {} },`
`1817`		`- "hostname": { "input": "example.com", "groups": { "0": "" } },`
	`1824`	`+ "hostname": { "input": "example.com", "groups": { "0": null } },`
`1818`	`1825`	`"pathname": { "input": "/foo", "groups": {} }`
`1819`	`1826`	`}`
`1820`	`1827`	`},`
`@@ -1850,9 +1857,10 @@`
`1850`	`1857`	`"hostname": "(sub(?:.))?example.com",`
`1851`	`1858`	`"pathname": "/foo"`
`1852`	`1859`	`},`
	`1860`	+ "//": "The `null` below is translated to undefined in the test harness.",
`1853`	`1861`	`"expected_match": {`
`1854`	`1862`	`"protocol": { "input": "https", "groups": {} },`
`1855`		`- "hostname": { "input": "example.com", "groups": { "0": "" } },`
	`1863`	`+ "hostname": { "input": "example.com", "groups": { "0": null } },`
`1856`	`1864`	`"pathname": { "input": "/foo", "groups": {} }`
`1857`	`1865`	`}`
`1858`	`1866`	`},`
`@@ -2299,9 +2307,10 @@`
`2299`	`2307`	`"protocol": "data",`
`2300`	`2308`	`"pathname": "text/javascript,let x = 100/:tens?5;"`
`2301`	`2309`	`},`
	`2310`	+ "//": "The `null` below is translated to undefined in the test harness.",
`2302`	`2311`	`"expected_match": {`
`2303`	`2312`	`"protocol": { "input": "data", "groups": {} },`
`2304`		`- "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": "" } }`
	`2313`	`+ "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": null } }`
`2305`	`2314`	`}`
`2306`	`2315`	`},`
`2307`	`2316`	`{`
`@@ -2416,7 +2425,6 @@`
`2416`	`2425`	`"expected_obj": "error"`
`2417`	`2426`	`},`
`2418`	`2427`	`{`
`2419`		`- "skip": "bug in rust-url: https://github.com/servo/rust-url/pull/718",`
`2420`	`2428`	`"pattern": [{ "hostname": "bad\|hostname" }],`
`2421`	`2429`	`"expected_obj": "error"`
`2422`	`2430`	`},`
`@@ -2618,13 +2626,15 @@`
`2618`	`2626`	`}`
`2619`	`2627`	`},`
`2620`	`2628`	`{`
	`2629`	`+ "skip": "only works in ecmascript variety of regex",`
`2621`	`2630`	`"pattern": [{ "pathname": "{}*?" }],`
`2622`	`2631`	`"inputs": [{ "pathname": "foobar" }],`
`2623`	`2632`	`"expected_obj": {`
`2624`	`2633`	`"pathname": "(.)?"`
`2625`	`2634`	`},`
	`2635`	+ "//": "The `null` below is translated to undefined in the test harness.",
`2626`	`2636`	`"expected_match": {`
`2627`		`- "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": "" }}`
	`2637`	`+ "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": null }}`
`2628`	`2638`	`}`
`2629`	`2639`	`},`
`2630`	`2640`	`{`