From 844097edf44bf1424d28e2d5fbefda90c1c8f46c Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 18 Aug 2026 01:57:02 +0900 Subject: refactor(pcre): hand back the match instead of copying it out Preg::match4 and Preg::replace_callback gave callers a PregMatchedGroups: an IndexMap rebuilt from the match with an owned String per group, plus a second String for a named group's name key. That is the copy PregMatches shed when it started wrapping regex::Captures, reinstated one layer up -- and nearly every regex call in the tree goes through Preg rather than the shim's preg_* directly, so almost nothing saw the borrow. PregMatchedGroups existed only to drop the null (unmatched) groups the old PregMatches held as Option values. PregMatches::get reports a non-participating group as None on its own, so the two read alike and the type collapses into it. Call sites still reach groups through get(&CaptureKey::ByIndex(N)); what changes is that the value arrives as a &str borrowed from the subject, which the signatures now carry as a lifetime. Three places needed the borrow reckoned with rather than a mechanical rewrite: PhpFileCleaner::clean and Problem::get_messages read their groups out before mutating what the match borrows, and Git::get_authentication_failure names the lifetime of its url argument, which the result borrows instead of self. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe/src/util/git.rs | 85 +++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 22 deletions(-) (limited to 'crates/shirabe/src/util/git.rs') diff --git a/crates/shirabe/src/util/git.rs b/crates/shirabe/src/util/git.rs index 2b012d4c..e7239987 100644 --- a/crates/shirabe/src/util/git.rs +++ b/crates/shirabe/src/util/git.rs @@ -14,7 +14,7 @@ use crate::util::ProcessExecutor; use crate::util::Url; use crate::util::{AuthHelper, StoreAuth}; use indexmap::IndexMap; -use shirabe_pcre::{CaptureKey, Preg, PregMatchedGroups}; +use shirabe_pcre::{CaptureKey, Preg, PregMatches}; use shirabe_php_shim::{ AnyThrowable, CmpOp, InvalidArgumentException, PHP_EOL, PhpMixed, RuntimeException, array_map, clearstatcache, explode, implode, in_array_loose, in_array_strict, is_dir, php_regex, @@ -230,13 +230,16 @@ impl Git { php_regex!(r"{^(?:composer|origin)\s+https?://(.+):(.+)@([^/]+)}im"), &output, ) { - let m3 = m.get(&CaptureKey::ByIndex(3)).cloned().unwrap_or_default(); + let m3 = m + .get(&CaptureKey::ByIndex(3)) + .unwrap_or_default() + .to_string(); if !self.io.has_authentication(&m3) { self.io.borrow_mut().set_authentication( m3, - rawurldecode(&m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default()), + rawurldecode(m.get(&CaptureKey::ByIndex(1)).unwrap_or_default()), Some(rawurldecode( - &m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(), + m.get(&CaptureKey::ByIndex(2)).unwrap_or_default(), )), ); } @@ -262,8 +265,14 @@ impl Git { _ => vec![], }; for protocol in &protocols_list { - let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); - let m2 = m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(); + let m1 = m + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); + let m2 = m + .get(&CaptureKey::ByIndex(2)) + .unwrap_or_default() + .to_string(); let proto_url = if protocol == "ssh" { format!("git@{}:{}", m1, m2) } else { @@ -291,7 +300,10 @@ impl Git { } // failed to checkout, first check git accessibility - let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); + let m1 = m + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); if !self.io.has_authentication(&m1) && !self.io.is_interactive() { self.throw_exception( &format!( @@ -357,8 +369,14 @@ impl Git { ) }); if let Some(m) = github_matched { - let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); - let m2 = m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(); + let m1 = m + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); + let m2 = m + .get(&CaptureKey::ByIndex(2)) + .unwrap_or_default() + .to_string(); if !self.io.has_authentication(&m1) { let mut git_hub_util = GitHub::new( self.io.clone(), @@ -421,9 +439,14 @@ impl Git { None, )?; - let domain = m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(); - let mut repo_with_git_part = - m.get(&CaptureKey::ByIndex(3)).cloned().unwrap_or_default(); + let domain = m + .get(&CaptureKey::ByIndex(2)) + .unwrap_or_default() + .to_string(); + let mut repo_with_git_part = m + .get(&CaptureKey::ByIndex(3)) + .unwrap_or_default() + .to_string(); if !repo_with_git_part.ends_with(".git") { repo_with_git_part.push_str(".git"); } @@ -565,9 +588,18 @@ impl Git { url, ) }) { - let mut m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); - let m2 = m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(); - let m3 = m.get(&CaptureKey::ByIndex(3)).cloned().unwrap_or_default(); + let mut m1 = m + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); + let m2 = m + .get(&CaptureKey::ByIndex(2)) + .unwrap_or_default() + .to_string(); + let m3 = m + .get(&CaptureKey::ByIndex(3)) + .unwrap_or_default() + .to_string(); if m1 == "git" { m1 = "https".to_string(); } @@ -641,9 +673,18 @@ impl Git { } } else if let Some(m) = self.get_authentication_failure(url) { // private non-github/gitlab/bitbucket repo that failed to authenticate - let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); - let mut m2 = m.get(&CaptureKey::ByIndex(2)).cloned().unwrap_or_default(); - let m3 = m.get(&CaptureKey::ByIndex(3)).cloned().unwrap_or_default(); + let m1 = m + .get(&CaptureKey::ByIndex(1)) + .unwrap_or_default() + .to_string(); + let mut m2 = m + .get(&CaptureKey::ByIndex(2)) + .unwrap_or_default() + .to_string(); + let m3 = m + .get(&CaptureKey::ByIndex(3)) + .unwrap_or_default() + .to_string(); let mut auth_parts: Option = None; if m2.contains("@") { let parts = explode("@", &m2); @@ -1083,7 +1124,7 @@ impl Git { Ok(false) } - fn get_authentication_failure(&self, url: &str) -> Option { + fn get_authentication_failure<'u>(&self, url: &'u str) -> Option> { let m = Preg::is_match3(php_regex!(r"{^(https?://)([^/]+)(.*)$}i"), url)?; let auth_failures = [ @@ -1172,8 +1213,8 @@ impl Git { return Ok(Some( matches .get(&CaptureKey::ByIndex(1)) - .cloned() - .unwrap_or_default(), + .unwrap_or_default() + .to_string(), )); } } @@ -1295,7 +1336,7 @@ impl Git { && let Some(matches) = Preg::is_match3(php_regex!(r"/^git version (\d+(?:\.\d+)+)/m"), &output) { - *version = Some(matches.get(&CaptureKey::ByIndex(1)).cloned()); + *version = Some(matches.get(&CaptureKey::ByIndex(1)).map(str::to_string)); } } version.clone().unwrap_or(None) -- cgit v1.3.1-4-g156e