Skip to content

Drop the unused incomplete_attributes from pyrefly coverage #4031

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

Open
jorenham wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −114
Open

Drop the unused incomplete_attributes from pyrefly coverage #4031

Show file tree
Hide file tree
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
114 changes: 7 additions & 107 deletions pyrefly/lib/commands/coverage/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ 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::ClassType;
use pyrefly_types::types::Type;
use pyrefly_util::forgetter::Forgetter;
Expand Down Expand Up @@ -924,24 +923,6 @@ fn return_annotation_range(bindings: &Bindings, return_idx: Idx<Key>) -> Option<
}
}

/// Only the first parameter (`self`/`cls`) is allowed to be unannotated.
fn is_function_completely_annotated(
bindings: &Bindings,
answers: &Answers,
undecorated_idx: Idx<KeyUndecoratedFunction>,
) -> bool {
let fun = bindings.get(undecorated_idx);
let return_idx = bindings.key_to_idx(&Key::ReturnType(ShortIdentifier::new(&fun.def.name)));
if return_annotation_range(bindings, return_idx).is_none() {
return false;
}

let implicit_receiver = has_implicit_receiver(fun, answers, undecorated_idx);
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.
fn is_type_known(ty: &Type) -> bool {
!ty.is_any()
Expand Down Expand Up @@ -1139,37 +1120,17 @@ fn parse_classes(
module: &Module,
bindings: &Bindings,
answers: &Answers,
transaction: &Transaction,
handle: &Handle,
tco_classes: &SmallSet<Idx<KeyClass>>,
) -> Vec<ReportClass> {
let mut classes = Vec::new();
let deleted = bindings.module_deletes();

// group method definitions by class
let mut methods_by_class: HashMap<Idx<KeyClass>, Vec<Idx<KeyUndecoratedFunction>>> =
HashMap::new();
for idx in bindings.keys::<Key>() {
if let Key::Definition(_) = bindings.idx_to_key(idx)
&& let Binding::Function(x, _pred, _class_meta) = bindings.get(idx)
{
let undecorated_idx = bindings.get(*x).undecorated_idx;
if let Some(class_key) = bindings.get(undecorated_idx).class_key {
methods_by_class
.entry(class_key)
.or_default()
.push(undecorated_idx);
}
}
}

for class_idx in bindings.keys::<KeyClass>() {
// Skip @type_check_only classes.
if tco_classes.contains(&class_idx) {
continue;
}
let binding_class = bindings.get(class_idx);
let cls_binding = match binding_class {
let cls_binding = match bindings.get(class_idx) {
BindingClass::ClassDef(cls) => cls,
BindingClass::FunctionalClassDef(..) => continue,
};
Expand All @@ -1183,67 +1144,13 @@ fn parse_classes(
if parent.is_toplevel() && deleted.contains(&name.id) {
continue;
}
let class_type = match answers.get_idx(class_idx) {
Some(result) => match &result.0 {
Some(cls) => cls.clone(),
None => continue,
},
None => continue,
};
let class_name = class_fqn(module, parent, name);
let mro = answers
.get_idx(bindings.key_to_idx(&KeyClassMro(ClassDefIndex(class_type.index().0))))
.unwrap_or_else(|| Arc::new(ClassMro::Cyclic));
// Check methods defined directly on this class
let mut incomplete_attributes = Vec::new();
for &undecorated_idx in methods_by_class.get(&class_idx).into_iter().flatten() {
if !is_function_completely_annotated(bindings, answers, undecorated_idx) {
incomplete_attributes.push(IncompleteAttribute {
name: bindings.get(undecorated_idx).def.name.to_string(),
declared_in: class_name.clone(),
});
}
}
// Check inherited methods
for ancestor_class_type in mro.ancestors_no_object() {
let ancestor_class = ancestor_class_type.class_object();
// Skip methods inherited from builtins
if ancestor_class.module_name().as_str() == "builtins" {
continue;
}
let ancestor_name = class_fqn(
ancestor_class.module(),
ancestor_class.qname().parent(),
ancestor_class.name(),
);
let Some(ancestor_class_fields) = transaction.get_class_fields(handle, ancestor_class)
else {
continue;
};
for field_name in ancestor_class_fields.names() {
let field_name_str = field_name.to_string();
// Skip if we already have this attribute listed (it has been overridden
// by the current class or another class in the MRO)
if incomplete_attributes
.iter()
.any(|a| a.name == field_name_str)
{
continue;
}
if !ancestor_class_fields.is_field_annotated(field_name) {
incomplete_attributes.push(IncompleteAttribute {
name: field_name_str,
declared_in: ancestor_name.clone(),
});
}
}
// Skip classes that fail to resolve.
if answers.get_idx(class_idx).is_none_or(|c| c.0.is_none()) {
continue;
}
let location = range_to_location(module, cls_binding.def.range);
incomplete_attributes.sort();
classes.push(ReportClass {
name: class_name,
incomplete_attributes,
location,
name: class_fqn(module, parent, name),
location: range_to_location(module, cls_binding.def.range),
});
}
classes.sort();
Expand Down Expand Up @@ -1356,14 +1263,7 @@ impl ModuleSymbols {
&tco_classes,
);
merge_overloads(&mut functions);
let classes = parse_classes(
&module,
&bindings,
&answers,
transaction,
handle,
&tco_classes,
);
let classes = parse_classes(&module, &bindings, &answers, &tco_classes);
let mut variables = parse_variables(
&module,
&bindings,
Expand Down
7 changes: 0 additions & 7 deletions pyrefly/lib/commands/coverage/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,9 @@ pub struct Function {
pub range: TextRange,
}

#[derive(Debug, Serialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct IncompleteAttribute {
pub name: String,
pub declared_in: String,
}

#[derive(Debug, Serialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct ReportClass {
pub name: String,
pub incomplete_attributes: Vec<IncompleteAttribute>,
pub location: Location,
}

Expand Down
Loading