From d05b1d7ab3556578b3ad4719f053f6410017599d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 14 Aug 2024 17:14:10 -0700 Subject: [PATCH 1/2] [red-knot] fix lookups of possibly-shadowed builtins --- crates/red_knot_python_semantic/src/types.rs | 23 ++++++++++- .../src/types/infer.rs | 39 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 37430d95c3aa6..24d89a4ebecf0 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -145,8 +145,27 @@ impl<'db> Type<'db> { matches!(self, Type::Unbound) } - pub const fn is_unknown(&self) -> bool { - matches!(self, Type::Unknown) + pub fn may_be_unbound(&self, db: &'db dyn Db) -> bool { + match self { + Type::Unbound => true, + Type::Union(union) => union.contains(db, Type::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 + .elements(db) + .into_iter() + .fold(UnionBuilder::new(db), |builder, ty| { + builder.add(ty.replace_unbound_with(db, replacement)) + }) + .build(), + ty => *ty, + } } #[must_use] diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index ea39ee0725736..5af2cfbc543a2 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1360,10 +1360,13 @@ impl<'db> TypeInferenceBuilder<'db> { 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); + unbound_ty = unbound_ty.replace_unbound_with( + self.db, + builtins_symbol_ty_by_name(self.db, id), + ); } Some(unbound_ty) } else { @@ -2163,6 +2166,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<()> { From 2a4b4c9e8ee3d4bc9d2ee96bf21f72dedf10532f Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 15 Aug 2024 07:43:44 -0700 Subject: [PATCH 2/2] code review comments --- crates/red_knot_python_semantic/src/types.rs | 6 ++++ .../src/types/builder.rs | 31 +++++++++++++++++-- .../src/types/infer.rs | 9 +++--- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 24d89a4ebecf0..e6f739df3a009 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -145,10 +145,16 @@ impl<'db> Type<'db> { matches!(self, Type::Unbound) } + 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, } } diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index 9f8af0f295160..8581ff546434d 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -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); @@ -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); } } @@ -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)); + } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 5af2cfbc543a2..5b4ba7ffb3406 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1354,7 +1354,7 @@ 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) @@ -1363,12 +1363,13 @@ impl<'db> TypeInferenceBuilder<'db> { if unbound_ty.may_be_unbound(self.db) && Some(self.scope) != builtins_scope(self.db) { - unbound_ty = unbound_ty.replace_unbound_with( + 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) }