diff --git a/crates/ty_ide/src/code_action.rs b/crates/ty_ide/src/code_action.rs index ee21d81f2c..3fcb33703e 100644 --- a/crates/ty_ide/src/code_action.rs +++ b/crates/ty_ide/src/code_action.rs @@ -29,17 +29,11 @@ pub fn code_actions( let mut actions = Vec::new(); - // Suggest imports for unresolved references (often ideal) - // TODO: suggest qualifying with an already imported symbol + // Suggest imports/qualifications for unresolved references (often ideal) let is_unresolved_reference = lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL); if is_unresolved_reference - && let Some(import_quick_fix) = create_import_symbol_quick_fix(db, file, diagnostic_range) - { - actions.extend(import_quick_fix); - } - if is_unresolved_reference - && let Some(import_quick_fix) = create_qualify_symbol_quick_fix(db, file, diagnostic_range) + && let Some(import_quick_fix) = unresolved_fixes(db, file, diagnostic_range) { actions.extend(import_quick_fix); } @@ -54,7 +48,7 @@ pub fn code_actions( actions } -fn create_import_symbol_quick_fix( +fn unresolved_fixes( db: &dyn Db, file: File, diagnostic_range: TextRange, @@ -64,27 +58,7 @@ fn create_import_symbol_quick_fix( let symbol = &node.expr_name()?.id; Some( - completion::missing_imports(db, file, &parsed, symbol, node) - .into_iter() - .map(|import| QuickFix { - title: import.label, - edits: vec![import.edit], - preferred: true, - }), - ) -} - -fn create_qualify_symbol_quick_fix( - db: &dyn Db, - file: File, - diagnostic_range: TextRange, -) -> Option> { - let parsed = parsed_module(db, file).load(db); - let node = covering_node(parsed.syntax().into(), diagnostic_range).node(); - let symbol = &node.expr_name()?.id; - - Some( - completion::missing_qualifications(db, file, &parsed, symbol, node) + completion::unresolved_fixes(db, file, &parsed, symbol, node) .into_iter() .map(|import| QuickFix { title: import.label, @@ -503,6 +477,108 @@ mod tests { "); } + #[test] + fn unresolved_deprecated() { + let test = CodeActionTest::with_source( + r#" + @deprecated("do not use") + def my_func(): ... + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" + info[code-action]: import warnings.deprecated + --> main.py:2:14 + | + 2 | @deprecated("do not use") + | ^^^^^^^^^^ + 3 | def my_func(): ... + | + help: This is a preferred code action + 1 + from warnings import deprecated + 2 | + 3 | @deprecated("do not use") + 4 | def my_func(): ... + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:2:14 + | + 2 | @deprecated("do not use") + | ^^^^^^^^^^ + 3 | def my_func(): ... + | + 1 | + - @deprecated("do not use") + 2 + @deprecated("do not use") # ty:ignore[unresolved-reference] + 3 | def my_func(): ... + 4 | + "#); + } + + #[test] + fn unresolved_deprecated_warnings_imported() { + let test = CodeActionTest::with_source( + r#" + import warnings + + @deprecated("do not use") + def my_func(): ... + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" + info[code-action]: import warnings.deprecated + --> main.py:4:14 + | + 2 | import warnings + 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + help: This is a preferred code action + 1 + from warnings import deprecated + 2 | + 3 | import warnings + 4 | + + info[code-action]: qualify warnings.deprecated + --> main.py:4:14 + | + 2 | import warnings + 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + help: This is a preferred code action + 1 | + 2 | import warnings + 3 | + - @deprecated("do not use") + 4 + @warnings.deprecated("do not use") + 5 | def my_func(): ... + 6 | + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:4:14 + | + 2 | import warnings + 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + 1 | + 2 | import warnings + 3 | + - @deprecated("do not use") + 4 + @deprecated("do not use") # ty:ignore[unresolved-reference] + 5 | def my_func(): ... + 6 | + "#); + } + pub(super) struct CodeActionTest { pub(super) db: ty_project::TestDb, pub(super) file: File, diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index e06e76faad..b9fcca402f 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -67,6 +67,7 @@ impl<'db> Completions<'db> { self.items } + // Convert this collection into a list of "import..." fixes fn into_imports(mut self) -> Vec { self.items.sort_by(compare_suggestions); self.items @@ -82,25 +83,24 @@ impl<'db> Completions<'db> { .collect() } - fn into_qualified(mut self, range: TextRange) -> Vec { + // Convert this collection into a list of "qualify..." fixes + fn into_qualifications(mut self, range: TextRange) -> Vec { self.items.sort_by(compare_suggestions); self.items .dedup_by(|c1, c2| (&c1.name, c1.module_name) == (&c2.name, c2.module_name)); self.items .into_iter() .filter_map(|item| { - if item.import.is_none() { - Some(ImportEdit { - label: format!("qualify {}", item.insert.as_ref()?), - edit: Edit::replacement( - item.insert?.into_string(), - range.start(), - range.end(), - ), - }) - } else { - None + // If we would have to actually import something, don't suggest the qualification + // (we could, maybe we should, but for now, we don't) + if item.import.is_some() { + return None; } + + Some(ImportEdit { + label: format!("qualify {}", item.insert.as_ref()?), + edit: Edit::replacement(item.insert?.into_string(), range.start(), range.end()), + }) }) .collect() } @@ -578,15 +578,19 @@ pub(crate) struct ImportEdit { pub edit: Edit, } -pub(crate) fn missing_imports( +/// Get fixes that would resolve an unresolved reference +pub(crate) fn unresolved_fixes( db: &dyn Db, file: File, parsed: &ParsedModuleRef, symbol: &str, node: AnyNodeRef, ) -> Vec { - let mut completions = Completions::exactly(db, symbol); + let mut results = Vec::new(); let scoped = ScopedTarget { node }; + + // Request imports we could add to put the symbol in scope + let mut completions = Completions::exactly(db, symbol); add_unimported_completions( db, file, @@ -597,19 +601,10 @@ pub(crate) fn missing_imports( }, &mut completions, ); + results.extend(completions.into_imports()); - completions.into_imports() -} - -pub(crate) fn missing_qualifications( - db: &dyn Db, - file: File, - parsed: &ParsedModuleRef, - symbol: &str, - node: AnyNodeRef, -) -> Vec { + // Request qualifications we could apply to the symbol to make it resolve let mut completions = Completions::exactly(db, symbol); - let scoped = ScopedTarget { node }; add_unimported_completions( db, file, @@ -620,8 +615,9 @@ pub(crate) fn missing_qualifications( }, &mut completions, ); + results.extend(completions.into_qualifications(node.range())); - completions.into_qualified(node.range()) + results } /// Adds completions derived from keywords.