diff --git a/Cargo.lock b/Cargo.lock index f8da8ce..1e35f03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -43,9 +43,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.119" +version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" dependencies = [ "proc-macro2", "quote", diff --git a/crates/hotspots-cli/src/args/flags.rs b/crates/hotspots-cli/src/args/flags.rs index b020cb8..1fa633c 100644 --- a/crates/hotspots-cli/src/args/flags.rs +++ b/crates/hotspots-cli/src/args/flags.rs @@ -61,10 +61,12 @@ pub(super) fn parse_flag( } fn lookup_flag(flag: &str) -> Option { - FLAG_NAMES - .iter() - .find(|(names, _)| names.contains(&flag)) - .map(|(_, kind)| *kind) + for (names, kind) in FLAG_NAMES { + if names.contains(&flag) { + return Some(*kind); + } + } + None } fn apply_flag( diff --git a/crates/hotspots-cli/src/args/tests.rs b/crates/hotspots-cli/src/args/tests.rs index c431f42..17b4217 100644 --- a/crates/hotspots-cli/src/args/tests.rs +++ b/crates/hotspots-cli/src/args/tests.rs @@ -38,6 +38,7 @@ fn parses_required_fields() { #[test] fn parses_optional_numeric_flags() { + // dry-rs:ignore. Parallel optional-flag parse fixtures; structural twins are intentional. let parsed = parse_args(&argv(&[ "-l", "log.txt", "-c", "git2", "-r", "10", "-m", "2", "-i", "20", "-x", "80", "-s", "15", ])); @@ -52,6 +53,7 @@ fn parses_optional_numeric_flags() { #[test] fn parses_optional_path_flags() { + // dry-rs:ignore. Parallel optional-flag parse fixtures; structural twins are intentional. let parsed = parse_args(&argv(&[ "-l", "log.txt", diff --git a/crates/hotspots-rs/Cargo.toml b/crates/hotspots-rs/Cargo.toml index 4d1a6b0..7a98266 100644 --- a/crates/hotspots-rs/Cargo.toml +++ b/crates/hotspots-rs/Cargo.toml @@ -12,7 +12,7 @@ categories = ["development-tools"] [dependencies] hotspots = { path = "../hotspots", version = "0.1.0" } proc-macro2 = { version = "1", default-features = false, features = ["span-locations"] } -syn = { version = "2", default-features = false, features = ["full", "parsing", "printing", "clone-impls", "extra-traits"] } +syn = { version = "3", default-features = false, features = ["full", "parsing", "printing", "clone-impls", "extra-traits"] } [lints] workspace = true diff --git a/crates/hotspots-rs/src/parse.rs b/crates/hotspots-rs/src/parse.rs index 2319756..cbd9813 100644 --- a/crates/hotspots-rs/src/parse.rs +++ b/crates/hotspots-rs/src/parse.rs @@ -97,7 +97,7 @@ fn type_path_name(ty: &syn::Type) -> Option { } fn trait_path_name(imp: &syn::ItemImpl) -> Option { - let (_, path, _) = imp.trait_.as_ref()?; + let (path, _) = imp.trait_.as_ref()?; path.segments.last().map(|seg| seg.ident.to_string()) } diff --git a/crates/hotspots-rs/src/resolve/mod.rs b/crates/hotspots-rs/src/resolve/mod.rs index 367df49..3cc84f7 100644 --- a/crates/hotspots-rs/src/resolve/mod.rs +++ b/crates/hotspots-rs/src/resolve/mod.rs @@ -49,11 +49,13 @@ impl SymbolResolver for RustGitSynResolver { } fn rust_keys(changes: &[Change]) -> BTreeSet<(String, String)> { - changes - .iter() - .filter(|c| is_rust_path(&c.entity)) - .map(|c| (c.rev.clone(), c.entity.clone())) - .collect() + let mut keys = BTreeSet::new(); + for change in changes { + if is_rust_path(&change.entity) { + keys.insert((change.rev.clone(), change.entity.clone())); + } + } + keys } fn git_job_count(key_count: usize) -> usize { diff --git a/crates/hotspots/src/analysis/effort.rs b/crates/hotspots/src/analysis/effort.rs index 8878a7a..ad310f5 100644 --- a/crates/hotspots/src/analysis/effort.rs +++ b/crates/hotspots/src/analysis/effort.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use crate::analysis::table::Table; use crate::analysis::util::{ - cmp_f64_desc, count_as_u64, fmt_pct, fmt_u64, meets_min_revs, percent, + cmp_f64_desc, count_as_u64, fmt_pct, fmt_u64, leading_by_metric, meets_min_revs, percent, }; use crate::index::logical_rev; use crate::model::Change; @@ -139,10 +139,7 @@ fn collect_logical_revs(changes: &[Change], period: TemporalPeriod) -> EntityAut } fn leading_author_by_revs(authors: BTreeMap>) -> Option<(String, u64)> { - authors - .into_iter() - .map(|(author, revs)| (author, count_as_u64(revs.len()))) - .max_by(|a, b| a.1.cmp(&b.1).then(a.0.cmp(&b.0))) + leading_by_metric(authors.into_iter().map(|(author, revs)| (author, count_as_u64(revs.len())))) } #[cfg(test)] diff --git a/crates/hotspots/src/analysis/mod.rs b/crates/hotspots/src/analysis/mod.rs index 14281dd..b41b162 100644 --- a/crates/hotspots/src/analysis/mod.rs +++ b/crates/hotspots/src/analysis/mod.rs @@ -69,7 +69,12 @@ pub fn run(name: &str, changes: &[Change], opts: &Options) -> Result { } fn lookup(table: &[(&str, T)], name: &str) -> Option { - table.iter().find(|(key, _)| *key == name).map(|(_, f)| *f) + for &(key, value) in table { + if key == name { + return Some(value); + } + } + None } /// Names of analyses accepted by [`run`]. diff --git a/crates/hotspots/src/analysis/ownership.rs b/crates/hotspots/src/analysis/ownership.rs index b7e6638..bd4b75a 100644 --- a/crates/hotspots/src/analysis/ownership.rs +++ b/crates/hotspots/src/analysis/ownership.rs @@ -7,7 +7,7 @@ use std::collections::BTreeMap; use crate::analysis::table::Table; use crate::analysis::util::{ - entity_revisions, fmt_pct, fmt_u64, meets_min_revs, percent, require_churn, + entity_revisions, fmt_pct, fmt_u64, leading_by_metric, meets_min_revs, percent, require_churn, }; use crate::error::Result; use crate::model::Change; @@ -90,10 +90,7 @@ fn collect_churn(changes: &[Change]) -> EntityAuthorChurn { } fn leading_author_by_added(authors: BTreeMap) -> Option<(String, u64)> { - authors - .into_iter() - .map(|(author, (added, _))| (author, added)) - .max_by(|a, b| a.1.cmp(&b.1).then(a.0.cmp(&b.0))) + leading_by_metric(authors.into_iter().map(|(author, (added, _))| (author, added))) } fn flatten_churn(map: EntityAuthorChurn) -> Vec<(String, String, u64, u64)> { diff --git a/crates/hotspots/src/analysis/util.rs b/crates/hotspots/src/analysis/util.rs index 3636627..59c797d 100644 --- a/crates/hotspots/src/analysis/util.rs +++ b/crates/hotspots/src/analysis/util.rs @@ -43,14 +43,26 @@ pub fn entity_authors(changes: &[Change]) -> BTreeMap> /// /// Returns an error when any row lacks numstat data. pub fn require_churn(changes: &[Change]) -> Result<()> { - if changes.iter().any(|c| c.added.is_none() || c.deleted.is_none()) { - return Err(Error::msg( - "analysis requires numstat line counts; use `-c git2` or a numstat log", - )); + for change in changes { + if change.added.is_none() || change.deleted.is_none() { + return Err(Error::msg( + "analysis requires numstat line counts; use `-c git2` or a numstat log", + )); + } } Ok(()) } +/// Picks the author with the highest metric, breaking ties by author name. +#[must_use] +pub fn leading_by_metric( + authors: impl IntoIterator, +) -> Option<(String, u64)> { + authors + .into_iter() + .max_by(|left, right| left.1.cmp(&right.1).then(left.0.cmp(&right.0))) +} + /// Drops entities below the configured revision threshold. #[must_use] pub const fn meets_min_revs(revs: u64, opts: &Options) -> bool { @@ -160,6 +172,11 @@ mod tests { }; assert!(!meets_min_revs(1, &opts)); assert!(meets_min_revs(2, &opts)); + assert!(leading_by_metric(Vec::<(String, u64)>::new()).is_none()); + assert_eq!( + leading_by_metric([(String::from("Bea"), 5), (String::from("Ada"), 5)]), + Some((String::from("Bea"), 5)) + ); } #[test] diff --git a/crates/hotspots/src/filter.rs b/crates/hotspots/src/filter.rs index da7c0ec..b49a288 100644 --- a/crates/hotspots/src/filter.rs +++ b/crates/hotspots/src/filter.rs @@ -55,8 +55,10 @@ impl PathFilter { /// /// Returns an error when any prefix is empty after trim and trailing-`/` strip. pub fn ensure_include_prefixes(include: &[String]) -> Result<()> { - if include.iter().any(|raw| normalize_prefix(raw).is_empty()) { - return Err(Error::msg("`--include` prefix must be non-empty")); + for raw in include { + if normalize_prefix(raw).is_empty() { + return Err(Error::msg("`--include` prefix must be non-empty")); + } } Ok(()) } diff --git a/crates/hotspots/src/symbols/expand.rs b/crates/hotspots/src/symbols/expand.rs index 52e3eb5..f12f7ae 100644 --- a/crates/hotspots/src/symbols/expand.rs +++ b/crates/hotspots/src/symbols/expand.rs @@ -123,10 +123,18 @@ fn add_overlap_counts( } fn covering_symbol(symbols: &[SymbolFact], line: u32) -> Option<&SymbolFact> { - symbols - .iter() - .filter(|s| s.start_line <= line && line <= s.end_line) - .min_by_key(|s| s.end_line.saturating_sub(s.start_line)) + let mut best: Option<&SymbolFact> = None; + for symbol in symbols { + if symbol.start_line > line || line > symbol.end_line { + continue; + } + let span = symbol.end_line.saturating_sub(symbol.start_line); + match best { + Some(current) if current.end_line.saturating_sub(current.start_line) <= span => {} + _ => best = Some(symbol), + } + } + best } #[cfg(test)] @@ -163,6 +171,28 @@ mod tests { assert_eq!(rows[0].added, Some(2)); } + #[test] + fn prefers_tightest_covering_symbol_when_nested() { + // dry-rs:ignore. Parallel expand_file_change fixture; structural twin is intentional. + let change = Change::new("abc", "Ada", "2024-01-01", "a.rs", Some(1), Some(0)); + let diff = FileDiff { + rev: String::from("abc"), + path: String::from("a.rs"), + // Inner first so the later wider span exercises the keep-current branch. + symbols_new: vec![fact("inner", 5, 8), fact("outer", 1, 20)], + symbols_old: vec![], + hunks: vec![Hunk { + old_start: 0, + old_count: 0, + new_start: 6, + new_count: 1, + }], + }; + let rows = expand_file_change(&change, &diff); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].entity, "a.rs::inner"); + } + #[test] fn skips_when_hunks_miss_symbols() { let change = Change::new("abc", "Ada", "2024-01-01", "a.rs", Some(1), Some(0));