diff --git a/pyrefly/lib/commands/report.rs b/pyrefly/lib/commands/report.rs index a8b7898e82..3e9740d1b5 100644 --- a/pyrefly/lib/commands/report.rs +++ b/pyrefly/lib/commands/report.rs @@ -140,6 +140,53 @@ impl SlotCounts { } } +/// Annotation quality for a single slot, ordered worst-to-best so that +/// `min()` gives "worst-wins" semantics. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum SlotRank { + Untyped, + Any, + Typed, + Skip, +} + +impl SlotRank { + /// Map (has_annotation, is_type_known) to a rank. + fn classify(has_annotation: bool, is_type_known: bool) -> Self { + match (has_annotation, is_type_known) { + (false, _) => SlotRank::Untyped, + (true, true) => SlotRank::Typed, + (true, false) => SlotRank::Any, + } + } +} + +impl From<&Parameter> for SlotRank { + fn from(param: &Parameter) -> Self { + SlotRank::classify(param.annotation.is_some(), param.is_type_known) + } +} + +impl From for SlotCounts { + fn from(rank: SlotRank) -> Self { + match rank { + SlotRank::Typed => SlotCounts::typed(), + SlotRank::Any => SlotCounts::any(), + SlotRank::Untyped => SlotCounts::untyped(), + SlotRank::Skip => SlotCounts::default(), + } + } +} + +/// Parameter dedup key for overload merging. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +enum ParamKey { + Positional(usize), + Named(String), + VarPositional, + VarKeyword, +} + #[derive(Debug, Copy, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)] struct Location { line: usize, @@ -153,6 +200,9 @@ struct Parameter { annotation: Option, /// Whether the resolved type contains no `Any`. is_type_known: bool, + /// Overload merge key (`None` for self/cls and implicit params). + #[serde(skip_serializing)] + merge_key: Option, location: Location, } @@ -322,18 +372,32 @@ impl ReportArgs { ) } - fn extract_parameters(params: &Parameters) -> Vec<&ruff_python_ast::Parameter> { - let mut all_params = Vec::new(); - all_params.extend(params.posonlyargs.iter().map(|p| &p.parameter)); - all_params.extend(params.args.iter().map(|p| &p.parameter)); - if let Some(vararg) = ¶ms.vararg { - all_params.push(vararg); + /// All parameters with merge keys: positional-only → index, keyword-capable → name, + /// variadic → singleton, implicit receiver → `None`. + fn params_with_keys( + params: &Parameters, + has_implicit_receiver: bool, + ) -> Vec<(Option, &ruff_python_ast::Parameter)> { + let mut result = Vec::new(); + for (i, p) in params.posonlyargs.iter().enumerate() { + result.push((Some(ParamKey::Positional(i + 1)), &p.parameter)); } - all_params.extend(params.kwonlyargs.iter().map(|p| &p.parameter)); - if let Some(kwarg) = ¶ms.kwarg { - all_params.push(kwarg); + for p in params.args.iter().chain(¶ms.kwonlyargs) { + result.push(( + Some(ParamKey::Named(p.parameter.name.to_string())), + &p.parameter, + )); } - all_params + if let Some(v) = ¶ms.vararg { + result.push((Some(ParamKey::VarPositional), v)); + } + if let Some(v) = ¶ms.kwarg { + result.push((Some(ParamKey::VarKeyword), v)); + } + if has_implicit_receiver && let Some((key, _)) = result.first_mut() { + *key = None; + } + result } fn range_to_location(module: &Module, range: TextRange) -> Location { @@ -405,14 +469,63 @@ impl ReportArgs { } } - /// Classify a single annotation slot: is it typed, any, or untyped? - fn classify_slot(has_annotation: bool, is_type_known: bool) -> SlotCounts { - if !has_annotation { - SlotCounts::untyped() - } else if is_type_known { - SlotCounts::typed() - } else { - SlotCounts::any() + /// Merge overloads with the same qualified name into one entry, + /// keeping the worst annotation quality per deduplicated slot. + fn merge_overloads(functions: &mut Vec) { + let mut groups: HashMap> = HashMap::new(); + for (i, func) in functions.iter().enumerate() { + if !func.is_property { + groups.entry(func.name.clone()).or_default().push(i); + } + } + + let mut to_remove: HashSet = HashSet::new(); + for indices in groups.into_values().filter(|g| g.len() >= 2) { + let mut param_slots: HashMap = HashMap::new(); + let mut return_rank = SlotRank::Skip; + + for &idx in &indices { + let func = &functions[idx]; + let short_name = func.name.rsplit('.').next().unwrap_or(&func.name); + let has_annotation = func.return_annotation.is_some(); + let ret = if !has_annotation && Self::is_implicit_dunder_return(short_name) { + SlotRank::Skip + } else { + SlotRank::classify(has_annotation, func.is_return_type_known) + }; + return_rank = return_rank.min(ret); + + for param in &func.parameters { + if let Some(key) = ¶m.merge_key { + let entry = param_slots.entry(key.clone()).or_insert(SlotRank::Skip); + *entry = (*entry).min(param.into()); + } + } + } + + // Fold deduplicated slots (Skip contributes zero counts). + let slots = std::iter::once(return_rank) + .chain(param_slots.values().copied()) + .map(SlotCounts::from) + .fold(SlotCounts::default(), SlotCounts::merge); + let n_params = param_slots + .into_values() + .filter(|r| *r != SlotRank::Skip) + .count(); + + functions[indices[0]].slots = slots; + functions[indices[0]].n_params = n_params; + functions[indices[0]].is_type_known = slots.n_untyped == 0 && slots.n_any == 0; + to_remove.extend(&indices[1..]); + } + + if !to_remove.is_empty() { + let mut i = 0; + functions.retain(|_| { + let keep = !to_remove.contains(&i); + i += 1; + keep + }); } } @@ -519,7 +632,7 @@ impl ReportArgs { .get_idx(*annot_idx) .and_then(|awt| awt.annotation.ty.as_ref().map(Self::is_type_known)) .unwrap_or(false); - let slots = Self::classify_slot(annotation_text.is_some(), is_type_known); + let slots = SlotRank::classify(annotation_text.is_some(), is_type_known).into(); variables.push(Variable { name: qualified_name, annotation: annotation_text, @@ -714,7 +827,7 @@ impl ReportArgs { .and_then(|awt| awt.annotation.ty.as_ref().map(Self::is_type_known)) }) .unwrap_or(false); - let slots = Self::classify_slot(annotation_text.is_some(), is_type_known); + let slots = SlotRank::classify(annotation_text.is_some(), is_type_known).into(); attrs.push(Variable { name: qualified_name, @@ -824,29 +937,27 @@ impl ReportArgs { .is_some_and(|t| Self::is_type_known(&t)); let mut parameters = Vec::new(); - let all_params = Self::extract_parameters(&fun.def.parameters); + let implicit_receiver = + Self::has_implicit_receiver(fun, answers, decorated.undecorated_idx); + let all_params = Self::params_with_keys(&fun.def.parameters, implicit_receiver); let mut all_params_type_known = true; - // Compute slot counts: return + non-self/cls params. - // Some dunder methods have implicit return types that don't need - // annotation (__init__ → None, __bool__ → bool, __len__ → int, etc.). - // These are always excluded from coverage, even when explicitly annotated. + // Implicit dunder returns (e.g. __init__ → None) are always + // excluded from coverage, even when explicitly annotated. let has_implicit_return = fun.class_key.is_some() && Self::is_implicit_dunder_return(fun.def.name.as_str()); let return_slot = if has_implicit_return { SlotCounts::default() } else { - Self::classify_slot(return_annotation.is_some(), is_return_type_known) + SlotRank::classify(return_annotation.is_some(), is_return_type_known).into() }; let mut func_slots = return_slot; let mut n_params = 0usize; - let fn_name = fun.def.name.as_str(); - let implicit_receiver = - Self::has_implicit_receiver(fun, answers, decorated.undecorated_idx); + let mut non_self_index = 0usize; - for (i, param) in all_params.iter().enumerate() { + for (merge_key, param) in &all_params { let param_name = param.name.as_str(); - let is_self = i == 0 && implicit_receiver; + let is_self = merge_key.is_none(); let param_annotation = param .annotation .as_ref() @@ -866,20 +977,31 @@ impl ReportArgs { false }; - // Check if this non-self param has an implicit type for a dunder method. - // These are always excluded from coverage, even when explicitly annotated. + // Implicit dunder params are always excluded, even when annotated. let is_implicit_param = !is_self && fun.class_key.is_some() - && Self::is_implicit_dunder_param(fn_name, i.saturating_sub(1)); + && Self::is_implicit_dunder_param(fun.def.name.as_str(), non_self_index); - if !is_param_type_known && !is_self && !is_implicit_param { + // self/cls and implicit params are excluded from slot counting. + let effective_key = if is_self || is_implicit_param { + None + } else { + merge_key.clone() + }; + + if !is_self { + non_self_index += 1; + } + + if !is_param_type_known && effective_key.is_some() { all_params_type_known = false; } - if !is_self && !is_implicit_param { - let param_slot = - Self::classify_slot(param_annotation.is_some(), is_param_type_known); - func_slots = func_slots.merge(param_slot); + if effective_key.is_some() { + func_slots = func_slots.merge( + SlotRank::classify(param_annotation.is_some(), is_param_type_known) + .into(), + ); n_params += 1; } @@ -887,6 +1009,7 @@ impl ReportArgs { name: param_name.to_owned(), annotation: param_annotation, is_type_known: is_param_type_known, + merge_key: effective_key, location: Self::range_to_location(module, param.range), }); } @@ -894,8 +1017,7 @@ impl ReportArgs { let is_fully_annotated = return_annotation.is_some() && parameters .iter() - .enumerate() - .all(|(i, p)| (i == 0 && implicit_receiver) || p.annotation.is_some()); + .all(|p| p.merge_key.is_none() || p.annotation.is_some()); let is_type_known = is_fully_annotated && is_return_type_known && all_params_type_known; let is_property = answers @@ -979,18 +1101,10 @@ impl ReportArgs { return false; } - let all_params = Self::extract_parameters(&fun.def.parameters); let implicit_receiver = Self::has_implicit_receiver(fun, answers, undecorated_idx); - for (i, param) in all_params.iter().enumerate() { - if i == 0 && implicit_receiver { - continue; - } - if param.annotation.is_none() { - return false; - } - } - - true + Self::params_with_keys(&fun.def.parameters, implicit_receiver) + .iter() + .all(|(key, param)| key.is_none() || param.annotation.is_some()) } /// Only a bare `Any` counts as unknown; container types like `list[Any]` are known. @@ -1518,6 +1632,7 @@ impl ReportArgs { let tco_classes = Self::collect_type_check_only_classes(&bindings); let mut functions = Self::parse_functions(&module, &bindings, &answers, &exports, &tco_classes); + Self::merge_overloads(&mut functions); let mut classes = Self::parse_classes( &module, &bindings, @@ -1545,13 +1660,14 @@ impl ReportArgs { { let py_exports = transaction.get_exports(py_handle); let py_tco_classes = Self::collect_type_check_only_classes(&py_bindings); - let py_functions = Self::parse_functions( + let mut py_functions = Self::parse_functions( &py_module, &py_bindings, &py_answers, &py_exports, &py_tco_classes, ); + Self::merge_overloads(&mut py_functions); let py_classes = Self::parse_classes( &py_module, &py_bindings, @@ -1695,8 +1811,9 @@ mod tests { let line_count = module.lined_buffer().line_index().line_count(); let tco_classes = ReportArgs::collect_type_check_only_classes(&bindings); - let functions = + let mut functions = ReportArgs::parse_functions(&module, &bindings, &answers, &exports, &tco_classes); + ReportArgs::merge_overloads(&mut functions); let classes = ReportArgs::parse_classes( &module, &bindings, @@ -1775,6 +1892,7 @@ mod tests { let tco_classes = ReportArgs::collect_type_check_only_classes(&bindings); let mut functions = ReportArgs::parse_functions(&module, &bindings, &answers, &exports, &tco_classes); + ReportArgs::merge_overloads(&mut functions); let mut classes = ReportArgs::parse_classes( &module, &bindings, @@ -1808,13 +1926,14 @@ mod tests { let py_exports = py_txn.get_exports(&py_handle); let py_tco_classes = ReportArgs::collect_type_check_only_classes(&py_bindings); - let py_functions = ReportArgs::parse_functions( + let mut py_functions = ReportArgs::parse_functions( &py_module, &py_bindings, &py_answers, &py_exports, &py_tco_classes, ); + ReportArgs::merge_overloads(&mut py_functions); let py_classes = ReportArgs::parse_classes( &py_module, &py_bindings, @@ -2009,15 +2128,25 @@ mod tests { is_return_type_known: has_return, parameters: params .into_iter() - .map(|(param_name, annotated)| Parameter { - name: param_name.to_owned(), - annotation: if annotated { - Some("str".to_owned()) - } else { - None - }, - is_type_known: annotated, - location: Location { line: 1, column: 1 }, + .enumerate() + .map(|(i, (param_name, annotated))| { + let is_self = + i == 0 && (matches!(param_name, "self" | "cls") || name == "__new__"); + Parameter { + name: param_name.to_owned(), + annotation: if annotated { + Some("str".to_owned()) + } else { + None + }, + is_type_known: annotated, + merge_key: if is_self { + None + } else { + Some(ParamKey::Named(param_name.to_owned())) + }, + location: Location { line: 1, column: 1 }, + } }) .collect(), is_type_known: false, // Not relevant for annotation-only tests @@ -2033,11 +2162,10 @@ mod tests { if function.return_annotation.is_none() { return false; } - let implicit_receiver = function.name == "__new__"; - function.parameters.iter().enumerate().all(|(i, p)| { - (i == 0 && (p.name == "self" || p.name == "cls" || implicit_receiver)) - || p.annotation.is_some() - }) + function + .parameters + .iter() + .all(|p| p.merge_key.is_none() || p.annotation.is_some()) } // Fully annotated function @@ -2144,6 +2272,13 @@ mod tests { compare_snapshot("overloads.expected.json", &report); } + /// @overload merging: partial annotations, different param counts, non-overloaded. + #[test] + fn test_report_overloads_partial() { + let report = build_module_report_for_test("overloads_partial.py"); + compare_snapshot("overloads_partial.expected.json", &report); + } + /// @dataclass, Enum, TypedDict, NamedTuple: schema class fields. /// Current: schema class fields are reported as regular variables. /// Typestats: schema fields are IMPLICIT (0 typable). diff --git a/pyrefly/lib/test/report/test_files/overloads.expected.json b/pyrefly/lib/test/report/test_files/overloads.expected.json index ba85c84850..b3f2ad592f 100644 --- a/pyrefly/lib/test/report/test_files/overloads.expected.json +++ b/pyrefly/lib/test/report/test_files/overloads.expected.json @@ -19,18 +19,6 @@ "column": 1 } }, - { - "kind": "function", - "name": "test.f", - "n_typable": 2, - "n_typed": 2, - "n_any": 0, - "n_untyped": 0, - "location": { - "line": 16, - "column": 1 - } - }, { "kind": "function", "name": "test.WithOverloads.method", @@ -43,18 +31,6 @@ "column": 5 } }, - { - "kind": "function", - "name": "test.WithOverloads.method", - "n_typable": 2, - "n_typed": 2, - "n_any": 0, - "n_untyped": 0, - "location": { - "line": 28, - "column": 5 - } - }, { "kind": "class", "name": "test.WithOverloads", @@ -69,16 +45,16 @@ } ], "type_ignores": [], - "n_typable": 8, - "n_typed": 8, + "n_typable": 4, + "n_typed": 4, "n_any": 0, "n_untyped": 0, "coverage": 100.0, "strict_coverage": 100.0, - "n_functions": 2, - "n_methods": 2, - "n_function_params": 2, - "n_method_params": 2, + "n_functions": 1, + "n_methods": 1, + "n_function_params": 1, + "n_method_params": 1, "n_classes": 1, "n_attrs": 0, "n_properties": 0, diff --git a/pyrefly/lib/test/report/test_files/overloads_partial.expected.json b/pyrefly/lib/test/report/test_files/overloads_partial.expected.json new file mode 100644 index 0000000000..0dc8e06947 --- /dev/null +++ b/pyrefly/lib/test/report/test_files/overloads_partial.expected.json @@ -0,0 +1,62 @@ +{ + "name": "test", + "names": [ + "test.partial", + "test.different_params", + "test.standalone" + ], + "line_count": 40, + "symbol_reports": [ + { + "kind": "function", + "name": "test.partial", + "n_typable": 2, + "n_typed": 1, + "n_any": 0, + "n_untyped": 1, + "location": { + "line": 12, + "column": 1 + } + }, + { + "kind": "function", + "name": "test.different_params", + "n_typable": 3, + "n_typed": 3, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 25, + "column": 1 + } + }, + { + "kind": "function", + "name": "test.standalone", + "n_typable": 3, + "n_typed": 3, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 38, + "column": 1 + } + } + ], + "type_ignores": [], + "n_typable": 8, + "n_typed": 7, + "n_any": 0, + "n_untyped": 1, + "coverage": 87.5, + "strict_coverage": 87.5, + "n_functions": 3, + "n_methods": 0, + "n_function_params": 5, + "n_method_params": 0, + "n_classes": 0, + "n_attrs": 0, + "n_properties": 0, + "n_type_ignores": 0 +} diff --git a/pyrefly/lib/test/report/test_files/overloads_partial.py b/pyrefly/lib/test/report/test_files/overloads_partial.py new file mode 100644 index 0000000000..6fcb733fc6 --- /dev/null +++ b/pyrefly/lib/test/report/test_files/overloads_partial.py @@ -0,0 +1,39 @@ +# 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. + +# Overload merging edge cases. + +from typing import overload + + +# Partial: one overload typed, one missing return → worst wins. +@overload +def partial(x: int) -> int: ... + + +@overload +def partial(x: str): ... + + +def partial(x): + return x + + +# Different param counts → union of keys. +@overload +def different_params(x: int) -> int: ... + + +@overload +def different_params(x: int, y: str) -> str: ... + + +def different_params(x, y=None): + return x + + +# Non-overloaded, unaffected by merge. +def standalone(a: int, b: str) -> bool: + return True