From 5220abbee77997be9502d0b5ac0f0e9459964f97 Mon Sep 17 00:00:00 2001 From: jorenham Date: Wed, 22 Jul 2026 18:56:43 +0200 Subject: [PATCH] Dedupe the `names` bookkeeping in `pyrefly coverage` --- pyrefly/lib/commands/coverage/collect.rs | 33 +++++++++--------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/pyrefly/lib/commands/coverage/collect.rs b/pyrefly/lib/commands/coverage/collect.rs index a7dac48d93..5520a39617 100644 --- a/pyrefly/lib/commands/coverage/collect.rs +++ b/pyrefly/lib/commands/coverage/collect.rs @@ -404,9 +404,7 @@ fn filter_module_report_to_public(report: &mut ModuleReport, public_fqns: &HashS report .symbol_reports .retain(|sym| is_public_fqn(sym.name(), &module_prefix, public_fqns)); - report - .names - .retain(|n| is_public_fqn(n, &module_prefix, public_fqns)); + report.names = symbol_names(&report.symbol_reports); // type ignores attach to the module, not symbols, so filtering leaves them untouched report.slots = report @@ -1462,6 +1460,12 @@ impl ModuleSymbols { } } +/// The symbol names in report order, deduped (e.g. conditional redefinitions repeat names). +fn symbol_names(symbol_reports: &[SymbolReport]) -> Vec { + let unique: SmallSet<&str> = symbol_reports.iter().map(SymbolReport::name).collect(); + unique.iter().copied().map(str::to_owned).collect() +} + fn build_module_report( name: String, path: String, @@ -1474,11 +1478,9 @@ fn build_module_report( ) -> ModuleReport { let mut symbol_reports = Vec::new(); let mut total_slots = SlotCounts::default(); - let mut names = Vec::new(); for var in variables { total_slots = total_slots.merge(var.slots); - names.push(var.name.clone()); symbol_reports.push(SymbolReport::Attr { name: var.name.clone(), slots: var.slots, @@ -1497,7 +1499,6 @@ fn build_module_report( } } else { total_slots = total_slots.merge(func.slots); - names.push(func.name.clone()); symbol_reports.push(SymbolReport::Function { name: func.name.clone(), slots: func.slots, @@ -1508,7 +1509,6 @@ fn build_module_report( } for (name, slots, location) in &property_map { total_slots = total_slots.merge(*slots); - names.push(name.clone()); symbol_reports.push(SymbolReport::Property { name: name.clone(), slots: *slots, @@ -1517,7 +1517,6 @@ fn build_module_report( } for cls in classes { - names.push(cls.name.clone()); symbol_reports.push(SymbolReport::Class { name: cls.name.clone(), slots: SlotCounts::default(), @@ -1525,10 +1524,6 @@ fn build_module_report( }); } - // Overloads and property accessors produce duplicate names. - let mut seen = SmallSet::new(); - names.retain(|n| seen.insert(n.clone())); - // Match prefixes against the derived (file-based) name: symbol names are // built from it and only rewritten to the override name later. let module_prefix = format!("{}.", derived_name); @@ -1539,23 +1534,18 @@ fn build_module_report( // A `--module` override renames the module, so rewrite symbol prefixes to match. if name != derived_name { - let rewrite = |s: &mut String| { + for sym in &mut symbol_reports { + let s = sym.name_mut(); if let Some(rest) = s.strip_prefix(&module_prefix) { *s = format!("{name}.{rest}"); } - }; - for n in &mut names { - rewrite(n); - } - for sym in &mut symbol_reports { - rewrite(sym.name_mut()); } } ModuleReport { name, path, - names, + names: symbol_names(&symbol_reports), line_count, symbol_reports, type_ignores: suppressions, @@ -2864,7 +2854,8 @@ def g(x: int) -> int: .collect(); filter_module_report_to_public(&mut report, &public_fqns); - assert_eq!(report.names, vec!["pkg.Foo", "pkg.bar"]); + // `names` is rebuilt from the retained symbols, not filtered from the input. + assert_eq!(report.names, vec!["pkg.Foo", "pkg.Foo.method", "pkg.bar"]); assert_eq!(report.symbol_reports.len(), 3); // Foo, Foo.method, bar assert_eq!(report.symbols.n_functions, 1); assert_eq!(report.symbols.n_methods, 1);