From d47088c8f83f966f734c254a23073eeee123c347 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Wed, 5 Feb 2025 14:01:58 -0500 Subject: [PATCH] [red-knot] fix unresolvable import range (#15976) This causes the diagnostic to highlight the actual unresovable import instead of the entire `from ... import ...` statement. While we're here, we expand the test coverage to cover all of the possible ways that an `import` or a `from ... import` can fail. Some considerations: * The first commit in this PR adds a regression test for the current behavior. * This creates a new `mdtest/diagnostics` directory. Are folks cool with this? I guess the idea is to put tests more devoted to diagnostics than semantics in this directory. (Although I'm guessing there will be some overlap.) Fixes #15866 --- crates/red_knot/tests/cli.rs | 4 +- .../mdtest/diagnostics/unresolved_import.md | 87 +++++++++++++++++++ ...vable_import_that_does_not_use_`from`.snap | 32 +++++++ ...solvable_module_but_unresolvable_item.snap | 35 ++++++++ ...`from`_with_an_unknown_current_module.snap | 32 +++++++ ..._`from`_with_an_unknown_nested_module.snap | 32 +++++++ ...ng_`from`_with_an_unresolvable_module.snap | 32 +++++++ ...ing_`from`_with_too_many_leading_dots.snap | 44 ++++++++++ .../src/types/infer.rs | 12 ++- 9 files changed, 305 insertions(+), 5 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_An_unresolvable_import_that_does_not_use_`from`.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_a_resolvable_module_but_unresolvable_item.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_current_module.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_nested_module.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unresolvable_module.snap create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_too_many_leading_dots.snap diff --git a/crates/red_knot/tests/cli.rs b/crates/red_knot/tests/cli.rs index 5454308565..7f7e583a17 100644 --- a/crates/red_knot/tests/cli.rs +++ b/crates/red_knot/tests/cli.rs @@ -103,10 +103,10 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> { exit_code: 1 ----- stdout ----- error: lint:unresolved-import - --> /child/test.py:2:1 + --> /child/test.py:2:6 | 2 | from utils import add - | ^^^^^^^^^^^^^^^^^^^^^ Cannot resolve import `utils` + | ^^^^^ Cannot resolve import `utils` 3 | 4 | stat = add(10, 15) | diff --git a/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md b/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md new file mode 100644 index 0000000000..059cb4e5e9 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md @@ -0,0 +1,87 @@ +# Unresolved import diagnostics + + + +## Using `from` with an unresolvable module + +This example demonstrates the diagnostic when a `from` style import is used with a module that could +not be found: + +```py +from does_not_exist import add # error: [unresolved-import] + +stat = add(10, 15) +``` + +## Using `from` with too many leading dots + +This example demonstrates the diagnostic when a `from` style import is used with a presumptively +valid path, but where there are too many leading dots. + +`package/__init__.py`: + +```py +``` + +`package/foo.py`: + +```py +def add(x, y): + return x + y +``` + +`package/subpackage/subsubpackage/__init__.py`: + +```py +from ....foo import add # error: [unresolved-import] + +stat = add(10, 15) +``` + +## Using `from` with an unknown current module + +This is another case handled separately in Red Knot, where a `.` provokes relative module name +resolution, but where the module name is not resolvable. + +```py +from .does_not_exist import add # error: [unresolved-import] + +stat = add(10, 15) +``` + +## Using `from` with an unknown nested module + +Like the previous test, but with sub-modules to ensure the span is correct. + +```py +from .does_not_exist.foo.bar import add # error: [unresolved-import] + +stat = add(10, 15) +``` + +## Using `from` with a resolvable module but unresolvable item + +This ensures that diagnostics for an unresolvable item inside a resolvable import highlight the item +and not the entire `from ... import ...` statement. + +`a.py`: + +```py +does_exist1 = 1 +does_exist2 = 2 +``` + +```py +from a import does_exist1, does_not_exist, does_exist2 # error: [unresolved-import] +``` + +## An unresolvable import that does not use `from` + +This ensures that an unresolvable `import ...` statement highlights just the module name and not the +entire statement. + +```py +import does_not_exist # error: [unresolved-import] + +x = does_not_exist.foo +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_An_unresolvable_import_that_does_not_use_`from`.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_An_unresolvable_import_that_does_not_use_`from`.snap new file mode 100644 index 0000000000..e975741103 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_An_unresolvable_import_that_does_not_use_`from`.snap @@ -0,0 +1,32 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - An unresolvable import that does not use `from` +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## mdtest_snippet__1.py + +``` +1 | import does_not_exist # error: [unresolved-import] +2 | +3 | x = does_not_exist.foo +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/mdtest_snippet__1.py:1:8 + | +1 | import does_not_exist # error: [unresolved-import] + | ^^^^^^^^^^^^^^ Cannot resolve import `does_not_exist` +2 | +3 | x = does_not_exist.foo + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_a_resolvable_module_but_unresolvable_item.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_a_resolvable_module_but_unresolvable_item.snap new file mode 100644 index 0000000000..2764c497f5 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_a_resolvable_module_but_unresolvable_item.snap @@ -0,0 +1,35 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - Using `from` with a resolvable module but unresolvable item +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## a.py + +``` +1 | does_exist1 = 1 +2 | does_exist2 = 2 +``` + +## mdtest_snippet__1.py + +``` +1 | from a import does_exist1, does_not_exist, does_exist2 # error: [unresolved-import] +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/mdtest_snippet__1.py:1:28 + | +1 | from a import does_exist1, does_not_exist, does_exist2 # error: [unresolved-import] + | ^^^^^^^^^^^^^^ Module `a` has no member `does_not_exist` + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_current_module.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_current_module.snap new file mode 100644 index 0000000000..86ee2858eb --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_current_module.snap @@ -0,0 +1,32 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - Using `from` with an unknown current module +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## mdtest_snippet__1.py + +``` +1 | from .does_not_exist import add # error: [unresolved-import] +2 | +3 | stat = add(10, 15) +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/mdtest_snippet__1.py:1:7 + | +1 | from .does_not_exist import add # error: [unresolved-import] + | ^^^^^^^^^^^^^^ Cannot resolve import `.does_not_exist` +2 | +3 | stat = add(10, 15) + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_nested_module.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_nested_module.snap new file mode 100644 index 0000000000..2d5befaad1 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unknown_nested_module.snap @@ -0,0 +1,32 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - Using `from` with an unknown nested module +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## mdtest_snippet__1.py + +``` +1 | from .does_not_exist.foo.bar import add # error: [unresolved-import] +2 | +3 | stat = add(10, 15) +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/mdtest_snippet__1.py:1:7 + | +1 | from .does_not_exist.foo.bar import add # error: [unresolved-import] + | ^^^^^^^^^^^^^^^^^^^^^^ Cannot resolve import `.does_not_exist.foo.bar` +2 | +3 | stat = add(10, 15) + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unresolvable_module.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unresolvable_module.snap new file mode 100644 index 0000000000..cd7a41b2af --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_an_unresolvable_module.snap @@ -0,0 +1,32 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - Using `from` with an unresolvable module +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## mdtest_snippet__1.py + +``` +1 | from does_not_exist import add # error: [unresolved-import] +2 | +3 | stat = add(10, 15) +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/mdtest_snippet__1.py:1:6 + | +1 | from does_not_exist import add # error: [unresolved-import] + | ^^^^^^^^^^^^^^ Cannot resolve import `does_not_exist` +2 | +3 | stat = add(10, 15) + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_too_many_leading_dots.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_too_many_leading_dots.snap new file mode 100644 index 0000000000..603a2137d0 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/unresolved_import.md_-_Unresolved_import_diagnostics_-_Using_`from`_with_too_many_leading_dots.snap @@ -0,0 +1,44 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: unresolved_import.md - Unresolved import diagnostics - Using `from` with too many leading dots +mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unresolved_import.md +--- + +# Python source files + +## package/__init__.py + +``` +``` + +## package/foo.py + +``` +1 | def add(x, y): +2 | return x + y +``` + +## package/subpackage/subsubpackage/__init__.py + +``` +1 | from ....foo import add # error: [unresolved-import] +2 | +3 | stat = add(10, 15) +``` + +# Diagnostics + +``` +error: lint:unresolved-import + --> /src/package/subpackage/subsubpackage/__init__.py:1:10 + | +1 | from ....foo import add # error: [unresolved-import] + | ^^^ Cannot resolve import `....foo` +2 | +3 | stat = add(10, 15) + | + +``` diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index b60ec208d3..da4793d7e7 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2538,6 +2538,12 @@ impl<'db> TypeInferenceBuilder<'db> { // - Absolute `*` imports (`from collections import *`) // - Relative `*` imports (`from ...foo import *`) let ast::StmtImportFrom { module, level, .. } = import_from; + // For diagnostics, we want to highlight the unresolvable + // module and not the entire `from ... import ...` statement. + let module_ref = module + .as_ref() + .map(AnyNodeRef::from) + .unwrap_or_else(|| AnyNodeRef::from(import_from)); let module = module.as_deref(); let module_name = if let Some(level) = NonZeroU32::new(*level) { @@ -2572,7 +2578,7 @@ impl<'db> TypeInferenceBuilder<'db> { "Relative module resolution `{}` failed: too many leading dots", format_import_from_module(*level, module), ); - report_unresolved_module(&self.context, import_from, *level, module); + report_unresolved_module(&self.context, module_ref, *level, module); self.add_unknown_declaration_with_binding(alias.into(), definition); return; } @@ -2582,14 +2588,14 @@ impl<'db> TypeInferenceBuilder<'db> { format_import_from_module(*level, module), self.file().path(self.db()) ); - report_unresolved_module(&self.context, import_from, *level, module); + report_unresolved_module(&self.context, module_ref, *level, module); self.add_unknown_declaration_with_binding(alias.into(), definition); return; } }; let Some(module_ty) = self.module_type_from_name(&module_name) else { - report_unresolved_module(&self.context, import_from, *level, module); + report_unresolved_module(&self.context, module_ref, *level, module); self.add_unknown_declaration_with_binding(alias.into(), definition); return; };