From 3e702e12f73194a113faabbec7c28deb03c03c4d Mon Sep 17 00:00:00 2001 From: Connor Skees <39542938+connorskees@users.noreply.github.com> Date: Mon, 2 Dec 2024 22:04:59 -0500 Subject: [PATCH] red-knot: support narrowing for bool(E) (#14668) Resolves https://github.com/astral-sh/ruff/issues/14547 by delegating narrowing to `E` for `bool(E)` where `E` is some expression. This change does not include other builtin class constructors which should also work in this position, like `int(..)` or `float(..)`, as the original issue does not mention these. It should be easy enough to add checks for these as well if we want to. I don't see a lot of markdown tests for malformed input, maybe there's a better place for the no args and too many args cases to go? I did see after the fact that it looks like this task was intended for a new hire.. my apologies. I got here from https://github.com/astral-sh/ruff/issues/13694, which is marked help-wanted. --------- Co-authored-by: David Peter --- .../resources/mdtest/narrow/bool-call.md | 32 ++++++++ .../src/types/narrow.rs | 78 +++++++++++-------- 2 files changed, 77 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 0000000000..d7ae47b4fd --- /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): # 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): # 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 93e2f5afa3..5b46cf3a5a 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -388,46 +388,58 @@ impl<'db> NarrowingConstraintsBuilder<'db> { let scope = self.scope(); let inference = infer_expression_types(self.db, expression); + 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 inference - .expression_ty(expr_call.func.scoped_expression_id(self.db, scope)) - .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) - } + let class_info_ty = + inference.expression_ty(class_info.scoped_expression_id(self.db, scope)); + + 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) } - 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 - }, - ) - } else { - None - } + 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 + }, + ) + } + // 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, }