Skip to content

Commit

Permalink
Prioritize attribute in from/import statement (#15041)
Browse files Browse the repository at this point in the history
This tweaks the new semantics from #15026 a bit when a symbol could be
interpreted both as an attribute and a submodule of a package. For
`from...import`, we should actually prioritize the attribute, because of
how the statement itself is implemented [1].

> 1. check if the imported module has an attribute by that name
> 2. if not, attempt to import a submodule with that name and then check
the imported module again for that attribute

[1] https://docs.python.org/3/reference/simple_stmts.html#the-import-statement
  • Loading branch information
dcreager authored Dec 17, 2024
1 parent 91c9168 commit e8e461d
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Conflicting attributes and submodules

## Via import

```py
import a.b

reveal_type(a.b) # revealed: <module 'a.b'>
```

```py path=a/__init__.py
b = 42
```

```py path=a/b.py
```

## Via from/import

```py
from a import b

reveal_type(b) # revealed: Literal[42]
```

```py path=a/__init__.py
b = 42
```

```py path=a/b.py
```

## Via both

```py
import a.b
from a import b

reveal_type(b) # revealed: <module 'a.b'>
reveal_type(a.b) # revealed: <module 'a.b'>
```

```py path=a/__init__.py
b = 42
```

```py path=a/b.py
```

## Via both (backwards)

In this test, we infer a different type for `b` than the runtime behavior of the Python interpreter.
The interpreter will not load the submodule `a.b` during the `from a import b` statement, since `a`
contains a non-module attribute named `b`. (See the [definition][from-import] of a `from...import`
statement for details.) However, because our import tracking is flow-insensitive, we will see that
`a.b` is imported somewhere in the file, and therefore assume that the `from...import` statement
sees the submodule as the value of `b` instead of the integer.

```py
from a import b
import a.b

# Python would say `Literal[42]` for `b`
reveal_type(b) # revealed: <module 'a.b'>
reveal_type(a.b) # revealed: <module 'a.b'>
```

```py path=a/__init__.py
b = 42
```

```py path=a/b.py
```

[from-import]: https://docs.python.org/3/reference/simple_stmts.html#the-import-statement
51 changes: 26 additions & 25 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2307,11 +2307,26 @@ impl<'db> TypeInferenceBuilder<'db> {
asname: _,
} = alias;

// Check if the symbol being imported is a submodule. This won't get handled by the
// `Type::member` call below because it relies on the semantic index's `imported_modules`
// set. The semantic index does not include information about `from...import` statements
// because there are two things it cannot determine while only inspecting the content of
// the current file:
// First try loading the requested attribute from the module.
if let Symbol::Type(ty, boundness) = module_ty.member(self.db, name) {
if boundness == Boundness::PossiblyUnbound {
// TODO: Consider loading _both_ the attribute and any submodule and unioning them
// together if the attribute exists but is possibly-unbound.
self.diagnostics.add_lint(
&POSSIBLY_UNBOUND_IMPORT,
AnyNodeRef::Alias(alias),
format_args!("Member `{name}` of module `{module_name}` is possibly unbound",),
);
}
self.add_declaration_with_binding(alias.into(), definition, ty, ty);
return;
};

// If the module doesn't bind the symbol, check if it's a submodule. This won't get
// handled by the `Type::member` call because it relies on the semantic index's
// `imported_modules` set. The semantic index does not include information about
// `from...import` statements because there are two things it cannot determine while only
// inspecting the content of the current file:
//
// - whether the imported symbol is an attribute or submodule
// - whether the containing file is in a module or a package (needed to correctly resolve
Expand All @@ -2336,26 +2351,12 @@ impl<'db> TypeInferenceBuilder<'db> {
}
}

// Otherwise load the requested attribute from the module.
let Symbol::Type(ty, boundness) = module_ty.member(self.db, name) else {
self.diagnostics.add_lint(
&UNRESOLVED_IMPORT,
AnyNodeRef::Alias(alias),
format_args!("Module `{module_name}` has no member `{name}`",),
);
self.add_unknown_declaration_with_binding(alias.into(), definition);
return;
};

if boundness == Boundness::PossiblyUnbound {
self.diagnostics.add_lint(
&POSSIBLY_UNBOUND_IMPORT,
AnyNodeRef::Alias(alias),
format_args!("Member `{name}` of module `{module_name}` is possibly unbound",),
);
}

self.add_declaration_with_binding(alias.into(), definition, ty, ty);
self.diagnostics.add_lint(
&UNRESOLVED_IMPORT,
AnyNodeRef::Alias(alias),
format_args!("Module `{module_name}` has no member `{name}`",),
);
self.add_unknown_declaration_with_binding(alias.into(), definition);
}

fn infer_return_statement(&mut self, ret: &ast::StmtReturn) {
Expand Down

0 comments on commit e8e461d

Please sign in to comment.