Skip to content

feat: improve suggestion hints across name and attribute errors #1705

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
wants to merge 1 commit into
base: main
Choose a base branch
from
+385 −65
Open

feat: improve suggestion hints across name and attribute errors #1705

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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 17 additions & 17 deletions conformance/third_party/conformance.exp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
"code": -2,
"column": 23,
"concise_description": "Could not find name `a`",
"description": "Could not find name `a`",
"description": "Could not find name `a`\n Did you mean `S`?",
"line": 83,
"name": "unknown-name",
"severity": "error",
Expand All @@ -126,7 +126,7 @@
"code": -2,
"column": 28,
"concise_description": "Could not find name `b`",
"description": "Could not find name `b`",
"description": "Could not find name `b`\n Did you mean `S`?",
"line": 83,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -1005,7 +1005,7 @@
"code": -2,
"column": 44,
"concise_description": "Could not find name `a`",
"description": "Could not find name `a`",
"description": "Could not find name `a`\n Did you mean `S`?",
"line": 56,
"name": "unknown-name",
"severity": "error",
Expand All @@ -1016,7 +1016,7 @@
"code": -2,
"column": 49,
"concise_description": "Could not find name `b`",
"description": "Could not find name `b`",
"description": "Could not find name `b`\n Did you mean `S`?",
"line": 56,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -1284,7 +1284,7 @@
"code": -2,
"column": 14,
"concise_description": "Could not find name `ClassF`",
"description": "Could not find name `ClassF`",
"description": "Could not find name `ClassF`\n Did you mean `ClassC`?",
"line": 80,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -2220,7 +2220,7 @@
"code": -2,
"column": 7,
"concise_description": "Object of class `Proto9` has no attribute `other_attribute2`",
"description": "Object of class `Proto9` has no attribute `other_attribute2`",
"description": "Object of class `Proto9` has no attribute `other_attribute2`\n Did you mean `other_attribute`?",
"line": 197,
"name": "missing-attribute",
"severity": "error",
Expand Down Expand Up @@ -2653,7 +2653,7 @@
"code": -2,
"column": 14,
"concise_description": "Could not find name `var`",
"description": "Could not find name `var`",
"description": "Could not find name `var`\n Did you mean `vars`?",
"line": 40,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -3585,7 +3585,7 @@
"code": -2,
"column": 7,
"concise_description": "Object of class `DC1` has no attribute `x`",
"description": "Object of class `DC1` has no attribute `x`",
"description": "Object of class `DC1` has no attribute `x`\n Did you mean `a`?",
"line": 28,
"name": "missing-attribute",
"severity": "error",
Expand All @@ -3596,7 +3596,7 @@
"code": -2,
"column": 7,
"concise_description": "Object of class `DC1` has no attribute `y`",
"description": "Object of class `DC1` has no attribute `y`",
"description": "Object of class `DC1` has no attribute `y`\n Did you mean `a`?",
"line": 29,
"name": "missing-attribute",
"severity": "error",
Expand Down Expand Up @@ -8082,7 +8082,7 @@
"code": -2,
"column": 1,
"concise_description": "Cannot set item in `Point`",
"description": "Cannot set item in `Point`\n Object of class `Point` has no attribute `__setitem__`",
"description": "Cannot set item in `Point`\n Object of class `Point` has no attribute `__setitem__`\n Did you mean `__getitem__`?",
"line": 41,
"name": "unsupported-operation",
"severity": "error",
Expand All @@ -8104,7 +8104,7 @@
"code": -2,
"column": 5,
"concise_description": "Cannot delete item in `Point`",
"description": "Cannot delete item in `Point`\n Object of class `Point` has no attribute `__delitem__`",
"description": "Cannot delete item in `Point`\n Object of class `Point` has no attribute `__delitem__`\n Did you mean `__getitem__`?",
"line": 43,
"name": "unsupported-operation",
"severity": "error",
Expand Down Expand Up @@ -9270,7 +9270,7 @@
"code": -2,
"column": 19,
"concise_description": "Could not find name `a`",
"description": "Could not find name `a`",
"description": "Could not find name `a`\n Did you mean `T`?",
"line": 41,
"name": "unknown-name",
"severity": "error",
Expand All @@ -9281,7 +9281,7 @@
"code": -2,
"column": 24,
"concise_description": "Could not find name `b`",
"description": "Could not find name `b`",
"description": "Could not find name `b`\n Did you mean `T`?",
"line": 41,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -9325,7 +9325,7 @@
"code": -2,
"column": 17,
"concise_description": "Could not find name `var1`",
"description": "Could not find name `var1`",
"description": "Could not find name `var1`\n Did you mean `vars`?",
"line": 45,
"name": "unknown-name",
"severity": "error",
Expand Down Expand Up @@ -11093,7 +11093,7 @@
"code": -2,
"column": 1,
"concise_description": "Object of class `Movie` has no attribute `clear`",
"description": "Object of class `Movie` has no attribute `clear`",
"description": "Object of class `Movie` has no attribute `clear`\n Did you mean `year`?",
"line": 47,
"name": "missing-attribute",
"severity": "error",
Expand All @@ -11115,7 +11115,7 @@
"code": -2,
"column": 1,
"concise_description": "Object of class `MovieOptional` has no attribute `clear`",
"description": "Object of class `MovieOptional` has no attribute `clear`",
"description": "Object of class `MovieOptional` has no attribute `clear`\n Did you mean `year`?",
"line": 62,
"name": "missing-attribute",
"severity": "error",
Expand Down Expand Up @@ -11514,7 +11514,7 @@
"code": -2,
"column": 21,
"concise_description": "Key `y` is not defined in TypedDict `A3`",
"description": "Key `y` is not defined in TypedDict `A3`",
"description": "Key `y` is not defined in TypedDict `A3`\n Did you mean `x`?",
"line": 69,
"name": "bad-typed-dict-key",
"severity": "error",
Expand Down
1 change: 1 addition & 0 deletions pyrefly/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ serde_json = { version = "1.0.140", features = ["alloc", "float_roundtrip", "raw
serde_repr = "0.1.14"
starlark_map = "0.13.0"
static_assertions = "1.1.0"
strsim = "0.11.1"
tempfile = "3.22"
tokio = { version = "1.47.1", features = ["macros", "rt"] }
toml = { version = "0.9.8", features = ["preserve_order"] }
Expand Down
82 changes: 77 additions & 5 deletions pyrefly/lib/alt/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use ruff_python_ast::name::Name;
use ruff_text_size::TextRange;
use starlark_map::small_set::SmallSet;
use vec1::Vec1;
use vec1::vec1;

use crate::alt::answers::LookupAnswer;
use crate::alt::answers_solver::AnswersSolver;
Expand All @@ -41,6 +42,7 @@ use crate::export::exports::Export;
use crate::export::exports::ExportLocation;
use crate::export::exports::Exports;
use crate::solver::solver::SubsetError;
use crate::suggest::best_suggestion;
use crate::types::callable::FuncMetadata;
use crate::types::callable::Function;
use crate::types::callable::FunctionKind;
Expand Down Expand Up @@ -518,7 +520,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
todo_ctx: &str,
) -> Type {
let attr_base = self.as_attribute_base(base.clone());
let lookup_result = attr_base.map_or_else(
let lookup_result = attr_base.clone().map_or_else(
|| LookupResult::internal_error(InternalError::AttributeBaseUndefined(base.clone())),
|attr_base| self.lookup_attr_from_base(attr_base, attr_name),
);
Expand All @@ -545,13 +547,83 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
} else {
error_messages.sort();
error_messages.dedup();
self.error(
errors,
let mut msg = vec1![error_messages.join("\n")];
if let Some(suggestion) = attr_base
.as_ref()
.and_then(|attr_base| self.suggest_attribute_name(attr_name, attr_base))
{
msg.push(format!("Did you mean `{suggestion}`?"));
}
errors.add(
range,
ErrorInfo::new(ErrorKind::MissingAttribute, context),
error_messages.join("\n"),
)
msg,
);
Type::any_error()
}
}

fn add_class_fields(&self, class: &Class, candidates: &mut SmallSet<Name>) {
let mut add_fields_for = |cls: &Class| {
for name in self.get_class_field_map(cls).keys() {
candidates.insert(name.clone());
}
};
add_fields_for(class);
let mro = self.get_mro_for_class(class);
for ancestor in mro.ancestors_no_object() {
add_fields_for(ancestor.class_object());
}
}

fn collect_attribute_candidates_from_base(
&self,
base: &AttributeBase1,
candidates: &mut SmallSet<Name>,
) {
match base {
AttributeBase1::ClassInstance(class) => {
self.add_class_fields(class.class_object(), candidates);
}
AttributeBase1::ClassObject(class_base) => {
self.add_class_fields(class_base.class_object(), candidates);
}
AttributeBase1::LiteralString => {
self.add_class_fields(self.stdlib.str().class_object(), candidates);
}
AttributeBase1::TypedDict(td) => {
self.add_class_fields(td.class_object(), candidates);
}
AttributeBase1::SuperInstance(class, ..) => {
self.add_class_fields(class.class_object(), candidates);
}
AttributeBase1::Quantified(_, class_type) => {
self.add_class_fields(class_type.class_object(), candidates);
}
AttributeBase1::SelfType(class_type) => {
self.add_class_fields(class_type.class_object(), candidates);
}
AttributeBase1::ProtocolSubset(inner) => {
self.collect_attribute_candidates_from_base(inner, candidates);
}
AttributeBase1::Intersect(lhs, rhs) => {
for b in lhs {
self.collect_attribute_candidates_from_base(b, candidates);
}
for b in rhs {
self.collect_attribute_candidates_from_base(b, candidates);
}
}
_ => {}
}
}

fn suggest_attribute_name(&self, missing: &Name, attr_base: &AttributeBase) -> Option<Name> {
let mut candidates = SmallSet::new();
for base in attr_base.0.iter() {
self.collect_attribute_candidates_from_base(base, &mut candidates);
}
best_suggestion(missing, candidates.iter().map(|candidate| (candidate, 0)))
}

/// Can the attribute be successfully looked up in all cases?
Expand Down
21 changes: 14 additions & 7 deletions pyrefly/lib/alt/class/typed_dict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use crate::error::context::ErrorInfo;
use crate::error::context::TypeCheckContext;
use crate::error::context::TypeCheckKind;
use crate::solver::solver::SubsetError;
use crate::suggest::best_suggestion;
use crate::types::annotation::Qualifier;
use crate::types::callable::Callable;
use crate::types::callable::FuncMetadata;
Expand Down Expand Up @@ -120,15 +121,21 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
);
}
None => {
self.error(
check_errors,
let mut msg = vec1![format!(
"Key `{}` is not defined in TypedDict `{}`",
key_name,
typed_dict.name()
)];
if let Some(suggestion) = best_suggestion(
&key_name,
fields.keys().map(|candidate| (candidate, 0usize)),
) {
msg.push(format!("Did you mean `{suggestion}`?"));
}
check_errors.add(
key.range(),
ErrorInfo::Kind(ErrorKind::BadTypedDictKey),
format!(
"Key `{}` is not defined in TypedDict `{}`",
key_name,
typed_dict.name()
),
msg,
);
}
}
Expand Down
29 changes: 18 additions & 11 deletions pyrefly/lib/alt/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ use crate::error::collector::ErrorCollector;
use crate::error::context::ErrorContext;
use crate::error::context::ErrorInfo;
use crate::error::context::TypeCheckContext;
use crate::suggest::best_suggestion;
use crate::types::callable::Callable;
use crate::types::callable::Param;
use crate::types::callable::ParamList;
Expand Down Expand Up @@ -2126,25 +2127,31 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
let key_ty = self.expr_infer(slice, errors);
self.distribute_over_union(&key_ty, |ty| match ty {
Type::Literal(Lit::Str(field_name)) => {
if let Some(field) =
self.typed_dict_field(&typed_dict, &Name::new(field_name))
{
let fields = self.typed_dict_fields(&typed_dict);
if let Some(field) = fields.get(&Name::new(field_name)) {
field.ty.clone()
} else if let ExtraItems::Extra(extra) =
self.typed_dict_extra_items(typed_dict.class_object())
{
extra.ty
} else {
self.error(
errors,
let mut msg = vec1![format!(
"TypedDict `{}` does not have key `{}`",
typed_dict.name(),
field_name
)];
if let Some(suggestion) = best_suggestion(
&Name::new(field_name),
fields.keys().map(|candidate| (candidate, 0usize)),
) {
msg.push(format!("Did you mean `{suggestion}`?"));
}
errors.add(
slice.range(),
ErrorInfo::Kind(ErrorKind::BadTypedDictKey),
format!(
"TypedDict `{}` does not have key `{}`",
typed_dict.name(),
field_name
),
)
msg,
);
Type::any_error()
}
}
Type::ClassType(cls)
Expand Down
Loading