Skip to content

Commit 705f663

Browse files
committed
improve behaviour on encountering invalid syntax and add more tests
1 parent 3fec884 commit 705f663

File tree

6 files changed

+196
-114
lines changed

6 files changed

+196
-114
lines changed

crates/red_knot_python_semantic/resources/mdtest/import/star.md

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,81 @@ reveal_type(X) # revealed: Unknown | Literal[7]
134134
reveal_type(Y) # revealed: Unknown | Literal[3]
135135
```
136136

137+
### Global-scope symbols defined in many other ways
138+
139+
`a.py`:
140+
141+
```py
142+
import typing
143+
from collections import OrderedDict
144+
from collections import OrderedDict as Foo
145+
146+
A, B = 1, (C := 2)
147+
D: (E := 4) = (F := 5) # error: [invalid-type-form]
148+
149+
for G in [1]:
150+
...
151+
152+
for (H := 4).whatever in [2]:
153+
... # error: [unresolved-attribute]
154+
155+
class I: ...
156+
157+
def J(): ...
158+
159+
type K = int
160+
161+
with () as L:
162+
... # error: [invalid-context-manager]
163+
164+
match 42:
165+
case {"something": M}:
166+
...
167+
case [*N]:
168+
...
169+
case [O]:
170+
...
171+
case P | Q:
172+
...
173+
case object(foo=R):
174+
...
175+
case object(S):
176+
...
177+
case T:
178+
...
179+
```
180+
181+
`b.py`:
182+
183+
```py
184+
from a import *
185+
186+
# fmt: off
187+
188+
print((
189+
A,
190+
B,
191+
C,
192+
D,
193+
E,
194+
F,
195+
G, # TODO: could emit diagnostic about being possibly unbound
196+
H,
197+
I,
198+
J,
199+
K,
200+
L,
201+
M, # TODO: could emit diagnostic about being possibly unbound
202+
N, # TODO: could emit diagnostic about being possibly unbound
203+
O, # TODO: could emit diagnostic about being possibly unbound
204+
P, # TODO: could emit diagnostic about being possibly unbound
205+
Q, # TODO: could emit diagnostic about being possibly unbound
206+
R, # TODO: could emit diagnostic about being possibly unbound
207+
S, # TODO: could emit diagnostic about being possibly unbound
208+
T, # TODO: could emit diagnostic about being possibly unbound
209+
))
210+
```
211+
137212
### Global-scope names starting with underscores
138213

139214
Global-scope names starting with underscores are not imported from a `*` import (unless the module
@@ -657,5 +732,30 @@ def f():
657732
reveal_type(X) # revealed: Unknown
658733
```
659734

735+
### `*` combined with other aliases in the list
736+
737+
`a.py`:
738+
739+
```py
740+
X: bool = True
741+
_Y: bool = False
742+
```
743+
744+
`b.py`:
745+
746+
<!-- blacken-docs:off -->
747+
748+
```py
749+
from a import *, _Y # error: [invalid-syntax]
750+
751+
# The import statement above is invalid syntax,
752+
# but it's pretty obvious that the user wanted to do a `*` import,
753+
# so we import all public names from `a` anyway, to minimize cascading errors
754+
reveal_type(X) # revealed: bool
755+
reveal_type(_Y) # revealed: bool
756+
```
757+
758+
<!-- blacken-docs:on -->
759+
660760
[python language reference for import statements]: https://docs.python.org/3/reference/simple_stmts.html#the-import-statement
661761
[typing spec]: https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols

crates/red_knot_python_semantic/src/semantic_index/builder.rs

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -998,40 +998,41 @@ where
998998
}
999999
}
10001000
ast::Stmt::ImportFrom(node) => {
1001-
if node.is_wildcard_import() {
1002-
// Wildcard imports are invalid syntax everywhere except the top-level scope,
1003-
// and thus do not bind any definitions anywhere else
1004-
if self.current_scope() == self.scope_stack[0].file_scope_id {
1005-
if let Ok(module_name) =
1006-
module_name_from_import_statement(self.db, self.file, node)
1007-
{
1008-
if let Some(module) = resolve_module(self.db, &module_name) {
1009-
let exported_names = find_exports(self.db, module.file());
1010-
if !exported_names.is_empty() {
1011-
for export in find_exports(self.db, module.file()) {
1012-
let symbol_id = self.add_symbol(export.clone());
1013-
let node_ref =
1014-
StarImportDefinitionNodeRef { node, symbol_id };
1015-
self.add_definition(symbol_id, node_ref);
1001+
for (alias_index, alias) in node.names.iter().enumerate() {
1002+
if &alias.name == "*" {
1003+
// Wildcard imports are invalid syntax everywhere except the top-level scope,
1004+
// and thus do not bind any definitions anywhere else
1005+
if self.current_scope() == self.scope_stack[0].file_scope_id {
1006+
if let Ok(module_name) =
1007+
module_name_from_import_statement(self.db, self.file, node)
1008+
{
1009+
if let Some(module) = resolve_module(self.db, &module_name) {
1010+
let exported_names = find_exports(self.db, module.file());
1011+
if !exported_names.is_empty() {
1012+
for export in find_exports(self.db, module.file()) {
1013+
let symbol_id = self.add_symbol(export.clone());
1014+
let node_ref =
1015+
StarImportDefinitionNodeRef { node, symbol_id };
1016+
self.add_definition(symbol_id, node_ref);
1017+
}
1018+
continue;
10161019
}
1017-
return;
10181020
}
10191021
}
10201022
}
1023+
1024+
let symbol = self.add_symbol(Name::new_static("*"));
1025+
self.add_definition(
1026+
symbol,
1027+
ImportFromDefinitionNodeRef {
1028+
node,
1029+
alias_index: 0,
1030+
is_reexported: false,
1031+
},
1032+
);
1033+
continue;
10211034
}
1022-
let symbol = self.add_symbol(Name::new_static("*"));
1023-
self.add_definition(
1024-
symbol,
1025-
ImportFromDefinitionNodeRef {
1026-
node,
1027-
alias_index: 0,
1028-
is_reexported: false,
1029-
},
1030-
);
1031-
return;
1032-
}
10331035

1034-
for (alias_index, alias) in node.names.iter().enumerate() {
10351036
let (symbol_name, is_reexported) = if let Some(asname) = &alias.asname {
10361037
(&asname.id, asname.id == alias.name.id)
10371038
} else {

crates/red_knot_python_semantic/src/semantic_index/definition.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,7 @@ impl<'db> DefinitionNodeRef<'db> {
430430
alias_index,
431431
is_reexported: _,
432432
}) => (&node.names[alias_index]).into(),
433-
Self::ImportStar(StarImportDefinitionNodeRef { node, .. }) => {
434-
debug_assert_eq!(node.names.len(), 1);
435-
(&node.names[0]).into()
436-
}
433+
Self::ImportStar(StarImportDefinitionNodeRef { node, .. }) => (&node.names[0]).into(),
437434
Self::Function(node) => node.into(),
438435
Self::Class(node) => node.into(),
439436
Self::TypeAlias(node) => node.into(),

0 commit comments

Comments
 (0)