Skip to content

Dedupe the names bookkeeping in pyrefly coverage #4252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jorenham wants to merge 1 commit into from
Closed

Dedupe the names bookkeeping in pyrefly coverage #4252

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 12 additions & 21 deletions pyrefly/lib/commands/coverage/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String> {
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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -1517,18 +1517,13 @@ 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(),
location: cls.location,
});
}

// 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);
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
Loading