From 059d528b76914aaefebc42705984586ebb1c607a Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 8 May 2026 23:45:50 +0900 Subject: refactor(advisory): fix clippy warnings Implement std::str::FromStr for AuditFormat and AbandonedHandling instead of ad-hoc from_str methods (resolves should_implement_trait). Group Auditor::audit() parameters into AuditOptions to resolve too_many_arguments. --- crates/mozart-core/src/advisory.rs | 32 ++++++++++------ crates/mozart-registry/src/advisory.rs | 36 +++++++++++------- crates/mozart/src/commands/audit.rs | 38 ++++++++++--------- crates/mozart/src/commands/require.rs | 67 ++++++++++++++++++---------------- 4 files changed, 98 insertions(+), 75 deletions(-) diff --git a/crates/mozart-core/src/advisory.rs b/crates/mozart-core/src/advisory.rs index 4752c8b..f20405e 100644 --- a/crates/mozart-core/src/advisory.rs +++ b/crates/mozart-core/src/advisory.rs @@ -32,14 +32,18 @@ impl AuditFormat { Self::Summary => FORMAT_SUMMARY, } } +} + +impl std::str::FromStr for AuditFormat { + type Err = (); - pub fn from_str(s: &str) -> Option { + fn from_str(s: &str) -> Result { match s { - FORMAT_TABLE => Some(Self::Table), - FORMAT_PLAIN => Some(Self::Plain), - FORMAT_JSON => Some(Self::Json), - FORMAT_SUMMARY => Some(Self::Summary), - _ => None, + FORMAT_TABLE => Ok(Self::Table), + FORMAT_PLAIN => Ok(Self::Plain), + FORMAT_JSON => Ok(Self::Json), + FORMAT_SUMMARY => Ok(Self::Summary), + _ => Err(()), } } } @@ -61,13 +65,17 @@ impl AbandonedHandling { Self::Fail => ABANDONED_FAIL, } } +} + +impl std::str::FromStr for AbandonedHandling { + type Err = (); - pub fn from_str(s: &str) -> Option { + fn from_str(s: &str) -> Result { match s { - ABANDONED_IGNORE => Some(Self::Ignore), - ABANDONED_REPORT => Some(Self::Report), - ABANDONED_FAIL => Some(Self::Fail), - _ => None, + ABANDONED_IGNORE => Ok(Self::Ignore), + ABANDONED_REPORT => Ok(Self::Report), + ABANDONED_FAIL => Ok(Self::Fail), + _ => Err(()), } } } @@ -182,7 +190,7 @@ impl AuditConfig { let audit_abandoned = audit_val .get("abandoned") .and_then(|v| v.as_str()) - .and_then(AbandonedHandling::from_str) + .and_then(|s| s.parse::().ok()) .unwrap_or_default(); let block_insecure = audit_val diff --git a/crates/mozart-registry/src/advisory.rs b/crates/mozart-registry/src/advisory.rs index 8cf112e..894a0ac 100644 --- a/crates/mozart-registry/src/advisory.rs +++ b/crates/mozart-registry/src/advisory.rs @@ -66,6 +66,17 @@ pub struct AbandonedPackage { pub replacement: Option, } +/// Options passed to `Auditor::audit()`. +pub struct AuditOptions<'a> { + pub format: AuditFormat, + pub warning_only: bool, + pub ignore_list: &'a IndexMap>, + pub abandoned: AbandonedHandling, + pub ignored_severities: &'a IndexMap>, + pub ignore_unreachable: bool, + pub ignore_abandoned: &'a IndexMap>, +} + /// Mirrors `Composer\Advisory\Auditor`. pub struct Auditor; @@ -82,34 +93,33 @@ impl Auditor { console: &Console, repo_set: &RepositorySet, packages: &[PackageInfo], - format: AuditFormat, - warning_only: bool, - ignore_list: &IndexMap>, - abandoned: AbandonedHandling, - ignored_severities: &IndexMap>, - ignore_unreachable: bool, - ignore_abandoned: &IndexMap>, + options: &AuditOptions<'_>, ) -> anyhow::Result { + let format = options.format; let (all_advisories, unreachable_repos) = repo_set .get_matching_security_advisories( packages, format == AuditFormat::Summary, - ignore_unreachable, + options.ignore_unreachable, ) .await?; let ProcessedAdvisories { advisories, ignored_advisories, - } = self.process_advisories(all_advisories, ignore_list, ignored_severities); + } = self.process_advisories( + all_advisories, + options.ignore_list, + options.ignored_severities, + ); - let abandoned_packages = if abandoned == AbandonedHandling::Ignore { + let abandoned_packages = if options.abandoned == AbandonedHandling::Ignore { vec![] } else { - self.filter_abandoned_packages(packages, ignore_abandoned) + self.filter_abandoned_packages(packages, options.ignore_abandoned) }; - let abandoned_count = if abandoned == AbandonedHandling::Fail { + let abandoned_count = if options.abandoned == AbandonedHandling::Fail { abandoned_packages.len() } else { 0 @@ -159,7 +169,7 @@ impl Auditor { let msg = format!( "Found {active_total} security vulnerability advisor{plurality} affecting {active_pkg_count} package{pkg_plurality}{punctuation}" ); - if warning_only { + if options.warning_only { console_writeln_error!(console, &console_format!("{msg}")); } else { console_writeln_error!(console, &console_format!("{msg}")); diff --git a/crates/mozart/src/commands/audit.rs b/crates/mozart/src/commands/audit.rs index 74cb536..df0e5a9 100644 --- a/crates/mozart/src/commands/audit.rs +++ b/crates/mozart/src/commands/audit.rs @@ -4,7 +4,7 @@ use clap::Args; use indexmap::IndexMap; use mozart_core::advisory::{AbandonedHandling, AuditConfig, AuditFormat}; use mozart_core::composer::Composer; -use mozart_registry::advisory::{Auditor, PackageInfo}; +use mozart_registry::advisory::{AuditOptions, Auditor, PackageInfo}; use mozart_registry::cache::{Cache, build_cache_config}; use mozart_registry::repository::RepositorySet; @@ -50,9 +50,9 @@ pub async fn execute( // Resolve format: CLI arg > config default (table) let format = match args.format.as_deref() { - Some(f) => match AuditFormat::from_str(f) { - Some(fmt) => fmt, - None => anyhow::bail!( + Some(f) => match f.parse::() { + Ok(fmt) => fmt, + Err(_) => anyhow::bail!( "Invalid format \"{f}\". Supported formats: table, plain, json, summary" ), }, @@ -61,9 +61,9 @@ pub async fn execute( // Resolve --abandoned: CLI > config let abandoned = match args.abandoned.as_deref() { - Some(s) => match AbandonedHandling::from_str(s) { - Some(h) => h, - None => anyhow::bail!( + Some(s) => match s.parse::() { + Ok(h) => h, + Err(_) => anyhow::bail!( "Invalid abandoned value \"{s}\". Supported values: ignore, report, fail" ), }, @@ -98,13 +98,15 @@ pub async fn execute( console, &repo_set, &packages, - format, - false, - &audit_config.ignore_list_for_audit, - abandoned, - &ignore_severities, - ignore_unreachable, - &audit_config.ignore_abandoned_for_audit, + &AuditOptions { + format, + warning_only: false, + ignore_list: &audit_config.ignore_list_for_audit, + abandoned, + ignored_severities: &ignore_severities, + ignore_unreachable, + ignore_abandoned: &audit_config.ignore_abandoned_for_audit, + }, ) .await?; @@ -460,14 +462,14 @@ mod tests { #[test] fn test_invalid_format() { let format = "xml"; - assert!(AuditFormat::from_str(format).is_none()); + assert!(format.parse::().is_err()); } #[test] fn test_valid_formats() { for fmt in &["table", "plain", "json", "summary"] { assert!( - AuditFormat::from_str(fmt).is_some(), + fmt.parse::().is_ok(), "format {fmt} should be valid" ); } @@ -475,14 +477,14 @@ mod tests { #[test] fn test_invalid_abandoned_value() { - assert!(AbandonedHandling::from_str("maybe").is_none()); + assert!("maybe".parse::().is_err()); } #[test] fn test_valid_abandoned_values() { for value in &["ignore", "report", "fail"] { assert!( - AbandonedHandling::from_str(value).is_some(), + value.parse::().is_ok(), "abandoned value {value} should be valid" ); } diff --git a/crates/mozart/src/commands/require.rs b/crates/mozart/src/commands/require.rs index 2b57a6d..39b055f 100644 --- a/crates/mozart/src/commands/require.rs +++ b/crates/mozart/src/commands/require.rs @@ -158,12 +158,13 @@ fn revert_composer_file(state: &CommandState, console: &mozart_core::console::Co } // Also remove any lock file that was created during this (failed) run if state.lock_path.exists() - && let Err(e) = std::fs::remove_file(&state.lock_path) { - console.write_error(&format!( - "Warning: Failed to delete {}: {e}", - state.lock_path.display() - )); - } + && let Err(e) = std::fs::remove_file(&state.lock_path) + { + console.write_error(&format!( + "Warning: Failed to delete {}: {e}", + state.lock_path.display() + )); + } } else { let msg = if state.lock_backup.is_some() { format!(" and {} to their", state.lock_path.display()) @@ -181,12 +182,13 @@ fn revert_composer_file(state: &CommandState, console: &mozart_core::console::Co )); } if let Some(ref lock_content) = state.lock_backup - && let Err(e) = std::fs::write(&state.lock_path, lock_content) { - console.write_error(&format!( - "Warning: Failed to revert {}: {e}", - state.lock_path.display() - )); - } + && let Err(e) = std::fs::write(&state.lock_path, lock_content) + { + console.write_error(&format!( + "Warning: Failed to revert {}: {e}", + state.lock_path.display() + )); + } } } @@ -408,26 +410,27 @@ async fn do_update( // if (!$this->firstRequire && $composer->getLocker()->isLocked()) // $install->setUpdateAllowList(array_keys($requirements)); if !state.first_require - && let Some(ref lock) = old_lock { - let with_deps = args.with_dependencies || args.update_with_dependencies; - let with_all_deps = args.with_all_dependencies || args.update_with_all_dependencies; - let newly_required: Vec = - additions.iter().map(|(name, _, _)| name.clone()).collect(); - let repo_requires = super::update::collect_repo_requires(&raw.repositories); - let allow_list = if with_all_deps { - super::update::expand_with_all_dependencies(newly_required, lock, &repo_requires) - } else if with_deps { - super::update::expand_with_direct_dependencies( - newly_required, - lock, - &IndexSet::new(), - &repo_requires, - ) - } else { - additions.iter().map(|(name, _, _)| name.clone()).collect() - }; - resolved = super::update::apply_partial_update(resolved, lock, &allow_list); - } + && let Some(ref lock) = old_lock + { + let with_deps = args.with_dependencies || args.update_with_dependencies; + let with_all_deps = args.with_all_dependencies || args.update_with_all_dependencies; + let newly_required: Vec = + additions.iter().map(|(name, _, _)| name.clone()).collect(); + let repo_requires = super::update::collect_repo_requires(&raw.repositories); + let allow_list = if with_all_deps { + super::update::expand_with_all_dependencies(newly_required, lock, &repo_requires) + } else if with_deps { + super::update::expand_with_direct_dependencies( + newly_required, + lock, + &IndexSet::new(), + &repo_requires, + ) + } else { + additions.iter().map(|(name, _, _)| name.clone()).collect() + }; + resolved = super::update::apply_partial_update(resolved, lock, &allow_list); + } let composer_json_content = if args.dry_run { package::to_json_pretty(raw)? -- cgit v1.3.1