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 1, 2024
1 parent 5118166 commit 2acaeef
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 28 deletions.
8 changes: 2 additions & 6 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,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),
use_def.public_may_be_unbound(symbol).then_some(Type::Never),
))
} else {
None
Expand All @@ -72,9 +70,7 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb
bindings_ty(
db,
use_def.public_bindings(symbol),
use_def
.public_may_be_unbound(symbol)
.then_some(Type::Unbound),
use_def.public_may_be_unbound(symbol).then_some(Type::Never),
)
}
}
Expand Down
83 changes: 61 additions & 22 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3060,7 +3060,7 @@ mod tests {
}

#[test]
fn imported_unbound_symbol_is_unknown() -> anyhow::Result<()> {
fn imported_unbound_symbol_is_never() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_files([
Expand All @@ -3071,8 +3071,8 @@ mod tests {

// the type as seen from external modules (`Unknown`)
// is different from the type inside the module itself (`Unbound`):
assert_public_ty(&db, "src/package/foo.py", "x", "Unbound");
assert_public_ty(&db, "src/package/bar.py", "x", "Unknown");
assert_public_ty(&db, "src/package/foo.py", "x", "Never");
assert_public_ty(&db, "src/package/bar.py", "x", "Never");

Ok(())
}
Expand Down Expand Up @@ -3620,7 +3620,7 @@ mod tests {
)?;

// TODO: sys.version_info, and need to understand @final and @type_check_only
assert_public_ty(&db, "src/a.py", "x", "Unknown | EllipsisType");
assert_public_ty(&db, "src/a.py", "x", "EllipsisType | Unknown");

Ok(())
}
Expand Down Expand Up @@ -3952,6 +3952,40 @@ mod tests {
Ok(())
}

#[test]
fn resolve_unbound_public() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_dedented(
"src/a.py",
"
if flag:
x: int = 1
",
)?;

assert_public_ty(&db, "src/a.py", "x", "int");

Ok(())
}

#[test]
fn resolve_unannotated_unbound_public() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_dedented(
"src/a.py",
"
if flag:
x = 1
",
)?;

assert_public_ty(&db, "src/a.py", "x", "Literal[1]");

Ok(())
}

#[test]
fn literal_int_arithmetic() -> anyhow::Result<()> {
let mut db = setup_db();
Expand Down Expand Up @@ -4129,8 +4163,8 @@ mod tests {
)?;

assert_public_ty(&db, "src/a.py", "x", "Literal[3, 4, 5]");
assert_public_ty(&db, "src/a.py", "r", "Unbound | Literal[2]");
assert_public_ty(&db, "src/a.py", "s", "Unbound | Literal[5]");
assert_public_ty(&db, "src/a.py", "r", "Literal[2]");
assert_public_ty(&db, "src/a.py", "s", "Literal[5]");
Ok(())
}

Expand Down Expand Up @@ -4271,7 +4305,7 @@ mod tests {
)
.unwrap();

assert_public_ty(&db, "src/a.py", "y", "Unbound | Literal[2, 3]");
assert_public_ty(&db, "src/a.py", "y", "Literal[2, 3]");
}

#[test]
Expand Down Expand Up @@ -4387,7 +4421,7 @@ mod tests {
let y_ty = symbol_ty(&db, function_scope, "y");
let x_ty = symbol_ty(&db, function_scope, "x");

assert_eq!(x_ty.display(&db).to_string(), "Unbound");
assert_eq!(x_ty.display(&db).to_string(), "Never");
assert_eq!(y_ty.display(&db).to_string(), "Literal[1]");

Ok(())
Expand Down Expand Up @@ -4452,7 +4486,7 @@ mod tests {
let y_ty = symbol_ty(&db, class_scope, "y");
let x_ty = symbol_ty(&db, class_scope, "x");

assert_eq!(x_ty.display(&db).to_string(), "Unbound | Literal[2]");
assert_eq!(x_ty.display(&db).to_string(), "Literal[2]");
assert_eq!(y_ty.display(&db).to_string(), "Literal[1]");

Ok(())
Expand Down Expand Up @@ -5115,7 +5149,7 @@ mod tests {
",
)?;

assert_public_ty(&db, "src/a.py", "x", "Unbound | int");
assert_public_ty(&db, "src/a.py", "x", "int");

Ok(())
}
Expand Down Expand Up @@ -5221,7 +5255,7 @@ mod tests {
",
)?;

assert_public_ty(&db, "src/a.py", "x", "Unbound | int");
assert_public_ty(&db, "src/a.py", "x", "int");

Ok(())
}
Expand All @@ -5235,6 +5269,8 @@ mod tests {
db.write_dedented(
"src/a.py",
"
from typing import reveal_type
async def foo():
class Iterator:
def __next__(self) -> int:
Expand All @@ -5246,10 +5282,12 @@ mod tests {
async for x in Iterator():
pass
reveal_type(x)
",
)?;

assert_scope_ty(&db, "src/a.py", &["foo"], "x", "Unbound | Unknown");
assert_file_diagnostics(&db, "src/a.py", &["Revealed type is 'Unbound | Unknown'."]);

Ok(())
}
Expand All @@ -5261,6 +5299,8 @@ mod tests {
db.write_dedented(
"src/a.py",
"
from typing import reveal_type
async def foo():
class IntAsyncIterator:
async def __anext__(self) -> int:
Expand All @@ -5272,11 +5312,13 @@ mod tests {
async for x in IntAsyncIterable():
pass
reveal_type(x)
",
)?;

// TODO(Alex) async iterables/iterators!
assert_scope_ty(&db, "src/a.py", &["foo"], "x", "Unbound | Unknown");
assert_file_diagnostics(&db, "src/a.py", &["Revealed type is 'Unbound | Unknown'."]);

Ok(())
}
Expand All @@ -5297,7 +5339,7 @@ mod tests {
&db,
"src/a.py",
"x",
r#"Unbound | Literal[1] | Literal["a"] | Literal[b"foo"]"#,
r#"Literal[1] | Literal["a"] | Literal[b"foo"]"#,
);

Ok(())
Expand Down Expand Up @@ -5326,7 +5368,7 @@ mod tests {
"src/a.py",
&["Object of type 'NotIterable' is not iterable."],
);
assert_public_ty(&db, "src/a.py", "x", "Unbound | Unknown");
assert_public_ty(&db, "src/a.py", "x", "Unknown");

Ok(())
}
Expand Down Expand Up @@ -5449,10 +5491,7 @@ mod tests {

assert_file_diagnostics(&db, "src/a.py", &[]);

// TODO: once we support `sys.version_info` branches,
// we can set `--target-version=py311` in this test
// and the inferred type will just be `BaseExceptionGroup` --Alex
assert_public_ty(&db, "src/a.py", "e", "Unknown | BaseExceptionGroup");
assert_public_ty(&db, "src/a.py", "e", "BaseExceptionGroup");

Ok(())
}
Expand All @@ -5478,7 +5517,7 @@ mod tests {
// and the inferred type will just be `BaseExceptionGroup` --Alex
//
// TODO more precise would be `ExceptionGroup[OSError]` --Alex
assert_public_ty(&db, "src/a.py", "e", "Unknown | BaseExceptionGroup");
assert_public_ty(&db, "src/a.py", "e", "BaseExceptionGroup");

Ok(())
}
Expand All @@ -5504,7 +5543,7 @@ mod tests {
// and the inferred type will just be `BaseExceptionGroup` --Alex
//
// TODO more precise would be `ExceptionGroup[TypeError | AttributeError]` --Alex
assert_public_ty(&db, "src/a.py", "e", "Unknown | BaseExceptionGroup");
assert_public_ty(&db, "src/a.py", "e", "BaseExceptionGroup");

Ok(())
}
Expand Down Expand Up @@ -5767,7 +5806,7 @@ mod tests {
",
)?;

assert_scope_ty(&db, "src/a.py", &["foo", "<listcomp>"], "z", "Unbound");
assert_scope_ty(&db, "src/a.py", &["foo", "<listcomp>"], "z", "Never");

// (There is a diagnostic for invalid syntax that's emitted, but it's not listed by `assert_file_diagnostics`)
assert_file_diagnostics(&db, "src/a.py", &[]);
Expand Down

0 comments on commit 2acaeef

Please sign in to comment.