mirror of https://github.com/astral-sh/ruff
[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
This commit is contained in:
parent
1f0ad675d3
commit
d47088c8f8
|
|
@ -103,10 +103,10 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> {
|
||||||
exit_code: 1
|
exit_code: 1
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
error: lint:unresolved-import
|
error: lint:unresolved-import
|
||||||
--> <temp_dir>/child/test.py:2:1
|
--> <temp_dir>/child/test.py:2:6
|
||||||
|
|
|
|
||||||
2 | from utils import add
|
2 | from utils import add
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^ Cannot resolve import `utils`
|
| ^^^^^ Cannot resolve import `utils`
|
||||||
3 |
|
3 |
|
||||||
4 | stat = add(10, 15)
|
4 | stat = add(10, 15)
|
||||||
|
|
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,87 @@
|
||||||
|
# Unresolved import diagnostics
|
||||||
|
|
||||||
|
<!-- snapshot-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
|
||||||
|
```
|
||||||
|
|
@ -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
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -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`
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -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)
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -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)
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -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)
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -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)
|
||||||
|
|
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -2538,6 +2538,12 @@ impl<'db> TypeInferenceBuilder<'db> {
|
||||||
// - Absolute `*` imports (`from collections import *`)
|
// - Absolute `*` imports (`from collections import *`)
|
||||||
// - Relative `*` imports (`from ...foo import *`)
|
// - Relative `*` imports (`from ...foo import *`)
|
||||||
let ast::StmtImportFrom { module, level, .. } = import_from;
|
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 = module.as_deref();
|
||||||
|
|
||||||
let module_name = if let Some(level) = NonZeroU32::new(*level) {
|
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",
|
"Relative module resolution `{}` failed: too many leading dots",
|
||||||
format_import_from_module(*level, module),
|
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);
|
self.add_unknown_declaration_with_binding(alias.into(), definition);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -2582,14 +2588,14 @@ impl<'db> TypeInferenceBuilder<'db> {
|
||||||
format_import_from_module(*level, module),
|
format_import_from_module(*level, module),
|
||||||
self.file().path(self.db())
|
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);
|
self.add_unknown_declaration_with_binding(alias.into(), definition);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let Some(module_ty) = self.module_type_from_name(&module_name) else {
|
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);
|
self.add_unknown_declaration_with_binding(alias.into(), definition);
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue