From 9e43d34b48d31de4a3ba577501b4349c8ab02957 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Fri, 30 Aug 2024 21:46:48 -0500 Subject: [PATCH 1/7] union builder prefers bool over union of literals true and false --- crates/red_knot_python_semantic/src/types/builder.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index e08a9d7e2d103..c6f9cb5c4800e 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -28,6 +28,8 @@ use crate::types::{IntersectionType, Type, UnionType}; use crate::{Db, FxOrderSet}; +use super::builtins_symbol_ty_by_name; + pub(crate) struct UnionBuilder<'db> { elements: FxOrderSet>, db: &'db dyn Db, @@ -60,6 +62,11 @@ impl<'db> UnionBuilder<'db> { match self.elements.len() { 0 => Type::Never, 1 => self.elements[0], + 2 if self.elements.contains(&Type::BooleanLiteral(true)) + && self.elements.contains(&Type::BooleanLiteral(false)) => + { + builtins_symbol_ty_by_name(self.db, "bool") + } _ => Type::Union(UnionType::new(self.db, self.elements)), } } From 4de3ee40ce55138d55aa0a178685eaaab01e1909 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Fri, 30 Aug 2024 22:21:21 -0500 Subject: [PATCH 2/7] unit test --- .../src/types/builder.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index c6f9cb5c4800e..c493769657c89 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -254,6 +254,11 @@ impl<'db> InnerIntersectionBuilder<'db> { mod tests { use super::{IntersectionBuilder, IntersectionType, Type, UnionBuilder, UnionType}; use crate::db::tests::TestDb; + use crate::program::{Program, SearchPathSettings}; + use crate::python_version::PythonVersion; + use crate::types::builtins_symbol_ty_by_name; + use crate::ProgramSettings; + use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; fn setup_db() -> TestDb { TestDb::new() @@ -265,6 +270,26 @@ mod tests { } } + fn setup_db_with_python() -> TestDb { + let db = TestDb::new(); + + let src_root = SystemPathBuf::from("/src"); + db.memory_file_system() + .create_directory_all(&src_root) + .unwrap(); + + Program::from_settings( + &db, + &ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings::new(src_root), + }, + ) + .expect("Valid search path settings"); + + db + } + #[test] fn build_union() { let db = setup_db(); @@ -303,6 +328,20 @@ mod tests { assert_eq!(ty, t0); } + #[test] + fn build_union_bool() { + let db = setup_db_with_python(); + let ty_expect = builtins_symbol_ty_by_name(&db, "bool"); + let t0 = Type::BooleanLiteral(true); + let t1 = Type::BooleanLiteral(true); + let t2 = Type::BooleanLiteral(false); + let ty_true = UnionBuilder::new(&db).add(t0).add(t1).build(); + let ty = UnionBuilder::new(&db).add(t0).add(t1).add(t2).build(); + + assert_eq!(ty_true, Type::BooleanLiteral(true)); + assert_eq!(ty, ty_expect); + } + #[test] fn build_union_flatten() { let db = setup_db(); From 9861b8478796dc7942967804c8d11a2c504aa278 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Fri, 30 Aug 2024 22:34:29 -0500 Subject: [PATCH 3/7] handle case when builtins is missing --- crates/red_knot_python_semantic/src/types/builder.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index c493769657c89..ad127d2cc9d28 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -65,7 +65,12 @@ impl<'db> UnionBuilder<'db> { 2 if self.elements.contains(&Type::BooleanLiteral(true)) && self.elements.contains(&Type::BooleanLiteral(false)) => { - builtins_symbol_ty_by_name(self.db, "bool") + let ty = builtins_symbol_ty_by_name(self.db, "bool"); + if ty.is_unbound() { + Type::Union(UnionType::new(self.db, self.elements)) + } else { + ty + } } _ => Type::Union(UnionType::new(self.db, self.elements)), } From 947d8e7c067b3f7a51d9f5033a439c7a9f480fdb Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sat, 31 Aug 2024 21:36:14 -0500 Subject: [PATCH 4/7] replace setup db --- crates/red_knot_python_semantic/src/types/builder.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index ad127d2cc9d28..46592b3facb25 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -265,17 +265,13 @@ mod tests { use crate::ProgramSettings; use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; - fn setup_db() -> TestDb { - TestDb::new() - } - impl<'db> UnionType<'db> { fn elements_vec(self, db: &'db TestDb) -> Vec> { self.elements(db).into_iter().collect() } } - fn setup_db_with_python() -> TestDb { + fn setup_db() -> TestDb { let db = TestDb::new(); let src_root = SystemPathBuf::from("/src"); From d4f721ebb726d8812dadeeabc04ad896fca0db8d Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sat, 31 Aug 2024 21:37:07 -0500 Subject: [PATCH 5/7] unit test in presence of additional type --- .../src/types/builder.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index 46592b3facb25..335662b1c09ff 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -331,16 +331,29 @@ mod tests { #[test] fn build_union_bool() { - let db = setup_db_with_python(); - let ty_expect = builtins_symbol_ty_by_name(&db, "bool"); + let db = setup_db(); + let bool_ty = builtins_symbol_ty_by_name(&db, "bool"); + let t0 = Type::BooleanLiteral(true); let t1 = Type::BooleanLiteral(true); let t2 = Type::BooleanLiteral(false); - let ty_true = UnionBuilder::new(&db).add(t0).add(t1).build(); - let ty = UnionBuilder::new(&db).add(t0).add(t1).add(t2).build(); + let t3 = Type::IntLiteral(17); + + let Type::Union(union) = UnionBuilder::new(&db).add(t0).add(t1).add(t3).build() else { + panic!("expected a union"); + }; + assert_eq!(union.elements_vec(&db), &[t0, t3]); + let Type::Union(union) = UnionBuilder::new(&db) + .add(t0) + .add(t1) + .add(t2) + .add(t3) + .build() + else { + panic!("expected a union"); + }; - assert_eq!(ty_true, Type::BooleanLiteral(true)); - assert_eq!(ty, ty_expect); + assert_eq!(union.elements_vec(&db), &[t3, bool_ty]); } #[test] From 2c3e6ea0b5605364bddc810614617c2a4b92b9a6 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sat, 31 Aug 2024 21:37:46 -0500 Subject: [PATCH 6/7] add simplify method to union builder --- .../src/types/builder.rs | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index 335662b1c09ff..28b18caa67b6d 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -58,20 +58,29 @@ impl<'db> UnionBuilder<'db> { self } - pub(crate) fn build(self) -> Type<'db> { + /// Performs the following normalizations: + /// - Replaces `Literal[True,False]` with `bool`. + /// - TODO For enums `E` with members `X1`,...,`Xn`, replaces + /// `Literal[E.X1,...,E.Xn]` with `E`. + fn simplify(&mut self) { + if self + .elements + .is_superset(&[Type::BooleanLiteral(true), Type::BooleanLiteral(false)].into()) + { + let bool_ty = builtins_symbol_ty_by_name(self.db, "bool"); + if !bool_ty.is_unbound() { + self.elements.remove(&Type::BooleanLiteral(true)); + self.elements.remove(&Type::BooleanLiteral(false)); + self.elements.insert(bool_ty); + } + } + } + + pub(crate) fn build(mut self) -> Type<'db> { + self.simplify(); match self.elements.len() { 0 => Type::Never, 1 => self.elements[0], - 2 if self.elements.contains(&Type::BooleanLiteral(true)) - && self.elements.contains(&Type::BooleanLiteral(false)) => - { - let ty = builtins_symbol_ty_by_name(self.db, "bool"); - if ty.is_unbound() { - Type::Union(UnionType::new(self.db, self.elements)) - } else { - ty - } - } _ => Type::Union(UnionType::new(self.db, self.elements)), } } From 479cdbe803a92a040ffbf77ffa21a733b6580d7e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sat, 31 Aug 2024 22:43:10 -0700 Subject: [PATCH 7/7] Update crates/red_knot_python_semantic/src/types/builder.rs --- crates/red_knot_python_semantic/src/types/builder.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index 28b18caa67b6d..da8e2668d8620 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -68,11 +68,9 @@ impl<'db> UnionBuilder<'db> { .is_superset(&[Type::BooleanLiteral(true), Type::BooleanLiteral(false)].into()) { let bool_ty = builtins_symbol_ty_by_name(self.db, "bool"); - if !bool_ty.is_unbound() { - self.elements.remove(&Type::BooleanLiteral(true)); - self.elements.remove(&Type::BooleanLiteral(false)); - self.elements.insert(bool_ty); - } + self.elements.remove(&Type::BooleanLiteral(true)); + self.elements.remove(&Type::BooleanLiteral(false)); + self.elements.insert(bool_ty); } }