Skip to content

Commit

Permalink
use Never type instead of Unknown for unbound public types
Browse files Browse the repository at this point in the history
Uses `Type::Never` instead of `Type::Unknown` for the case where
a publicly available variable is unbound. In the case where it is
unbound, we want a union of its actual type with `Never` instead of
`Unbound`, because the `Unbound` case will cause runtime error.
  • Loading branch information
pilleye committed Oct 15, 2024
1 parent b16f665 commit 77a9bff
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,39 @@

## Maybe unbound

```py
```py path=package/maybe_unbound.py
if flag:
y = 3
x = y
reveal_type(x) # revealed: Unbound | Literal[3]
reveal_type(y) # revealed: Unbound | Literal[3]
```

```py path=package/public.py
from .maybe_unbound import x, y # error: [possibly-unresolved-import]
reveal_type(x) # revealed: Literal[3]
reveal_type(y) # revealed: Literal[3]
```

## Maybe unbound annotated

```py path=package/maybe_unbound_annotated.py
if flag:
y: int = 3
x = y
reveal_type(x) # revealed: Unbound | Literal[3]
reveal_type(y) # revealed: Unbound | int
```

```py path=package/public.py
from .maybe_unbound_annotated import x, y # error: [possibly-unresolved-import]
reveal_type(x) # revealed: Literal[3]
reveal_type(y) # revealed: int
```

## Unbound

```py
```py path=unbound/
x = foo; foo = 1
reveal_type(x) # revealed: Unbound
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ reveal_type(x) # revealed: Literal[2, 3]

## Simple if-elif-else

```py
```py path=package/simple_if_elif_else.py
y = 1
y = 2
if flag:
Expand All @@ -28,11 +28,19 @@ else:
y = 5
s = y
x = y

reveal_type(x) # revealed: Literal[3, 4, 5]
reveal_type(r) # revealed: Unbound | Literal[2]
reveal_type(s) # revealed: Unbound | Literal[5]
```

```py path=package/public.py
from .simple_if_elif_else import r, s # error: [unresolved-import]

reveal_type(r) # revealed: Literal[2]
reveal_type(s) # revealed: Literal[5]
```

## Single symbol across if-elif-else

```py
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ reveal_type(y) # revealed: Literal[2, 3]

## Without wildcard

```py
```py path=package/without_wildcard.py
match 0:
case 1:
y = 2
Expand All @@ -24,6 +24,12 @@ match 0:
reveal_type(y) # revealed: Unbound | Literal[2, 3]
```

```py path=package/public.py
from .without_wildcard import y # error: [unresolved-import]

reveal_type(y) # revealed: Literal[2, 3]
```

## Basic match

```py
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
# Except star

TODO(Alex): Once we support `sys.version_info` branches, we can set `--target-version=py311` in these tests and the inferred type will just be `BaseExceptionGroup`

## Except\* with BaseException

```py
try:
x
except* BaseException as e:
reveal_type(e) # revealed: Unknown | BaseExceptionGroup
reveal_type(e) # revealed: BaseExceptionGroup
```

## Except\* with specific exception
Expand All @@ -18,7 +16,7 @@ try:
x
except* OSError as e:
# TODO(Alex): more precise would be `ExceptionGroup[OSError]`
reveal_type(e) # revealed: Unknown | BaseExceptionGroup
reveal_type(e) # revealed: BaseExceptionGroup
```

## Except\* with multiple exceptions
Expand All @@ -28,5 +26,5 @@ try:
x
except* (TypeError, AttributeError) as e:
#TODO(Alex): more precise would be `ExceptionGroup[TypeError | AttributeError]`.
reveal_type(e) # revealed: Unknown | BaseExceptionGroup
reveal_type(e) # revealed: BaseExceptionGroup
```
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ x

```py path=package/bar.py
from .foo import x # error: [unresolved-import]
reveal_type(x) # revealed: Unknown
reveal_type(x) # revealed: Never
```

## Bare to module
Expand Down Expand Up @@ -129,5 +129,5 @@ reveal_type(y) # revealed: Unknown
# TODO: submodule imports possibly not supported right now?
from . import foo # error: [unresolved-import]

reveal_type(foo) # revealed: Unknown
reveal_type(foo) # revealed: Never
```
32 changes: 28 additions & 4 deletions crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Basic `for` loop

```py
```py path=package/basic_for_loop.py
class IntIterator:
def __next__(self) -> int:
return 42
Expand All @@ -17,6 +17,12 @@ for x in IntIterable():
reveal_type(x) # revealed: Unbound | int
```

```py path=package/public.py
from .basic_for_loop import x # error: [unresolved-import]

reveal_type(x) # revealed: int
```

## With previous definition

```py
Expand Down Expand Up @@ -77,7 +83,7 @@ reveal_type(x) # revealed: int | Literal["foo"]

## With old-style iteration protocol

```py
```py path=package/without_oldstyle_iteration_protocol.py
class OldStyleIterable:
def __getitem__(self, key: int) -> int:
return 42
Expand All @@ -88,18 +94,30 @@ for x in OldStyleIterable():
reveal_type(x) # revealed: Unbound | int
```

```py path=package/public.py
from .without_oldstyle_iteration_protocol import x # error: [unresolved-import]

reveal_type(x) # revealed: int
```

## With heterogeneous tuple

```py
```py path=package/with_heterogeneous_tuple.py
for x in (1, 'a', b'foo'):
pass

reveal_type(x) # revealed: Unbound | Literal[1] | Literal["a"] | Literal[b"foo"]
```

```py path=package/public.py
from .with_heterogeneous_tuple import x # error: [unresolved-import]

reveal_type(x) # revealed: Literal[1] | Literal["a"] | Literal[b"foo"]
```

## With non-callable iterator

```py
```py path=with_noncallable_iterator/with_noncallable_iterator.py
class NotIterable:
if flag:
__iter__ = 1
Expand All @@ -112,6 +130,12 @@ for x in NotIterable(): # error: "Object of type `NotIterable` is not iterable"
reveal_type(x) # revealed: Unbound | Unknown
```

```py path=with_noncallable_iterator/with_noncallable_iterator.py
from .with_noncallable_iterator import x

reveal_type(x) # revealed: Unknown | int
```

## Invalid iterable

```py
Expand Down
30 changes: 17 additions & 13 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb
let _span = tracing::trace_span!("symbol_ty_by_id", ?symbol).entered();

let use_def = use_def_map(db, scope);
let unbound_ty = || use_def.public_may_be_unbound(symbol).then_some(Type::Never);

// If the symbol is declared, the public type is based on declarations; otherwise, it's based
// on inference from bindings.
Expand All @@ -58,9 +59,7 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb
Some(bindings_ty(
db,
use_def.public_bindings(symbol),
use_def
.public_may_be_unbound(symbol)
.then_some(Type::Unknown),
unbound_ty(),
))
} else {
None
Expand All @@ -69,17 +68,11 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb
// problem of the module we are importing from.
declarations_ty(db, declarations, undeclared_ty).unwrap_or_else(|(ty, _)| ty)
} else {
bindings_ty(
db,
use_def.public_bindings(symbol),
use_def
.public_may_be_unbound(symbol)
.then_some(Type::Unbound),
)
bindings_ty(db, use_def.public_bindings(symbol), unbound_ty())
}
}

/// Shorthand for `symbol_ty` that takes a symbol name instead of an ID.
/// Shorthand for `symbol_ty_by_id` that takes a symbol name instead of an ID.
fn symbol_ty<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Type<'db> {
let table = symbol_table(db, scope);
table
Expand Down Expand Up @@ -288,6 +281,14 @@ impl<'db> Type<'db> {
matches!(self, Type::Unbound)
}

fn contains_unbound(&self, db: &'db dyn Db) -> bool {
return self.is_equivalent_to(db, Type::Unbound)
|| match self {
Type::Union(union) => union.elements(db).iter().any(|ty| ty.contains_unbound(db)),
_ => false,
};
}

pub const fn is_never(&self) -> bool {
matches!(self, Type::Never)
}
Expand Down Expand Up @@ -376,12 +377,15 @@ impl<'db> Type<'db> {

#[must_use]
pub fn replace_unbound_with(&self, db: &'db dyn Db, replacement: Type<'db>) -> Type<'db> {
if self.is_equivalent_to(db, Type::Unbound) {
return replacement;
}

match self {
Type::Unbound => replacement,
Type::Union(union) => {
union.map(db, |element| element.replace_unbound_with(db, replacement))
}
ty => *ty,
_ => *self,
}
}

Expand Down
Loading

0 comments on commit 77a9bff

Please sign in to comment.