From cdafb552d601f4937542735e5a754858bde00b72 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Fri, 29 Nov 2024 00:45:50 -0500 Subject: [PATCH 1/7] red-knot: support narrowing for bool(E) --- .../mdtest/narrow/conditionals/boolean.md | 28 +++++++++++++++++++ .../src/types/narrow.rs | 21 ++++++++++++-- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md index 854e09ff0a50b..e74533f486a8d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md @@ -280,3 +280,31 @@ if x is None: if y is not x: reveal_type(y) # revealed: str | None ``` + +## `bool(..)` builtin containing expression narrows for expression + +```py +def flag() -> bool: ... + +x = 1 if flag() else None + +# valid invocation, positive +reveal_type(x) # revealed: Literal[1] | None +if bool(x is not None): + reveal_type(x) # revealed: Literal[1] + +# valid invocation, negative +reveal_type(x) # revealed: Literal[1] | None +if not bool(x is not None): + reveal_type(x) # revealed: None + +# no args/narrowing +reveal_type(x) # revealed: Literal[1] | None +if bool(): + reveal_type(x) # revealed: Literal[1] | None + +# invalid invocation, too many args +reveal_type(x) # revealed: Literal[1] | None +if bool(x, 5): + reveal_type(x) # revealed: Literal[1] | None +``` \ No newline at end of file diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index 7ce84cb7fb553..f1b9210aeaf2c 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -385,10 +385,11 @@ impl<'db> NarrowingConstraintsBuilder<'db> { let scope = self.scope(); let inference = infer_expression_types(self.db, expression); + let expr_ty = inference.expression_ty(expr_call.func.scoped_expression_id(self.db, scope)); + // TODO: add support for PEP 604 union types on the right hand side of `isinstance` // and `issubclass`, for example `isinstance(x, str | (int | float))`. - match inference - .expression_ty(expr_call.func.scoped_expression_id(self.db, scope)) + match expr_ty .into_function_literal() .and_then(|f| f.known(self.db)) .and_then(KnownFunction::constraint_function) @@ -426,7 +427,21 @@ impl<'db> NarrowingConstraintsBuilder<'db> { None } } - _ => None, + _ => { + if expr_call.arguments.args.len() == 1 + && expr_ty + .into_class_literal() + .is_some_and(|lit| lit.class.is_known(self.db, KnownClass::Bool)) + { + self.evaluate_expression_node_constraint( + &expr_call.arguments.args[0], + expression, + is_positive, + ) + } else { + None + } + } } } From 8109f4cb800162cf8ab9a7cb81eb06698b68b454 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Fri, 29 Nov 2024 01:03:40 -0500 Subject: [PATCH 2/7] lint: newline at end of file --- .../resources/mdtest/narrow/conditionals/boolean.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md index e74533f486a8d..3d1a81beea68c 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md @@ -307,4 +307,4 @@ if bool(): reveal_type(x) # revealed: Literal[1] | None if bool(x, 5): reveal_type(x) # revealed: Literal[1] | None -``` \ No newline at end of file +``` From a0b496de4055b7c2a48d197c00d89dd3ab063e59 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Fri, 29 Nov 2024 08:42:32 -0500 Subject: [PATCH 3/7] more robust argument validation --- .../resources/mdtest/narrow/conditionals/boolean.md | 9 +++++++-- crates/red_knot_python_semantic/src/types/narrow.rs | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md index 3d1a81beea68c..337430fd4e4e2 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md @@ -303,8 +303,13 @@ reveal_type(x) # revealed: Literal[1] | None if bool(): reveal_type(x) # revealed: Literal[1] | None -# invalid invocation, too many args +# invalid invocation, too many positional args reveal_type(x) # revealed: Literal[1] | None -if bool(x, 5): +if bool(x is not None, 5): + reveal_type(x) # revealed: Literal[1] | None + +# invalid invocation, too many kwargs +reveal_type(x) # revealed: Literal[1] | None +if bool(x is not None, y=5): reveal_type(x) # revealed: Literal[1] | None ``` diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index f1b9210aeaf2c..265b435b03922 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -428,7 +428,10 @@ impl<'db> NarrowingConstraintsBuilder<'db> { } } _ => { - if expr_call.arguments.args.len() == 1 + let is_valid_bool_invocation = + expr_call.arguments.args.len() == 1 && expr_call.arguments.keywords.is_empty(); + + if is_valid_bool_invocation && expr_ty .into_class_literal() .is_some_and(|lit| lit.class.is_known(self.db, KnownClass::Bool)) From 98858fa0902b86fa2075f4c2abf0787572f35206 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Mon, 2 Dec 2024 11:19:15 -0500 Subject: [PATCH 4/7] pr review --- .../resources/mdtest/narrow/conditionals/boolean.md | 2 +- crates/red_knot_python_semantic/src/types/narrow.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md index 337430fd4e4e2..10dc4c6cca225 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md @@ -300,7 +300,7 @@ if not bool(x is not None): # no args/narrowing reveal_type(x) # revealed: Literal[1] | None -if bool(): +if not bool(): reveal_type(x) # revealed: Literal[1] | None # invalid invocation, too many positional args diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index 265b435b03922..c0d7a76edb623 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -385,11 +385,12 @@ impl<'db> NarrowingConstraintsBuilder<'db> { let scope = self.scope(); let inference = infer_expression_types(self.db, expression); - let expr_ty = inference.expression_ty(expr_call.func.scoped_expression_id(self.db, scope)); + let callable_ty = + inference.expression_ty(expr_call.func.scoped_expression_id(self.db, scope)); // TODO: add support for PEP 604 union types on the right hand side of `isinstance` // and `issubclass`, for example `isinstance(x, str | (int | float))`. - match expr_ty + match callable_ty .into_function_literal() .and_then(|f| f.known(self.db)) .and_then(KnownFunction::constraint_function) @@ -432,7 +433,7 @@ impl<'db> NarrowingConstraintsBuilder<'db> { expr_call.arguments.args.len() == 1 && expr_call.arguments.keywords.is_empty(); if is_valid_bool_invocation - && expr_ty + && callable_ty .into_class_literal() .is_some_and(|lit| lit.class.is_known(self.db, KnownClass::Bool)) { From 5ca72ed3f70fc0bc6870159b93aae016f1369852 Mon Sep 17 00:00:00 2001 From: David Peter Date: Mon, 2 Dec 2024 20:53:17 +0100 Subject: [PATCH 5/7] Move tests to new file --- .../resources/mdtest/narrow/bool-call.md | 32 ++++++++++++++++++ .../mdtest/narrow/conditionals/boolean.md | 33 ------------------- 2 files changed, 32 insertions(+), 33 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md new file mode 100644 index 0000000000000..d0222cd138f37 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md @@ -0,0 +1,32 @@ +## Narrowing for `bool(..)` checks + +```py +def flag() -> bool: ... + +x = 1 if flag() else None + +# valid invocation, positive +reveal_type(x) # revealed: Literal[1] | None +if bool(x is not None): + reveal_type(x) # revealed: Literal[1] + +# valid invocation, negative +reveal_type(x) # revealed: Literal[1] | None +if not bool(x is not None): + reveal_type(x) # revealed: None + +# no args/narrowing +reveal_type(x) # revealed: Literal[1] | None +if not bool(): + reveal_type(x) # revealed: Literal[1] | None + +# invalid invocation, too many positional args +reveal_type(x) # revealed: Literal[1] | None +if bool(x is not None, 5): + reveal_type(x) # revealed: Literal[1] | None + +# invalid invocation, too many kwargs +reveal_type(x) # revealed: Literal[1] | None +if bool(x is not None, y=5): + reveal_type(x) # revealed: Literal[1] | None +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md index 10dc4c6cca225..854e09ff0a50b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals/boolean.md @@ -280,36 +280,3 @@ if x is None: if y is not x: reveal_type(y) # revealed: str | None ``` - -## `bool(..)` builtin containing expression narrows for expression - -```py -def flag() -> bool: ... - -x = 1 if flag() else None - -# valid invocation, positive -reveal_type(x) # revealed: Literal[1] | None -if bool(x is not None): - reveal_type(x) # revealed: Literal[1] - -# valid invocation, negative -reveal_type(x) # revealed: Literal[1] | None -if not bool(x is not None): - reveal_type(x) # revealed: None - -# no args/narrowing -reveal_type(x) # revealed: Literal[1] | None -if not bool(): - reveal_type(x) # revealed: Literal[1] | None - -# invalid invocation, too many positional args -reveal_type(x) # revealed: Literal[1] | None -if bool(x is not None, 5): - reveal_type(x) # revealed: Literal[1] | None - -# invalid invocation, too many kwargs -reveal_type(x) # revealed: Literal[1] | None -if bool(x is not None, y=5): - reveal_type(x) # revealed: Literal[1] | None -``` From fcfc53503bd7c7e196a17aed8ed6d5ba529b058e Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Mon, 2 Dec 2024 21:16:31 -0500 Subject: [PATCH 6/7] pr review --- .../resources/mdtest/narrow/bool-call.md | 4 +- .../src/types/narrow.rs | 67 +++++++++---------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md index d0222cd138f37..d7ae47b4fdd07 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/bool-call.md @@ -22,11 +22,11 @@ if not bool(): # invalid invocation, too many positional args reveal_type(x) # revealed: Literal[1] | None -if bool(x is not None, 5): +if bool(x is not None, 5): # TODO diagnostic reveal_type(x) # revealed: Literal[1] | None # invalid invocation, too many kwargs reveal_type(x) # revealed: Literal[1] | None -if bool(x is not None, y=5): +if bool(x is not None, y=5): # TODO diagnostic reveal_type(x) # revealed: Literal[1] | None ``` diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index c0d7a76edb623..e70a6b2a5c753 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -390,45 +390,43 @@ impl<'db> NarrowingConstraintsBuilder<'db> { // TODO: add support for PEP 604 union types on the right hand side of `isinstance` // and `issubclass`, for example `isinstance(x, str | (int | float))`. - match callable_ty - .into_function_literal() - .and_then(|f| f.known(self.db)) - .and_then(KnownFunction::constraint_function) - { - Some(function) if expr_call.arguments.keywords.is_empty() => { - if let [ast::Expr::Name(ast::ExprName { id, .. }), class_info] = + match callable_ty { + Type::FunctionLiteral(function_type) if expr_call.arguments.keywords.is_empty() => { + let function = function_type + .known(self.db) + .and_then(KnownFunction::constraint_function)?; + + let [ast::Expr::Name(ast::ExprName { id, .. }), class_info] = &*expr_call.arguments.args - { - let symbol = self.symbols().symbol_id_by_name(id).unwrap(); + else { + return None; + }; - let class_info_ty = - inference.expression_ty(class_info.scoped_expression_id(self.db, scope)); + let symbol = self.symbols().symbol_id_by_name(id).unwrap(); - let to_constraint = match function { - KnownConstraintFunction::IsInstance => { - |class_literal: ClassLiteralType<'db>| { - Type::instance(class_literal.class) - } - } - KnownConstraintFunction::IsSubclass => { - |class_literal: ClassLiteralType<'db>| { - Type::subclass_of(class_literal.class) - } - } - }; + let class_info_ty = + inference.expression_ty(class_info.scoped_expression_id(self.db, scope)); - generate_classinfo_constraint(self.db, &class_info_ty, to_constraint).map( - |constraint| { - let mut constraints = NarrowingConstraints::default(); - constraints.insert(symbol, constraint.negate_if(self.db, !is_positive)); - constraints - }, - ) - } else { - None - } + let to_constraint = match function { + KnownConstraintFunction::IsInstance => { + |class_literal: ClassLiteralType<'db>| Type::instance(class_literal.class) + } + KnownConstraintFunction::IsSubclass => { + |class_literal: ClassLiteralType<'db>| { + Type::subclass_of(class_literal.class) + } + } + }; + + generate_classinfo_constraint(self.db, &class_info_ty, to_constraint).map( + |constraint| { + let mut constraints = NarrowingConstraints::default(); + constraints.insert(symbol, constraint.negate_if(self.db, !is_positive)); + constraints + }, + ) } - _ => { + Type::ClassLiteral(..) => { let is_valid_bool_invocation = expr_call.arguments.args.len() == 1 && expr_call.arguments.keywords.is_empty(); @@ -446,6 +444,7 @@ impl<'db> NarrowingConstraintsBuilder<'db> { None } } + _ => None, } } From d8d45850a01019ff1cbbd7668182bccde2a65ae7 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Mon, 2 Dec 2024 21:59:10 -0500 Subject: [PATCH 7/7] pr review: avoid superfluous into_class_literal --- .../src/types/narrow.rs | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index e70a6b2a5c753..6005ab781acc2 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -426,23 +426,17 @@ impl<'db> NarrowingConstraintsBuilder<'db> { }, ) } - Type::ClassLiteral(..) => { - let is_valid_bool_invocation = - expr_call.arguments.args.len() == 1 && expr_call.arguments.keywords.is_empty(); - - if is_valid_bool_invocation - && callable_ty - .into_class_literal() - .is_some_and(|lit| lit.class.is_known(self.db, KnownClass::Bool)) - { - self.evaluate_expression_node_constraint( - &expr_call.arguments.args[0], - expression, - is_positive, - ) - } else { - None - } + // for the expression `bool(E)`, we further narrow the type based on `E` + Type::ClassLiteral(class_type) + if expr_call.arguments.args.len() == 1 + && expr_call.arguments.keywords.is_empty() + && class_type.class.is_known(self.db, KnownClass::Bool) => + { + self.evaluate_expression_node_constraint( + &expr_call.arguments.args[0], + expression, + is_positive, + ) } _ => None, }