diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-17 07:13:02 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-17 07:13:02 +0900 |
| commit | c7aa10384a2548167466b4c61f9eff29ed8611f6 (patch) | |
| tree | 4d0800f425eaea2a8fb8ee3f551f865f570d19f8 | |
| parent | 890c8213d15424bb714f2ff06d562ca2538c24e4 (diff) | |
| download | php-shirabe-c7aa10384a2548167466b4c61f9eff29ed8611f6.tar.gz php-shirabe-c7aa10384a2548167466b4c61f9eff29ed8611f6.tar.zst php-shirabe-c7aa10384a2548167466b4c61f9eff29ed8611f6.zip | |
refactor(preg): report unmatched groups as null throughout
The shim carried two reporting modes for the preg_* $matches maps: PHP's
default (trailing unmatched groups dropped, interior ones ""), and the
PREG_UNMATCHED_AS_NULL form, picked by calling a *_unmatched_as_null()
variant. The regex crate hands out Option<Match>, which maps onto the
null form directly, and no caller distinguished a dropped group from a
null one -- preg_match() and preg_match_all2() already reported nulls
unconditionally. Keep only the null form; the shim API no longer mirrors
PHP's flag set, which is intended.
Preg::is_match_with_indexed_captures() modelled PHP's "unset" as a
truncated Vec<String>, and now returns Vec<Option<String>>. That is what
Composer actually does: Preg::isMatch() always sets
PREG_UNMATCHED_AS_NULL, and its callers test groups with `!== null`.
preg_match_all(), preg_match_all_set_order() and
preg_split_delim_capture() still hand back Vec<String> and keep the ""
form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| -rw-r--r-- | crates/shirabe-pcre/src/preg.rs | 50 | ||||
| -rw-r--r-- | crates/shirabe-php-shim/src/preg.rs | 87 | ||||
| -rw-r--r-- | crates/shirabe/src/command/fund_command.rs | 2 | ||||
| -rw-r--r-- | crates/shirabe/src/command/update_command.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/src/installer/binary_installer.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/src/package/version/version_guesser.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/src/repository/vcs/svn_driver.rs | 12 | ||||
| -rw-r--r-- | crates/shirabe/src/util/hg.rs | 2 | ||||
| -rw-r--r-- | crates/shirabe/src/util/platform.rs | 4 |
9 files changed, 41 insertions, 134 deletions
diff --git a/crates/shirabe-pcre/src/preg.rs b/crates/shirabe-pcre/src/preg.rs index 3a997ad3..aa986ae8 100644 --- a/crates/shirabe-pcre/src/preg.rs +++ b/crates/shirabe-pcre/src/preg.rs @@ -13,9 +13,8 @@ pub use shirabe_php_shim::{CaptureKey, PregMatches, PregMatchesAll, PregMatchesAllWithOffsets}; use shirabe_php_shim::{ - PregPattern, preg_grep, preg_match_all_offset_capture_unmatched_as_null, preg_match_all2, - preg_match_map, preg_match2, preg_match2_unmatched_as_null, preg_replace_callback, - preg_replace2, + PregPattern, preg_grep, preg_match_all_offset_capture, preg_match_all2, preg_match_map, + preg_match2, preg_replace_callback, preg_replace2, }; preg_match_map! { @@ -48,7 +47,7 @@ impl Preg { offset: usize, ) -> bool { let mut internal = PregMatches::new(); - let result = preg_match2_unmatched_as_null(pattern, subject, &mut internal, offset); + let result = preg_match2(pattern, subject, &mut internal, offset); if let Some(out) = matches { *out = drop_null_matches(internal); @@ -76,8 +75,7 @@ impl Preg { matches: Option<&mut PregMatchesAllWithOffsets>, ) -> usize { let mut internal = PregMatchesAllWithOffsets::new(); - let result = - preg_match_all_offset_capture_unmatched_as_null(pattern, subject, &mut internal); + let result = preg_match_all_offset_capture(pattern, subject, &mut internal); if let Some(out) = matches { *out = internal; @@ -153,7 +151,7 @@ impl Preg { matches: &mut PregNamedGroups, ) -> bool { let mut internal = PregMatches::new(); - let result = preg_match2_unmatched_as_null(pattern, subject, &mut internal, 0); + let result = preg_match2(pattern, subject, &mut internal, 0); matches.clear(); for (key, value) in internal { @@ -165,38 +163,26 @@ impl Preg { result } + /// `is_match3` with the groups positioned by number rather than keyed, for callers that only + /// read numbered groups. Index 0 is the full match; an unmatched group is `None`. pub fn is_match_with_indexed_captures( pattern: impl PregPattern, subject: &str, - ) -> Option<Vec<String>> { - // Classic preg_match semantics (no PREG_UNMATCHED_AS_NULL): trailing - // unmatched groups are truncated, interior unmatched groups become "". + ) -> Option<Vec<Option<String>>> { let mut internal = PregMatches::new(); - let result = preg_match2(pattern, subject, &mut internal, 0); - - if !result { + if !preg_match2(pattern, subject, &mut internal, 0) { return None; } - let max_index = internal - .keys() - .filter_map(|key| match key { - CaptureKey::ByIndex(index) => Some(*index), - CaptureKey::ByName(_) => None, - }) - .max() - .unwrap_or(0); - - let mut captures = Vec::with_capacity(max_index + 1); - for index in 0..=max_index { - let value = internal - .get(&CaptureKey::ByIndex(index)) - .and_then(|value| value.clone()) - .unwrap_or_default(); - captures.push(value); - } - - Some(captures) + Some( + internal + .into_iter() + .filter_map(|(key, value)| match key { + CaptureKey::ByIndex(_) => Some(value), + CaptureKey::ByName(_) => None, + }) + .collect(), + ) } pub fn is_match_all( diff --git a/crates/shirabe-php-shim/src/preg.rs b/crates/shirabe-php-shim/src/preg.rs index 6b53bb59..0aa2c43e 100644 --- a/crates/shirabe-php-shim/src/preg.rs +++ b/crates/shirabe-php-shim/src/preg.rs @@ -36,10 +36,6 @@ macro_rules! preg_match_map { self.0.insert(key, value) } - pub fn keys(&self) -> ::indexmap::map::Keys<'_, $key, $value> { - self.0.keys() - } - pub fn iter(&self) -> ::indexmap::map::Iter<'_, $key, $value> { self.0.iter() } @@ -84,7 +80,7 @@ macro_rules! preg_match_map { preg_match_map! { /// A single match's `$matches`, keyed by both the named and the numbered form of each capture - /// group. A `None` value is a group the caller's flags reported as unmatched. + /// group. A `None` value is a group that did not participate in the match. pub struct PregMatches(CaptureKey => Option<String>); } @@ -149,26 +145,6 @@ pub fn preg_match2( matches: &mut PregMatches, offset: usize, ) -> bool { - preg_match2_impl(pattern, subject, matches, offset, false) -} - -// PREG_UNMATCHED_AS_NULL counterpart of preg_match2(). -pub fn preg_match2_unmatched_as_null( - pattern: impl PregPattern, - subject: &str, - matches: &mut PregMatches, - offset: usize, -) -> bool { - preg_match2_impl(pattern, subject, matches, offset, true) -} - -fn preg_match2_impl( - pattern: impl PregPattern, - subject: &str, - matches: &mut PregMatches, - offset: usize, - unmatched_as_null: bool, -) -> bool { let __resolved = pattern.resolve(); let (re, anchored) = __resolved.parts(); // An anchored (`A`) pattern must match starting exactly at `offset`; the `regex` crate cannot @@ -184,11 +160,7 @@ fn preg_match2_impl( matches.clear(); if let Some(caps) = &caps { let names: Vec<Option<&str>> = re.capture_names().collect(); - *matches = if unmatched_as_null { - single_match_map_unmatched_as_null(caps, &names) - } else { - single_match_map(caps, &names) - }; + *matches = single_match_map(caps, &names); } caps.is_some() @@ -263,31 +235,12 @@ pub fn preg_match_all_set_order( count } -// A non-participating group is reported at offset -1, holding "". +// A non-participating group is reported as None, at offset -1. pub fn preg_match_all_offset_capture( pattern: impl PregPattern, subject: &str, matches: &mut PregMatchesAllWithOffsets, ) -> usize { - preg_match_all_offset_capture_impl(pattern, subject, matches, false) -} - -// PREG_UNMATCHED_AS_NULL counterpart of preg_match_all_offset_capture(): a -// non-participating group holds null instead of "". -pub fn preg_match_all_offset_capture_unmatched_as_null( - pattern: impl PregPattern, - subject: &str, - matches: &mut PregMatchesAllWithOffsets, -) -> usize { - preg_match_all_offset_capture_impl(pattern, subject, matches, true) -} - -fn preg_match_all_offset_capture_impl( - pattern: impl PregPattern, - subject: &str, - matches: &mut PregMatchesAllWithOffsets, - unmatched_as_null: bool, -) -> usize { let __resolved = pattern.resolve(); let (re, _anchored) = __resolved.parts(); let group_count = re.captures_len(); @@ -300,8 +253,7 @@ fn preg_match_all_offset_capture_impl( for (g, column) in groups.iter_mut().enumerate() { let entry = match caps.get(g) { Some(m) => (Some(m.as_str().to_string()), m.start() as i64), - None if unmatched_as_null => (None, -1), - None => (Some(String::new()), -1), + None => (None, -1), }; column.push(entry); } @@ -706,37 +658,10 @@ fn php_match_row(caps: ®ex::Captures) -> Vec<String> { } // Builds a single match's `$matches` map with both named and numbered keys -// (the named key precedes its number). Trailing unmatched groups are dropped -// and interior ones become "". +// (the named key precedes its number). Every group is present; a +// non-participating one is None. fn single_match_map(caps: ®ex::Captures, names: &[Option<&str>]) -> PregMatches { let mut out = PregMatches::new(); - let group_count = caps.len(); - let last_participating = (0..group_count).rev().find(|&i| caps.get(i).is_some()); - - for i in 0..group_count { - let m = caps.get(i); - if m.is_none() - && let Some(last) = last_participating - && i > last - { - break; - } - let value = Some(m.map(|m| m.as_str().to_string()).unwrap_or_default()); - if let Some(Some(name)) = names.get(i) { - out.insert(CaptureKey::ByName((*name).to_string()), value.clone()); - } - out.insert(CaptureKey::ByIndex(i), value); - } - out -} - -// PREG_UNMATCHED_AS_NULL counterpart of single_match_map(): every group is -// present and non-participating ones are None. -fn single_match_map_unmatched_as_null( - caps: ®ex::Captures, - names: &[Option<&str>], -) -> PregMatches { - let mut out = PregMatches::new(); for i in 0..caps.len() { let value = caps.get(i).map(|m| m.as_str().to_string()); diff --git a/crates/shirabe/src/command/fund_command.rs b/crates/shirabe/src/command/fund_command.rs index 7f2dd24c..109943d6 100644 --- a/crates/shirabe/src/command/fund_command.rs +++ b/crates/shirabe/src/command/fund_command.rs @@ -67,7 +67,7 @@ impl FundCommand { php_regex!(r"{^https://github.com/([^/]+)$}"), &url, ) - && let Some(sponsor) = matches.into_iter().nth(1) + && let Some(sponsor) = matches.into_iter().nth(1).flatten() { url = format!("https://github.com/sponsors/{}", sponsor); } diff --git a/crates/shirabe/src/command/update_command.rs b/crates/shirabe/src/command/update_command.rs index 38a79e30..2eca6db7 100644 --- a/crates/shirabe/src/command/update_command.rs +++ b/crates/shirabe/src/command/update_command.rs @@ -466,10 +466,8 @@ impl Command for UpdateCommand { let Some(matches) = matches else { continue; }; - let constraint = parser.parse_constraints(&format!( - "~{}", - matches.get(1).cloned().unwrap_or_default() - ))?; + let constraint = parser + .parse_constraints(&format!("~{}", matches[1].clone().unwrap_or_default()))?; if let Some(existing) = temporary_constraints.get(&package.get_name()) { temporary_constraints.insert( package.get_name(), diff --git a/crates/shirabe/src/installer/binary_installer.rs b/crates/shirabe/src/installer/binary_installer.rs index d5ece82f..606e943f 100644 --- a/crates/shirabe/src/installer/binary_installer.rs +++ b/crates/shirabe/src/installer/binary_installer.rs @@ -328,10 +328,10 @@ impl BinaryInstaller { &bin_contents, ) { // carry over the existing shebang if present, otherwise add our own - let proxy_code = if m.get(1).is_none() { + let proxy_code = if m[1].is_none() { "#!/usr/bin/env php".to_string() } else { - trim(m.get(1).map(|s| s.as_str()).unwrap_or(""), None) + trim(m[1].as_deref().unwrap_or(""), None) }; let bin_path_exported = self .filesystem @@ -377,7 +377,7 @@ impl BinaryInstaller { $data = str_replace('__FILE__', var_export($this->realpath, true), $data);" .to_string(); } - if trim(m.first().map(|s| s.as_str()).unwrap_or(""), None) != "<?php" { + if trim(m[0].as_deref().unwrap_or(""), None) != "<?php" { stream_hint = " using a stream wrapper to prevent the shebang from being output on PHP<8\n *" .to_string(); diff --git a/crates/shirabe/src/package/version/version_guesser.rs b/crates/shirabe/src/package/version/version_guesser.rs index 4f46919e..4b5187f2 100644 --- a/crates/shirabe/src/package/version/version_guesser.rs +++ b/crates/shirabe/src/package/version/version_guesser.rs @@ -704,9 +704,9 @@ impl VersionGuesser { ); if let Some(matches) = Preg::is_match_with_indexed_captures(&url_pattern, &output) { - let m1 = matches.get(1).cloned().unwrap_or_default(); - let m2 = matches.get(2).cloned(); - let m3 = matches.get(3).cloned(); + let m1 = matches[1].clone().unwrap_or_default(); + let m2 = matches[2].clone(); + let m3 = matches[3].clone(); if let Some(m2) = m2.as_ref() && let Some(m3) = m3.as_ref() && (branches_path == *m2 || tags_path == *m2) diff --git a/crates/shirabe/src/repository/vcs/svn_driver.rs b/crates/shirabe/src/repository/vcs/svn_driver.rs index 297e8f33..db8c43f0 100644 --- a/crates/shirabe/src/repository/vcs/svn_driver.rs +++ b/crates/shirabe/src/repository/vcs/svn_driver.rs @@ -260,10 +260,10 @@ impl SvnDriver { let (path, rev) = if let Some(m) = Preg::is_match_with_indexed_captures(php_regex!(r"{^(.+?)(@\d+)?/$}"), &identifier) { - if m.get(2).is_some() { + if m[2].is_some() { ( - m.get(1).cloned().unwrap_or_default(), - m.get(2).cloned().unwrap_or_default(), + m[1].clone().unwrap_or_default(), + m[2].clone().unwrap_or_default(), ) } else { (identifier.clone(), String::new()) @@ -300,10 +300,10 @@ impl SvnDriver { let (path, rev) = if let Some(m) = Preg::is_match_with_indexed_captures(php_regex!(r"{^(.+?)(@\d+)?/$}"), &identifier) { - if m.get(2).is_some() { + if m[2].is_some() { ( - m.get(1).cloned().unwrap_or_default(), - m.get(2).cloned().unwrap_or_default(), + m[1].clone().unwrap_or_default(), + m[2].clone().unwrap_or_default(), ) } else { (identifier.clone(), String::new()) diff --git a/crates/shirabe/src/util/hg.rs b/crates/shirabe/src/util/hg.rs index 8570af6d..d9e58623 100644 --- a/crates/shirabe/src/util/hg.rs +++ b/crates/shirabe/src/util/hg.rs @@ -158,7 +158,7 @@ impl Hg { &output, ) { - return matches.into_iter().nth(1); + return matches.into_iter().nth(1).flatten(); } None }) diff --git a/crates/shirabe/src/util/platform.rs b/crates/shirabe/src/util/platform.rs index 21ad869c..629854b6 100644 --- a/crates/shirabe/src/util/platform.rs +++ b/crates/shirabe/src/util/platform.rs @@ -95,14 +95,12 @@ impl Platform { // Regex pattern compatibility: // The original pattern uses a conditional subpattern to make the trailing `%` required // only for the `%VAR%` form. The Rust regex crate does not support conditionals, so the - // two forms are written as an explicit alternation: `$VAR` or `%VAR%`. The branch that did - // not participate is reported as an empty string, which `\w+` can never capture. + // two forms are written as an explicit alternation: `$VAR` or `%VAR%`. Preg::replace_callback( php_regex!(r"#^(?:\$(?P<dvar>\w+)|%(?P<pvar>\w+)%)(?P<path>.*)#"), |matches: &PregMatchedGroups| -> String { let var = matches .get(&CaptureKey::ByName("dvar".to_string())) - .filter(|dvar| !dvar.is_empty()) .or_else(|| matches.get(&CaptureKey::ByName("pvar".to_string()))) .map(|s| s.as_str()) .unwrap_or(""); |
