Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] fix lookups of possibly-shadowed builtins #12898

Merged
merged 2 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,33 @@ impl<'db> Type<'db> {
matches!(self, Type::Unbound)
}

pub const fn is_unknown(&self) -> bool {
matches!(self, Type::Unknown)
pub const fn is_never(&self) -> bool {
matches!(self, Type::Never)
}

pub fn may_be_unbound(&self, db: &'db dyn Db) -> bool {
match self {
Type::Unbound => true,
Type::Union(union) => union.contains(db, Type::Unbound),
// Unbound can't appear in an intersection, because an intersection with Unbound
// simplifies to just Unbound.
_ => false,
}
}

#[must_use]
pub fn replace_unbound_with(&self, db: &'db dyn Db, replacement: Type<'db>) -> Type<'db> {
match self {
Type::Unbound => replacement,
Type::Union(union) => union
carljm marked this conversation as resolved.
Show resolved Hide resolved
.elements(db)
.into_iter()
.fold(UnionBuilder::new(db), |builder, ty| {
builder.add(ty.replace_unbound_with(db, replacement))
})
.build(),
ty => *ty,
}
}

#[must_use]
Expand Down
31 changes: 29 additions & 2 deletions crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ impl<'db> InnerIntersectionBuilder<'db> {
self.negative.retain(|elem| !pos.contains(elem));
}
Type::Never => {}
Type::Unbound => {}
_ => {
if !self.positive.remove(&ty) {
self.negative.insert(ty);
Expand All @@ -214,9 +215,13 @@ impl<'db> InnerIntersectionBuilder<'db> {

// Never is a subtype of all types
if self.positive.contains(&Type::Never) {
self.positive.clear();
self.positive.retain(Type::is_never);
self.negative.clear();
}

if self.positive.contains(&Type::Unbound) {
self.positive.retain(Type::is_unbound);
self.negative.clear();
self.positive.insert(Type::Never);
}
}

Expand Down Expand Up @@ -426,4 +431,26 @@ mod tests {

assert_eq!(ty, Type::Never);
}

#[test]
fn build_intersection_simplify_positive_unbound() {
let db = setup_db();
let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Unbound)
.add_positive(Type::IntLiteral(1))
.build();

assert_eq!(ty, Type::Unbound);
}

#[test]
fn build_intersection_simplify_negative_unbound() {
let db = setup_db();
let ty = IntersectionBuilder::new(&db)
.add_negative(Type::Unbound)
.add_positive(Type::IntLiteral(1))
.build();

assert_eq!(ty, Type::IntLiteral(1));
}
}
44 changes: 40 additions & 4 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,18 +1354,22 @@ impl<'db> TypeInferenceBuilder<'db> {
let symbol = symbols.symbol_by_name(id).unwrap();
if !symbol.is_defined() || !self.scope.is_function_like(self.db) {
// implicit global
let mut unbound_ty = if file_scope_id == FileScopeId::global() {
let unbound_ty = if file_scope_id == FileScopeId::global() {
Type::Unbound
} else {
global_symbol_ty_by_name(self.db, self.file, id)
};
// fallback to builtins
if matches!(unbound_ty, Type::Unbound)
if unbound_ty.may_be_unbound(self.db)
&& Some(self.scope) != builtins_scope(self.db)
{
unbound_ty = builtins_symbol_ty_by_name(self.db, id);
Some(unbound_ty.replace_unbound_with(
self.db,
builtins_symbol_ty_by_name(self.db, id),
))
} else {
Some(unbound_ty)
}
Some(unbound_ty)
} else {
Some(Type::Unbound)
}
Expand Down Expand Up @@ -2163,6 +2167,38 @@ mod tests {
Ok(())
}

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

db.write_dedented(
"/src/a.py",
"
if flag:
copyright = 1
def f():
y = copyright
",
)?;

let file = system_path_to_file(&db, "src/a.py").expect("Expected file to exist.");
let index = semantic_index(&db, file);
let function_scope = index
.child_scopes(FileScopeId::global())
.next()
.unwrap()
.0
.to_scope_id(&db, file);
let y_ty = symbol_ty_by_name(&db, function_scope, "y");

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

Ok(())
}

/// Class name lookups do fall back to globals, but the public type never does.
#[test]
fn unbound_class_local() -> anyhow::Result<()> {
Expand Down
Loading