From efe5bdb1987411a473d4af15451a376d20928245 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 12 Jun 2026 03:19:34 +0900 Subject: refactor(php-shim): replace literal sprintf calls with format! Convert every sprintf() call with a compile-time literal format string to format!, implementing Display for PhpMixed (delegating to php_to_string) so PhpMixed values render with PHP string semantics through {}. Also merge the format!-wrapped and conditional-literal dynamic sites into single format! calls. Genuinely runtime format strings (table styles, configurable error messages, command synopsis, progress-bar modifiers, regex-built messages) still go through sprintf. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shirabe/src/package/loader/array_loader.rs | 44 ++++++++++------------ .../src/package/loader/validating_array_loader.rs | 34 ++++++++--------- crates/shirabe/src/package/locker.rs | 38 ++++++++----------- 3 files changed, 51 insertions(+), 65 deletions(-) (limited to 'crates/shirabe/src/package') diff --git a/crates/shirabe/src/package/loader/array_loader.rs b/crates/shirabe/src/package/loader/array_loader.rs index 4df022b..8e87302 100644 --- a/crates/shirabe/src/package/loader/array_loader.rs +++ b/crates/shirabe/src/package/loader/array_loader.rs @@ -395,18 +395,16 @@ impl ArrayLoader { .unwrap_or(false); if !has_required { return Err(UnexpectedValueException { - message: sprintf( - "Package %s's source key should be specified as {\"type\": ..., \"url\": ..., \"reference\": ...},\n%s given.", - &[ - PhpMixed::String( - config - .get("name") - .and_then(|v| v.as_string()) - .unwrap_or("") - .to_string(), - ), - PhpMixed::String(json_encode(&source).unwrap_or_default()), - ], + message: format!( + "Package {}'s source key should be specified as {{\"type\": ..., \"url\": ..., \"reference\": ...}},\n{} given.", + PhpMixed::String( + config + .get("name") + .and_then(|v| v.as_string()) + .unwrap_or("") + .to_string(), + ), + PhpMixed::String(json_encode(&source).unwrap_or_default()), ), code: 0, } @@ -440,18 +438,16 @@ impl ArrayLoader { .unwrap_or(false); if !has_required { return Err(UnexpectedValueException { - message: sprintf( - "Package %s's dist key should be specified as {\"type\": ..., \"url\": ..., \"reference\": ..., \"shasum\": ...},\n%s given.", - &[ - PhpMixed::String( - config - .get("name") - .and_then(|v| v.as_string()) - .unwrap_or("") - .to_string(), - ), - PhpMixed::String(json_encode(&dist).unwrap_or_default()), - ], + message: format!( + "Package {}'s dist key should be specified as {{\"type\": ..., \"url\": ..., \"reference\": ..., \"shasum\": ...}},\n{} given.", + PhpMixed::String( + config + .get("name") + .and_then(|v| v.as_string()) + .unwrap_or("") + .to_string(), + ), + PhpMixed::String(json_encode(&dist).unwrap_or_default()), ), code: 0, } diff --git a/crates/shirabe/src/package/loader/validating_array_loader.rs b/crates/shirabe/src/package/loader/validating_array_loader.rs index e6db19e..4a1e2c2 100644 --- a/crates/shirabe/src/package/loader/validating_array_loader.rs +++ b/crates/shirabe/src/package/loader/validating_array_loader.rs @@ -195,9 +195,9 @@ impl ValidatingArrayLoader { for index in &license_keys { let license = licenses[index].clone(); if !is_string(&*license) { - self.warnings.push(sprintf( - "License %s should be a string.", - &[PhpMixed::String(json_encode(&*license).unwrap_or_default())], + self.warnings.push(format!( + "License {} should be a string.", + PhpMixed::String(json_encode(&*license).unwrap_or_default()), )); licenses.shift_remove(index); } @@ -218,23 +218,21 @@ impl ValidatingArrayLoader { if license_validator .validate(&trim(&license_to_validate, Some(" \t\n\r\0\u{0B}"))) { - self.warnings.push(sprintf( - "License %s must not contain extra spaces, make sure to trim it.", - &[PhpMixed::String( + self.warnings.push(format!( + "License {} must not contain extra spaces, make sure to trim it.", + PhpMixed::String( json_encode(&PhpMixed::String(license_str.clone())) .unwrap_or_default(), - )], + ), )); } else { - self.warnings.push(sprintf( - &format!( - "License %s is not a valid SPDX license identifier, see https://spdx.org/licenses/ if you use an open license.{}If the software is closed-source, you may use \"proprietary\" as license.", - PHP_EOL - ), - &[PhpMixed::String( + self.warnings.push(format!( + "License {} is not a valid SPDX license identifier, see https://spdx.org/licenses/ if you use an open license.{}If the software is closed-source, you may use \"proprietary\" as license.", + PhpMixed::String( json_encode(&PhpMixed::String(license_str.clone())) .unwrap_or_default(), - )], + ), + PHP_EOL )); } } @@ -251,11 +249,9 @@ impl ValidatingArrayLoader { Box::new(PhpMixed::Array(reindexed_map)), ); } else { - self.warnings.push(sprintf( - "License must be a string or array of strings, got %s.", - &[PhpMixed::String( - json_encode(&*license_val).unwrap_or_default(), - )], + self.warnings.push(format!( + "License must be a string or array of strings, got {}.", + PhpMixed::String(json_encode(&*license_val).unwrap_or_default(),), )); self.config.shift_remove("license"); } diff --git a/crates/shirabe/src/package/locker.rs b/crates/shirabe/src/package/locker.rs index ed53143..c61d807 100644 --- a/crates/shirabe/src/package/locker.rs +++ b/crates/shirabe/src/package/locker.rs @@ -750,9 +750,9 @@ impl Locker { if name.is_empty() || version.is_empty() { return Err(LogicException { - message: sprintf( - "Package \"%s\" has no version or name and can not be locked", - &[PhpMixed::String(package.to_string())], + message: format!( + "Package \"{}\" has no version or name and can not be locked", + PhpMixed::String(package.to_string()), ), code: 0, } @@ -975,11 +975,8 @@ impl Locker { let _ = &results; let mut description = provider.get_pretty_version(); if provider.get_name() != link.get_target() { - 'outer: for (method, text) in [ - ("getReplaces", "replaced as %s by %s"), - ("getProvides", "provided as %s by %s"), - ] - .iter() + 'outer: for (method, verb) in + [("getReplaces", "replaced"), ("getProvides", "provided")].iter() { let provider_links = match *method { "getReplaces" => provider.get_replaces(), @@ -988,20 +985,17 @@ impl Locker { }; for provider_link in provider_links.values() { if provider_link.get_target() == link.get_target() { - description = sprintf( - text, - &[ - PhpMixed::String( - provider_link - .get_pretty_constraint() - .to_string(), - ), - PhpMixed::String(format!( - "{} {}", - provider.get_pretty_name(), - provider.get_pretty_version() - )), - ], + description = format!( + "{} as {} by {}", + verb, + PhpMixed::String( + provider_link.get_pretty_constraint().to_string(), + ), + PhpMixed::String(format!( + "{} {}", + provider.get_pretty_name(), + provider.get_pretty_version() + )), ); break 'outer; } -- cgit v1.3.1