From 5114a8199a87c9e5584d92848e95deba22b73e98 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 18 Aug 2026 01:56:31 +0900 Subject: refactor(preg): back PregMatches with regex::Captures PregMatches was an IndexMap of owned Strings copied out of the match, so every preg_match2/preg_replace_callback call allocated a String per capture group (twice over for a named group) whether or not the caller read it. It now wraps the regex::Captures itself, held alongside the pattern it came from so groups stay reachable by both their named and their numbered form, and hands out &str borrowed from the subject. The subject's lifetime becomes a parameter of the type. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe-pcre/src/preg.rs | 10 +- crates/shirabe-php-shim/src/preg.rs | 130 +++++++++++---------- .../src/formatter/output_formatter.rs | 4 +- .../src/helper/progress_bar.rs | 12 +- .../src/input/string_input.rs | 22 ++-- crates/shirabe-symfony-process/src/process.rs | 12 +- crates/shirabe/src/command/config_command.rs | 40 ++++--- crates/shirabe/src/console/application.rs | 4 +- .../src/event_dispatcher/event_dispatcher.rs | 22 +++- 9 files changed, 149 insertions(+), 107 deletions(-) diff --git a/crates/shirabe-pcre/src/preg.rs b/crates/shirabe-pcre/src/preg.rs index 25feafc9..f3bfeac0 100644 --- a/crates/shirabe-pcre/src/preg.rs +++ b/crates/shirabe-pcre/src/preg.rs @@ -118,9 +118,9 @@ impl Preg { pub fn is_match_named(pattern: impl PregPattern, subject: &str) -> Option { Some( preg_match2(pattern, subject, 0)? - .into_iter() + .iter() .filter_map(|(key, value)| match (key, value) { - (CaptureKey::ByName(name), Some(value)) => Some((name, value)), + (CaptureKey::ByName(name), Some(value)) => Some((name, value.to_string())), _ => None, }) .collect(), @@ -135,9 +135,9 @@ impl Preg { ) -> Option>> { Some( preg_match2(pattern, subject, 0)? - .into_iter() + .iter() .filter_map(|(key, value)| match key { - CaptureKey::ByIndex(_) => Some(value), + CaptureKey::ByIndex(_) => Some(value.map(str::to_string)), CaptureKey::ByName(_) => None, }) .collect(), @@ -161,6 +161,6 @@ impl Preg { fn drop_null_matches(matches: &PregMatches) -> PregMatchedGroups { matches .iter() - .filter_map(|(key, value)| value.clone().map(|value| (key.clone(), value))) + .filter_map(|(key, value)| value.map(|value| (key, value.to_string()))) .collect() } diff --git a/crates/shirabe-php-shim/src/preg.rs b/crates/shirabe-php-shim/src/preg.rs index e758fd20..74b7f12d 100644 --- a/crates/shirabe-php-shim/src/preg.rs +++ b/crates/shirabe-php-shim/src/preg.rs @@ -41,26 +41,6 @@ macro_rules! preg_match_map { } } - impl ::std::ops::Index<&Q> for $name - where - Q: ?Sized + ::std::hash::Hash + ::indexmap::Equivalent<$key>, - { - type Output = $value; - - fn index(&self, key: &Q) -> &$value { - &self.0[key] - } - } - - /// Looks a group up by its position in the map rather than by key, as `IndexMap` does. - impl ::std::ops::Index for $name { - type Output = $value; - - fn index(&self, position: usize) -> &$value { - &self.0[position] - } - } - impl IntoIterator for $name { type Item = ($key, $value); type IntoIter = ::indexmap::map::IntoIter<$key, $value>; @@ -78,10 +58,44 @@ 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 that did not participate in the match. - pub struct PregMatches(CaptureKey => Option); +/// A single match's `$matches`: the `regex::Captures` the search produced, held alongside the +/// pattern that produced it so groups can be read by both their named and their numbered form. +/// `'h` is the lifetime of the searched subject, which the group values borrow from. +#[derive(Debug)] +pub struct PregMatches<'h> { + pattern: ResolvedPattern, + caps: regex::Captures<'h>, +} + +impl<'h> PregMatches<'h> { + fn new(pattern: ResolvedPattern, caps: regex::Captures<'h>) -> Self { + Self { pattern, caps } + } + + /// The value of the group `key` names, or `None` if that group did not participate in the + /// match. A group the pattern does not have reads as `None` too, matching how PHP reports a + /// `$matches` entry that is not there. + pub fn get(&self, key: &CaptureKey) -> Option<&'h str> { + let group = match key { + CaptureKey::ByIndex(index) => self.caps.get(*index), + CaptureKey::ByName(name) => self.caps.name(name), + }; + group.map(|group| group.as_str()) + } + + /// Every capture group under both its named and its numbered key (the name preceding its + /// number), in the order PHP fills `$matches` in. + pub fn iter(&self) -> impl Iterator)> + '_ { + let (re, _anchored) = self.pattern.parts(); + re.capture_names() + .enumerate() + .flat_map(move |(index, name)| { + let value = self.caps.get(index).map(|group| group.as_str()); + name.map(|name| (CaptureKey::ByName(name.to_string()), value)) + .into_iter() + .chain(std::iter::once((CaptureKey::ByIndex(index), value))) + }) + } } preg_match_map! { @@ -99,14 +113,18 @@ preg_match_map! { impl PregMatchesAll { /// The number PHP's `preg_match_all` returns: every column holds one entry per occurrence. pub fn occurrence_count(&self) -> usize { - self[&CaptureKey::ByIndex(0)].len() + self.get(&CaptureKey::ByIndex(0)) + .expect("group 0 is always present") + .len() } } impl PregMatchesAllWithOffsets { /// The number PHP's `preg_match_all` returns: every column holds one entry per occurrence. pub fn occurrence_count(&self) -> usize { - self[&CaptureKey::ByIndex(0)].len() + self.get(&CaptureKey::ByIndex(0)) + .expect("group 0 is always present") + .len() } } @@ -144,23 +162,27 @@ pub fn preg_match(pattern: impl PregPattern, subject: &str) -> Option Option { +// Returns None if the pattern did not match; otherwise the match's capture groups. +pub fn preg_match2<'h>( + pattern: impl PregPattern, + subject: &'h str, + offset: usize, +) -> Option> { let __resolved = pattern.resolve(); - let (re, anchored) = __resolved.parts(); - // An anchored (`A`) pattern must match starting exactly at `offset`; the `regex` crate cannot - // anchor a `captures_at` search, so search the sub-slice beginning at `offset` and require the - // match to start at its head. - let caps = if anchored { - re.captures(&subject[offset..]) - .filter(|c| c.get(0).map(|m| m.start()) == Some(0)) - } else { - re.captures_at(subject, offset) + let caps = { + let (re, anchored) = __resolved.parts(); + // An anchored (`A`) pattern must match starting exactly at `offset`; the `regex` crate + // cannot anchor a `captures_at` search, so search the sub-slice beginning at `offset` and + // require the match to start at its head. + if anchored { + re.captures(&subject[offset..]) + .filter(|c| c.get(0).map(|m| m.start()) == Some(0)) + } else { + re.captures_at(subject, offset) + } }?; - let names: Vec> = re.capture_names().collect(); - Some(single_match_map(&caps, &names)) + Some(PregMatches::new(__resolved, caps)) } // PREG_PATTERN_ORDER: the outer vec is indexed by capture group, the inner by @@ -343,25 +365,24 @@ pub fn preg_replace2( String::from_utf8_lossy(&out).into_owned() } -pub fn preg_replace_callback( +pub fn preg_replace_callback<'h, F>( pattern: impl PregPattern, mut callback: F, - subject: &str, + subject: &'h str, ) -> anyhow::Result where - F: FnMut(&PregMatches) -> anyhow::Result, + F: FnMut(&PregMatches<'h>) -> anyhow::Result, { let __resolved = pattern.resolve(); let (re, _anchored) = __resolved.parts(); - let names: Vec> = re.capture_names().collect(); let mut out: Vec = Vec::new(); let mut last = 0usize; for caps in re.captures_iter(subject) { let m = caps.get(0).unwrap(); out.extend_from_slice(&subject.as_bytes()[last..m.start()]); - let map = single_match_map(&caps, &names); - out.extend_from_slice(callback(&map)?.as_bytes()); + let matches = PregMatches::new(__resolved.clone(), caps); + out.extend_from_slice(callback(&matches)?.as_bytes()); last = m.end(); } out.extend_from_slice(&subject.as_bytes()[last..]); @@ -482,6 +503,7 @@ fn translate_php_pattern(pattern: &str) -> anyhow::Result<(String, bool)> { /// `LazyLock`) rather than an owned `regex::Regex` — `regex::Regex::clone()` does not share /// the underlying meta engine's search-cache pool, so producing a fresh owned clone here would pay /// a ~10us per-call cache warmup cost regardless of which path produced it (measured). +#[derive(Debug, Clone)] pub enum ResolvedPattern { Cached(Arc<(regex::Regex, bool)>), Static(&'static regex::Regex, bool), @@ -629,19 +651,3 @@ fn php_replacement_group(bytes: &[u8]) -> (usize, usize) { } (group, consumed) } - -// Builds a single match's `$matches` map with both named and numbered keys -// (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(); - - for i in 0..caps.len() { - let value = caps.get(i).map(|m| m.as_str().to_string()); - if let Some(Some(name)) = names.get(i) { - out.insert(CaptureKey::ByName((*name).to_string()), value.clone()); - } - out.insert(CaptureKey::ByIndex(i), value); - } - out -} diff --git a/crates/shirabe-symfony-console/src/formatter/output_formatter.rs b/crates/shirabe-symfony-console/src/formatter/output_formatter.rs index 1153bc9e..571d66c0 100644 --- a/crates/shirabe-symfony-console/src/formatter/output_formatter.rs +++ b/crates/shirabe-symfony-console/src/formatter/output_formatter.rs @@ -321,7 +321,9 @@ impl WrappableOutputFormatterInterface for OutputFormatter { // opening tag? let open = shirabe_php_shim::byte_at(&text, 1) != b'/'; let tag = if open { - matches[&CaptureKey::ByIndex(1)][i] + matches + .get(&CaptureKey::ByIndex(1)) + .expect("group 1 exists in the tag pattern")[i] .0 .clone() .expect("group 1 participates whenever the pattern matches") diff --git a/crates/shirabe-symfony-console/src/helper/progress_bar.rs b/crates/shirabe-symfony-console/src/helper/progress_bar.rs index 4764d617..7a7a3c53 100644 --- a/crates/shirabe-symfony-console/src/helper/progress_bar.rs +++ b/crates/shirabe-symfony-console/src/helper/progress_bar.rs @@ -799,7 +799,10 @@ impl ProgressBar { // $callback in PHP, expressed as a closure over $this and the matches. let callback = |matches: &PregMatches| -> anyhow::Result { - let name = matches[&CaptureKey::ByIndex(1)].clone().unwrap_or_default(); + let name = matches + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); let text: shirabe_php_shim::PhpMixed = if Self::get_placeholder_formatter_definition(&name).is_some() { @@ -813,10 +816,13 @@ impl ProgressBar { } else if let Some(message) = self.messages.get(&name) { shirabe_php_shim::PhpMixed::String(message.clone()) } else { - return Ok(matches[&CaptureKey::ByIndex(0)].clone().unwrap_or_default()); + return Ok(matches + .get(&CaptureKey::ByIndex(0)) + .unwrap_or_default() + .to_string()); }; - if let Some(modifier) = matches.get(&CaptureKey::ByIndex(2)).and_then(|m| m.clone()) { + if let Some(modifier) = matches.get(&CaptureKey::ByIndex(2)) { return Ok(shirabe_php_shim::sprintf(&format!("%{modifier}"), &[text])); } diff --git a/crates/shirabe-symfony-console/src/input/string_input.rs b/crates/shirabe-symfony-console/src/input/string_input.rs index b0c3b159..0dcc1a9d 100644 --- a/crates/shirabe-symfony-console/src/input/string_input.rs +++ b/crates/shirabe-symfony-console/src/input/string_input.rs @@ -61,15 +61,14 @@ impl StringInput { if token.is_some() { tokens.push(token.take().unwrap()); } - cursor += - shirabe_php_shim::strlen(m[&CaptureKey::ByIndex(0)].as_deref().unwrap_or("")); + cursor += shirabe_php_shim::strlen(m.get(&CaptureKey::ByIndex(0)).unwrap_or("")); } else if let Some(m) = preg_match2( format!(r#"/([^="'\s]+?)(=?)({}+)/A"#, Self::REGEX_QUOTED_STRING), input, cursor as usize, ) { let inner = shirabe_php_shim::substr( - m[&CaptureKey::ByIndex(3)].as_deref().unwrap_or(""), + m.get(&CaptureKey::ByIndex(3)).unwrap_or(""), 1, Some(-1), ); @@ -78,12 +77,11 @@ impl StringInput { token = Some(format!( "{}{}{}{}", token.unwrap_or_default(), - m[&CaptureKey::ByIndex(1)].as_deref().unwrap_or(""), - m[&CaptureKey::ByIndex(2)].as_deref().unwrap_or(""), + m.get(&CaptureKey::ByIndex(1)).unwrap_or(""), + m.get(&CaptureKey::ByIndex(2)).unwrap_or(""), shirabe_php_shim::stripcslashes(&replaced) )); - cursor += - shirabe_php_shim::strlen(m[&CaptureKey::ByIndex(0)].as_deref().unwrap_or("")); + cursor += shirabe_php_shim::strlen(m.get(&CaptureKey::ByIndex(0)).unwrap_or("")); } else if let Some(m) = preg_match2( format!(r"/{}/A", Self::REGEX_QUOTED_STRING), input, @@ -93,13 +91,12 @@ impl StringInput { "{}{}", token.unwrap_or_default(), shirabe_php_shim::stripcslashes(&shirabe_php_shim::substr( - m[&CaptureKey::ByIndex(0)].as_deref().unwrap_or(""), + m.get(&CaptureKey::ByIndex(0)).unwrap_or(""), 1, Some(-1) )) )); - cursor += - shirabe_php_shim::strlen(m[&CaptureKey::ByIndex(0)].as_deref().unwrap_or("")); + cursor += shirabe_php_shim::strlen(m.get(&CaptureKey::ByIndex(0)).unwrap_or("")); } else if let Some(m) = preg_match2( format!(r"/{}/A", Self::REGEX_UNQUOTED_STRING), input, @@ -108,10 +105,9 @@ impl StringInput { token = Some(format!( "{}{}", token.unwrap_or_default(), - m[&CaptureKey::ByIndex(1)].as_deref().unwrap_or("") + m.get(&CaptureKey::ByIndex(1)).unwrap_or("") )); - cursor += - shirabe_php_shim::strlen(m[&CaptureKey::ByIndex(0)].as_deref().unwrap_or("")); + cursor += shirabe_php_shim::strlen(m.get(&CaptureKey::ByIndex(0)).unwrap_or("")); } else { // should never happen return Err(InvalidArgumentException::new(format!( diff --git a/crates/shirabe-symfony-process/src/process.rs b/crates/shirabe-symfony-process/src/process.rs index 96853e90..0f7d3814 100644 --- a/crates/shirabe-symfony-process/src/process.rs +++ b/crates/shirabe-symfony-process/src/process.rs @@ -939,8 +939,11 @@ impl Process { ) | [^"]*+ )"/x"# ), |m: &PregMatches| -> anyhow::Result { - let m0 = m[&CaptureKey::ByIndex(0)].clone().unwrap_or_default(); - let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().flatten(); + let m0 = m + .get(&CaptureKey::ByIndex(0)) + .unwrap_or_default() + .to_string(); + let m1 = m.get(&CaptureKey::ByIndex(1)).map(str::to_string); if m1.is_none() { return Ok(m0); } @@ -1071,9 +1074,8 @@ impl Process { |matches: &PregMatches| -> anyhow::Result { let key = matches .get(&CaptureKey::ByIndex(1)) - .cloned() - .flatten() - .unwrap_or_default(); + .unwrap_or_default() + .to_string(); match env.get(&key) { None => Err(InvalidArgumentException::new(format!( "Command line is missing a value for parameter \"{}\": {}", diff --git a/crates/shirabe/src/command/config_command.rs b/crates/shirabe/src/command/config_command.rs index 86375f49..35cd2ab6 100644 --- a/crates/shirabe/src/command/config_command.rs +++ b/crates/shirabe/src/command/config_command.rs @@ -1038,7 +1038,7 @@ impl Command for ConfigCommand { .borrow_mut() .as_mut() .unwrap() - .remove_repository(&matches[1]); + .remove_repository(matches.get(&CaptureKey::ByIndex(1)).unwrap()); return Ok(0); } @@ -1052,7 +1052,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_repository( - &matches[1], + matches.get(&CaptureKey::ByIndex(1)).unwrap(), PhpMixed::Array(repo), input.borrow().get_option("append")?.as_bool() == Some(true), ); @@ -1072,7 +1072,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_repository( - &matches[1], + matches.get(&CaptureKey::ByIndex(1)).unwrap(), PhpMixed::Bool(false), input.borrow().get_option("append")?.as_bool() == Some(true), ); @@ -1086,7 +1086,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_repository( - &matches[1], + matches.get(&CaptureKey::ByIndex(1)).unwrap(), value, input.borrow().get_option("append")?.as_bool() == Some(true), ); @@ -1334,18 +1334,30 @@ impl Command for ConfigCommand { .borrow_mut() .as_mut() .unwrap() - .remove_config_setting(&format!("{}.{}", matches[1], matches[2])); + .remove_config_setting(&format!( + "{}.{}", + matches.get(&CaptureKey::ByIndex(1)).unwrap(), + matches.get(&CaptureKey::ByIndex(2)).unwrap() + )); self.config_source .borrow_mut() .as_mut() .unwrap() - .remove_config_setting(&format!("{}.{}", matches[1], matches[2])); + .remove_config_setting(&format!( + "{}.{}", + matches.get(&CaptureKey::ByIndex(1)).unwrap(), + matches.get(&CaptureKey::ByIndex(2)).unwrap() + )); return Ok(0); } - let key = format!("{}.{}", matches[1], matches[2]); - if matches[1] == "bitbucket-oauth" { + let key = format!( + "{}.{}", + matches.get(&CaptureKey::ByIndex(1)).unwrap(), + matches.get(&CaptureKey::ByIndex(2)).unwrap() + ); + if matches.get(&CaptureKey::ByIndex(1)).unwrap() == "bitbucket-oauth" { if 2 != values.len() { return Err(RuntimeException::new(format!( "Expected two arguments (consumer-key, consumer-secret), got {}", @@ -1372,7 +1384,9 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_config_setting(&key, PhpMixed::Array(obj)); - } else if matches[1] == "gitlab-token" && 2 == values.len() { + } else if matches.get(&CaptureKey::ByIndex(1)).unwrap() == "gitlab-token" + && 2 == values.len() + { self.config_source .borrow_mut() .as_mut() @@ -1387,7 +1401,7 @@ impl Command for ConfigCommand { .unwrap() .add_config_setting(&key, PhpMixed::Array(obj)); } else if matches!( - matches[1].as_str(), + matches.get(&CaptureKey::ByIndex(1)).unwrap().as_str(), "github-oauth" | "gitlab-oauth" | "gitlab-token" | "bearer" ) { if 1 != values.len() { @@ -1406,7 +1420,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_config_setting(&key, PhpMixed::String(values[0].clone())); - } else if matches[1] == "http-basic" { + } else if matches.get(&CaptureKey::ByIndex(1)).unwrap() == "http-basic" { if 2 != values.len() { return Err(RuntimeException::new(format!( "Expected two arguments (username, password), got {}", @@ -1427,7 +1441,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_config_setting(&key, PhpMixed::Array(obj)); - } else if matches[1] == "custom-headers" { + } else if matches.get(&CaptureKey::ByIndex(1)).unwrap() == "custom-headers" { if values.is_empty() { return Err(RuntimeException::new( "Expected at least one argument (header), got none".to_string(), @@ -1468,7 +1482,7 @@ impl Command for ConfigCommand { .as_mut() .unwrap() .add_config_setting(&key, PhpMixed::List(formatted_headers)); - } else if matches[1] == "forgejo-token" { + } else if matches.get(&CaptureKey::ByIndex(1)).unwrap() == "forgejo-token" { if 2 != values.len() { return Err(RuntimeException::new(format!( "Expected two arguments (username, access token), got {}", diff --git a/crates/shirabe/src/console/application.rs b/crates/shirabe/src/console/application.rs index 5385f790..ca645635 100644 --- a/crates/shirabe/src/console/application.rs +++ b/crates/shirabe/src/console/application.rs @@ -1792,8 +1792,8 @@ impl Application { let mut offset = 0i64; while let Some(m) = preg_match2(php_regex!(r"/.{1,10000}/u"), &utf8_string, offset as usize) { - let m0 = m[&shirabe_php_shim::CaptureKey::ByIndex(0)] - .as_deref() + let m0 = m + .get(&shirabe_php_shim::CaptureKey::ByIndex(0)) .unwrap_or(""); offset += shirabe_php_shim::strlen(m0); diff --git a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs index f03f82db..6cc1670d 100644 --- a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs +++ b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs @@ -956,7 +956,13 @@ try {{ if Platform::is_windows() { path_and_args = Preg::replace_callback( php_regex!("{^\\S+}"), - |m| str_replace("/", "\\", &m[0]), + |m| { + str_replace( + "/", + "\\", + m.get(&CaptureKey::ByIndex(0)).unwrap(), + ) + }, &path_and_args, ); } @@ -985,7 +991,11 @@ try {{ path_and_args = format!( "{}{}", path_to_exec, - substr(&path_and_args, strlen(&m[0]), None) + substr( + &path_and_args, + strlen(m.get(&CaptureKey::ByIndex(0)).unwrap()), + None + ) ); } } @@ -1001,7 +1011,13 @@ try {{ if Platform::is_windows() { exec = Preg::replace_callback( php_regex!("{^\\S+}"), - |m| str_replace("/", "\\", &m[0]), + |m| { + str_replace( + "/", + "\\", + m.get(&CaptureKey::ByIndex(0)).unwrap(), + ) + }, &exec, ); } -- cgit v1.3.1-4-g156e