[ty] Add "qualify ..." code fix for undefined references (#21968)

## Summary

If `import warnings` exists in the file, we will suggest an edit of
`deprecated -> warnings.deprecated` as "qualify warnings.deprecated"

## Test Plan

Should test more cases...
This commit is contained in:
Aria Desires 2025-12-15 10:14:36 -05:00 committed by GitHub
parent 5372bb3440
commit 8f530a7ab0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 428 additions and 127 deletions

View File

@ -29,12 +29,11 @@ pub fn code_actions(
let mut actions = Vec::new(); let mut actions = Vec::new();
// Suggest imports for unresolved references (often ideal) // Suggest imports/qualifications for unresolved references (often ideal)
// TODO: suggest qualifying with an already imported symbol
let is_unresolved_reference = let is_unresolved_reference =
lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL); lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL);
if is_unresolved_reference if is_unresolved_reference
&& let Some(import_quick_fix) = create_import_symbol_quick_fix(db, file, diagnostic_range) && let Some(import_quick_fix) = unresolved_fixes(db, file, diagnostic_range)
{ {
actions.extend(import_quick_fix); actions.extend(import_quick_fix);
} }
@ -49,7 +48,7 @@ pub fn code_actions(
actions actions
} }
fn create_import_symbol_quick_fix( fn unresolved_fixes(
db: &dyn Db, db: &dyn Db,
file: File, file: File,
diagnostic_range: TextRange, diagnostic_range: TextRange,
@ -59,7 +58,7 @@ fn create_import_symbol_quick_fix(
let symbol = &node.expr_name()?.id; let symbol = &node.expr_name()?.id;
Some( Some(
completion::missing_imports(db, file, &parsed, symbol, node) completion::unresolved_fixes(db, file, &parsed, symbol, node)
.into_iter() .into_iter()
.map(|import| QuickFix { .map(|import| QuickFix {
title: import.label, title: import.label,
@ -84,6 +83,7 @@ mod tests {
system::{DbWithWritableSystem, SystemPathBuf}, system::{DbWithWritableSystem, SystemPathBuf},
}; };
use ruff_diagnostics::Fix; use ruff_diagnostics::Fix;
use ruff_python_trivia::textwrap::dedent;
use ruff_text_size::{TextRange, TextSize}; use ruff_text_size::{TextRange, TextSize};
use ty_project::ProjectMetadata; use ty_project::ProjectMetadata;
use ty_python_semantic::{ use ty_python_semantic::{
@ -149,7 +149,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a / 0 # ty:ignore[division-by-zero] 2 | b = a / 0 # ty:ignore[division-by-zero]
| ^ | ^
@ -157,7 +157,6 @@ mod tests {
1 | 1 |
- b = a / 0 # ty:ignore[division-by-zero] - b = a / 0 # ty:ignore[division-by-zero]
2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference]
3 |
"); ");
} }
@ -171,7 +170,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a / 0 # ty:ignore[division-by-zero,] 2 | b = a / 0 # ty:ignore[division-by-zero,]
| ^ | ^
@ -179,7 +178,6 @@ mod tests {
1 | 1 |
- b = a / 0 # ty:ignore[division-by-zero,] - b = a / 0 # ty:ignore[division-by-zero,]
2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference]
3 |
"); ");
} }
@ -193,7 +191,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a / 0 # ty:ignore[division-by-zero ] 2 | b = a / 0 # ty:ignore[division-by-zero ]
| ^ | ^
@ -201,7 +199,6 @@ mod tests {
1 | 1 |
- b = a / 0 # ty:ignore[division-by-zero ] - b = a / 0 # ty:ignore[division-by-zero ]
2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference ] 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference ]
3 |
"); ");
} }
@ -215,7 +212,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a / 0 # ty:ignore[division-by-zero] some explanation 2 | b = a / 0 # ty:ignore[division-by-zero] some explanation
| ^ | ^
@ -223,7 +220,6 @@ mod tests {
1 | 1 |
- b = a / 0 # ty:ignore[division-by-zero] some explanation - b = a / 0 # ty:ignore[division-by-zero] some explanation
2 + b = a / 0 # ty:ignore[division-by-zero] some explanation # ty:ignore[unresolved-reference] 2 + b = a / 0 # ty:ignore[division-by-zero] some explanation # ty:ignore[unresolved-reference]
3 |
"); ");
} }
@ -241,13 +237,13 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:21 --> main.py:3:9
| |
2 | b = ( 2 | b = (
3 | / a # ty:ignore[division-by-zero] 3 | / a # ty:ignore[division-by-zero]
4 | | / 4 | | /
5 | | 0 5 | | 0
| |_____________________^ | |_________^
6 | ) 6 | )
| |
1 | 1 |
@ -274,13 +270,13 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:21 --> main.py:3:9
| |
2 | b = ( 2 | b = (
3 | / a 3 | / a
4 | | / 4 | | /
5 | | 0 # ty:ignore[division-by-zero] 5 | | 0 # ty:ignore[division-by-zero]
| |_____________________^ | |_________^
6 | ) 6 | )
| |
2 | b = ( 2 | b = (
@ -289,7 +285,6 @@ mod tests {
- 0 # ty:ignore[division-by-zero] - 0 # ty:ignore[division-by-zero]
5 + 0 # ty:ignore[division-by-zero, unresolved-reference] 5 + 0 # ty:ignore[division-by-zero, unresolved-reference]
6 | ) 6 | )
7 |
"); ");
} }
@ -307,13 +302,13 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:21 --> main.py:3:9
| |
2 | b = ( 2 | b = (
3 | / a # ty:ignore[division-by-zero] 3 | / a # ty:ignore[division-by-zero]
4 | | / 4 | | /
5 | | 0 # ty:ignore[division-by-zero] 5 | | 0 # ty:ignore[division-by-zero]
| |_____________________^ | |_________^
6 | ) 6 | )
| |
1 | 1 |
@ -339,7 +334,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:18 --> main.py:3:6
| |
2 | b = f""" 2 | b = f"""
3 | {a} 3 | {a}
@ -352,7 +347,6 @@ mod tests {
4 | more text 4 | more text
- """ - """
5 + """ # ty:ignore[unresolved-reference] 5 + """ # ty:ignore[unresolved-reference]
6 |
"#); "#);
} }
@ -371,7 +365,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:4:17 --> main.py:4:5
| |
2 | b = f""" 2 | b = f"""
3 | { 3 | {
@ -403,7 +397,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a + """ 2 | b = a + """
| ^ | ^
@ -415,7 +409,6 @@ mod tests {
3 | more text 3 | more text
- """ - """
4 + """ # ty:ignore[unresolved-reference] 4 + """ # ty:ignore[unresolved-reference]
5 |
"#); "#);
} }
@ -430,7 +423,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: Ignore 'unresolved-reference' for this line info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:17 --> main.py:2:5
| |
2 | b = a \ 2 | b = a \
| ^ | ^
@ -440,7 +433,6 @@ mod tests {
2 | b = a \ 2 | b = a \
- + "test" - + "test"
3 + + "test" # ty:ignore[unresolved-reference] 3 + + "test" # ty:ignore[unresolved-reference]
4 |
"#); "#);
} }
@ -454,7 +446,7 @@ mod tests {
assert_snapshot!(test.code_actions(&UNDEFINED_REVEAL), @r" assert_snapshot!(test.code_actions(&UNDEFINED_REVEAL), @r"
info[code-action]: import typing.reveal_type info[code-action]: import typing.reveal_type
--> main.py:2:13 --> main.py:2:1
| |
2 | reveal_type(1) 2 | reveal_type(1)
| ^^^^^^^^^^^ | ^^^^^^^^^^^
@ -463,10 +455,9 @@ mod tests {
1 + from typing import reveal_type 1 + from typing import reveal_type
2 | 2 |
3 | reveal_type(1) 3 | reveal_type(1)
4 |
info[code-action]: Ignore 'undefined-reveal' for this line info[code-action]: Ignore 'undefined-reveal' for this line
--> main.py:2:13 --> main.py:2:1
| |
2 | reveal_type(1) 2 | reveal_type(1)
| ^^^^^^^^^^^ | ^^^^^^^^^^^
@ -474,7 +465,230 @@ mod tests {
1 | 1 |
- reveal_type(1) - reveal_type(1)
2 + reveal_type(1) # ty:ignore[undefined-reveal] 2 + reveal_type(1) # ty:ignore[undefined-reveal]
");
}
#[test]
fn unresolved_deprecated() {
let test = CodeActionTest::with_source(
r#"
@<START>deprecated<END>("do not use")
def my_func(): ...
"#,
);
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: import warnings.deprecated
--> main.py:2:2
|
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:2
|
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(): ...
"#);
}
#[test]
fn unresolved_deprecated_warnings_imported() {
let test = CodeActionTest::with_source(
r#"
import warnings
@<START>deprecated<END>("do not use")
def my_func(): ...
"#,
);
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#"
info[code-action]: import warnings.deprecated
--> main.py:4:2
|
2 | import warnings
3 | 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:2
|
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(): ...
info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:4:2
|
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(): ...
"#);
}
// using `importlib.abc.ExecutionLoader` when no imports are in scope
#[test]
fn unresolved_loader() {
let test = CodeActionTest::with_source(
r#"
<START>ExecutionLoader<END>
"#,
);
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: import importlib.abc.ExecutionLoader
--> main.py:2:1
|
2 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
help: This is a preferred code action
1 + from importlib.abc import ExecutionLoader
2 |
3 | ExecutionLoader
info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:2:1
|
2 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
1 |
- ExecutionLoader
2 + ExecutionLoader # ty:ignore[unresolved-reference]
");
}
// using `importlib.abc.ExecutionLoader` when `import importlib` is in scope
//
// TODO: `importlib.abc` is available whenever `importlib` is, so qualifying
// `importlib.abc.ExecutionLoader` without adding imports is actually legal here!
#[test]
fn unresolved_loader_importlib_imported() {
let test = CodeActionTest::with_source(
r#"
import importlib
<START>ExecutionLoader<END>
"#,
);
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: import importlib.abc.ExecutionLoader
--> main.py:3:1
|
2 | import importlib
3 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
help: This is a preferred code action
1 + from importlib.abc import ExecutionLoader
2 |
3 | import importlib
4 | ExecutionLoader
info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:1
|
2 | import importlib
3 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
1 |
2 | import importlib
- ExecutionLoader
3 + ExecutionLoader # ty:ignore[unresolved-reference]
");
}
// Using `importlib.abc.ExecutionLoader` when `import importlib.abc` is in scope
#[test]
fn unresolved_loader_abc_imported() {
let test = CodeActionTest::with_source(
r#"
import importlib.abc
<START>ExecutionLoader<END>
"#,
);
assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r"
info[code-action]: import importlib.abc.ExecutionLoader
--> main.py:3:1
|
2 | import importlib.abc
3 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
help: This is a preferred code action
1 + from importlib.abc import ExecutionLoader
2 |
3 | import importlib.abc
4 | ExecutionLoader
info[code-action]: qualify importlib.abc.ExecutionLoader
--> main.py:3:1
|
2 | import importlib.abc
3 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
help: This is a preferred code action
1 |
2 | import importlib.abc
- ExecutionLoader
3 + importlib.abc.ExecutionLoader
info[code-action]: Ignore 'unresolved-reference' for this line
--> main.py:3:1
|
2 | import importlib.abc
3 | ExecutionLoader
| ^^^^^^^^^^^^^^^
|
1 |
2 | import importlib.abc
- ExecutionLoader
3 + ExecutionLoader # ty:ignore[unresolved-reference]
"); ");
} }
@ -493,7 +707,7 @@ mod tests {
db.init_program().unwrap(); db.init_program().unwrap();
let mut cleansed = source.to_string(); let mut cleansed = dedent(source).to_string();
let start = cleansed let start = cleansed
.find("<START>") .find("<START>")

View File

@ -67,6 +67,7 @@ impl<'db> Completions<'db> {
self.items self.items
} }
// Convert this collection into a list of "import..." fixes
fn into_imports(mut self) -> Vec<ImportEdit> { fn into_imports(mut self) -> Vec<ImportEdit> {
self.items.sort_by(compare_suggestions); self.items.sort_by(compare_suggestions);
self.items self.items
@ -82,6 +83,28 @@ impl<'db> Completions<'db> {
.collect() .collect()
} }
// Convert this collection into a list of "qualify..." fixes
fn into_qualifications(mut self, range: TextRange) -> Vec<ImportEdit> {
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 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()
}
/// Attempts to adds the given completion to this collection. /// Attempts to adds the given completion to this collection.
/// ///
/// When added, `true` is returned. /// When added, `true` is returned.
@ -555,15 +578,19 @@ pub(crate) struct ImportEdit {
pub edit: Edit, pub edit: Edit,
} }
pub(crate) fn missing_imports( /// Get fixes that would resolve an unresolved reference
pub(crate) fn unresolved_fixes(
db: &dyn Db, db: &dyn Db,
file: File, file: File,
parsed: &ParsedModuleRef, parsed: &ParsedModuleRef,
symbol: &str, symbol: &str,
node: AnyNodeRef, node: AnyNodeRef,
) -> Vec<ImportEdit> { ) -> Vec<ImportEdit> {
let mut completions = Completions::exactly(db, symbol); let mut results = Vec::new();
let scoped = ScopedTarget { node }; 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( add_unimported_completions(
db, db,
file, file,
@ -574,8 +601,23 @@ pub(crate) fn missing_imports(
}, },
&mut completions, &mut completions,
); );
results.extend(completions.into_imports());
completions.into_imports() // Request qualifications we could apply to the symbol to make it resolve
let mut completions = Completions::exactly(db, symbol);
add_unimported_completions(
db,
file,
parsed,
scoped,
|module_name: &ModuleName, symbol: &str| {
ImportRequest::import(module_name.as_str(), symbol).force()
},
&mut completions,
);
results.extend(completions.into_qualifications(node.range()));
results
} }
/// Adds completions derived from keywords. /// Adds completions derived from keywords.

View File

@ -48,6 +48,51 @@ expression: code_actions
}, },
"isPreferred": true "isPreferred": true
}, },
{
"title": "qualify warnings.deprecated",
"kind": "quickfix",
"diagnostics": [
{
"range": {
"start": {
"line": 3,
"character": 1
},
"end": {
"line": 3,
"character": 11
}
},
"severity": 1,
"code": "unresolved-reference",
"codeDescription": {
"href": "https://ty.dev/rules#unresolved-reference"
},
"source": "ty",
"message": "Name `deprecated` used when not defined"
}
],
"edit": {
"changes": {
"file://<temp_dir>/src/foo.py": [
{
"range": {
"start": {
"line": 3,
"character": 1
},
"end": {
"line": 3,
"character": 11
}
},
"newText": "warnings.deprecated"
}
]
}
},
"isPreferred": true
},
{ {
"title": "Ignore 'unresolved-reference' for this line", "title": "Ignore 'unresolved-reference' for this line",
"kind": "quickfix", "kind": "quickfix",