From 636e69e1efdb83531b1b1411df162fcdfbae7e38 Mon Sep 17 00:00:00 2001 From: jorenham Date: Thu, 21 May 2026 16:10:12 +0200 Subject: [PATCH] Fix inherited stub method.counts in `pyrefly coverage report` --- pyrefly/lib/commands/report.rs | 90 ++++++++++++++++++- .../stub_inherited_methods.expected.json | 63 +++++++++++++ .../test_files/stub_inherited_methods.py | 14 +++ .../test_files/stub_inherited_methods.pyi | 9 ++ 4 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 pyrefly/lib/test/report/test_files/stub_inherited_methods.expected.json create mode 100644 pyrefly/lib/test/report/test_files/stub_inherited_methods.py create mode 100644 pyrefly/lib/test/report/test_files/stub_inherited_methods.pyi diff --git a/pyrefly/lib/commands/report.rs b/pyrefly/lib/commands/report.rs index 444f54d0cc..9c1ee6d23c 100644 --- a/pyrefly/lib/commands/report.rs +++ b/pyrefly/lib/commands/report.rs @@ -24,7 +24,7 @@ use pyrefly_python::module_path::ModuleStyle; use pyrefly_python::nesting_context::NestingContext; use pyrefly_python::short_identifier::ShortIdentifier; use pyrefly_types::callable::PropertyRole; -use pyrefly_types::class::ClassDefIndex; +use pyrefly_types::class::{ClassDefIndex, ClassType}; use pyrefly_types::types::Type; use pyrefly_util::forgetter::Forgetter; use pyrefly_util::includes::Includes; @@ -1690,6 +1690,60 @@ impl ReportArgs { .collect() } + /// `module.Cls.member` names for each public class, including MRO-inherited ones. + fn collect_class_members( + module: &Module, + bindings: &Bindings, + answers: &Answers, + transaction: &Transaction, + handle: &Handle, + tco_classes: &SmallSet>, + ) -> SmallSet { + let fqname_prefix = if module.name() != ModuleName::unknown() { + format!("{}.", module.name()) + } else { + String::new() + }; + + let mut members = SmallSet::new(); + for idx in bindings.keys::() { + if tco_classes.contains(&idx) { + continue; + } + let BindingClass::ClassDef(binding) = bindings.get(idx) else { + continue; + }; + if Self::has_function_ancestor(&binding.parent) { + continue; + } + let Some(cls) = answers.get_idx(idx).and_then(|r| r.0.clone()) else { + continue; + }; + + let qname = Self::class_qualified_name(module, &binding.parent, &binding.def.name); + let fqname = format!("{fqname_prefix}{qname}"); + + let mro = answers + .get_idx(bindings.key_to_idx(&KeyClassMro(cls.index()))) + .unwrap_or_else(|| Arc::new(ClassMro::Cyclic)); + let mro: &[_] = match mro.as_ref() { + ClassMro::Resolved(a) => a, + ClassMro::Cyclic => &[], + }; + for obj in std::iter::once(&cls).chain(mro.iter().map(ClassType::class_object)) { + if obj.module_name().as_str() == "builtins" { + continue; + } + if let Some(fields) = transaction.get_class_fields(handle, obj) { + for name in fields.names() { + members.insert(format!("{fqname}.{name}")); + } + } + } + } + members + } + /// When a `.pyi` stub only covers a subset of a `.py` file's public /// symbols, add the uncovered symbols from the `.py` so that completeness /// metrics reflect the full module interface. @@ -1700,11 +1754,14 @@ impl ReportArgs { py_functions: Vec, py_variables: Vec, py_classes: Vec, + stub_class_members: &SmallSet, ) { let stub_func_names: SmallSet = stub_functions.iter().map(|f| f.name.clone()).collect(); for py_func in py_functions { - if !stub_func_names.contains(&py_func.name) { + if !stub_func_names.contains(&py_func.name) + && !stub_class_members.contains(&py_func.name) + { stub_functions.push(py_func); } } @@ -1712,7 +1769,8 @@ impl ReportArgs { let stub_var_names: SmallSet = stub_variables.iter().map(|v| v.name.clone()).collect(); for py_var in py_variables { - if !stub_var_names.contains(&py_var.name) { + if !stub_var_names.contains(&py_var.name) && !stub_class_members.contains(&py_var.name) + { stub_variables.push(py_var); } } @@ -2005,6 +2063,14 @@ impl ReportArgs { &py_answers, &py_tco_classes, )); + let stub_class_members = Self::collect_class_members( + &module, + &bindings, + &answers, + transaction, + handle, + &tco_classes, + ); Self::merge_uncovered_py_symbols( &mut functions, &mut variables, @@ -2012,6 +2078,7 @@ impl ReportArgs { py_functions, py_variables, py_classes, + &stub_class_members, ); } @@ -2283,6 +2350,14 @@ mod tests { )); // Merge uncovered symbols from .py into the stub report + let stub_class_members = ReportArgs::collect_class_members( + &module, + &bindings, + &answers, + &pyi_txn, + &pyi_handle, + &tco_classes, + ); ReportArgs::merge_uncovered_py_symbols( &mut functions, &mut variables, @@ -2290,6 +2365,7 @@ mod tests { py_functions, py_variables, py_classes, + &stub_class_members, ); ReportArgs::build_module_report( @@ -2354,6 +2430,14 @@ mod tests { compare_snapshot("partial_stub.expected.json", &report); } + /// gh-3519: don't double-count methods whose stub coverage is inherited. + #[test] + fn test_report_inherited_method_via_stub() { + let report = + build_stub_module_report("stub_inherited_methods.pyi", "stub_inherited_methods.py"); + compare_snapshot("stub_inherited_methods.expected.json", &report); + } + /// Stubs-only packages: .py discovered via site-package-path, merged like co-located stubs. #[test] fn test_report_external_stub_merge() { diff --git a/pyrefly/lib/test/report/test_files/stub_inherited_methods.expected.json b/pyrefly/lib/test/report/test_files/stub_inherited_methods.expected.json new file mode 100644 index 0000000000..71acabaa27 --- /dev/null +++ b/pyrefly/lib/test/report/test_files/stub_inherited_methods.expected.json @@ -0,0 +1,63 @@ +{ + "name": "test", + "path": "test.pyi", + "names": [ + "test.Base.m", + "test.Base", + "test.Sub" + ], + "line_count": 10, + "symbol_reports": [ + { + "kind": "function", + "name": "test.Base.m", + "n_typable": 2, + "n_typed": 2, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 7, + "column": 5 + } + }, + { + "kind": "class", + "name": "test.Base", + "n_typable": 0, + "n_typed": 0, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 6, + "column": 1 + } + }, + { + "kind": "class", + "name": "test.Sub", + "n_typable": 0, + "n_typed": 0, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 9, + "column": 1 + } + } + ], + "type_ignores": [], + "n_typable": 2, + "n_typed": 2, + "n_any": 0, + "n_untyped": 0, + "coverage": 100.0, + "strict_coverage": 100.0, + "n_functions": 0, + "n_methods": 1, + "n_function_params": 0, + "n_method_params": 1, + "n_classes": 2, + "n_attrs": 0, + "n_properties": 0, + "n_type_ignores": 0 +} diff --git a/pyrefly/lib/test/report/test_files/stub_inherited_methods.py b/pyrefly/lib/test/report/test_files/stub_inherited_methods.py new file mode 100644 index 0000000000..e1f45b868a --- /dev/null +++ b/pyrefly/lib/test/report/test_files/stub_inherited_methods.py @@ -0,0 +1,14 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + + +class Base: + def m(self, x): + return None + + +class Sub(Base): + def m(self, x): + return None diff --git a/pyrefly/lib/test/report/test_files/stub_inherited_methods.pyi b/pyrefly/lib/test/report/test_files/stub_inherited_methods.pyi new file mode 100644 index 0000000000..3efe9afe97 --- /dev/null +++ b/pyrefly/lib/test/report/test_files/stub_inherited_methods.pyi @@ -0,0 +1,9 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +class Base: + def m(self, x: int) -> None: ... + +class Sub(Base): ...