diff --git a/crates/ty_python_core/src/builder.rs b/crates/ty_python_core/src/builder.rs index cfa7ae1343448d..867bda884d3ca2 100644 --- a/crates/ty_python_core/src/builder.rs +++ b/crates/ty_python_core/src/builder.rs @@ -228,6 +228,13 @@ impl ConditionFlowSnapshot { } } +#[derive(Clone, Copy, Debug, Default)] +enum ExpressionContext { + #[default] + Value, + Condition, +} + pub(super) struct SemanticIndexBuilder<'db, 'ast> { // Builder state db: &'db dyn Db, @@ -253,6 +260,9 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> { has_future_annotations: bool, /// Whether we are currently visiting an `if TYPE_CHECKING` block. in_type_checking_block: bool, + /// How the next `visit_expr` consumes its expression. The visitor resets this to `Value` + /// before walking children; only short-circuit expressions propagate it to their operands. + next_expression_context: ExpressionContext, // Used for checking semantic syntax errors resolver_environment: ResolverEnvironment<'db>, @@ -324,6 +334,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { has_future_annotations: false, in_type_checking_block: false, + next_expression_context: ExpressionContext::Value, scopes: IndexVec::new(), place_tables: IndexVec::new(), @@ -2149,12 +2160,16 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { &mut self, predicate_node: &'ast ast::Expr, ) -> (PredicateOrLiteral<'db>, ScopedPredicateId) { - let predicate = self.build_predicate(predicate_node); + let predicate = self.build_predicate(predicate_node, ExpressionContext::Condition); let predicate_id = self.record_narrowing_constraint(predicate); (predicate, predicate_id) } - fn build_predicate(&mut self, predicate_node: &'ast ast::Expr) -> PredicateOrLiteral<'db> { + fn build_predicate( + &mut self, + predicate_node: &'ast ast::Expr, + context: ExpressionContext, + ) -> PredicateOrLiteral<'db> { // Some commonly used test expressions are eagerly evaluated as `true` // or `false` here for performance reasons. This list does not need to // be exhaustive. More complex expressions will still evaluate to the @@ -2186,7 +2201,17 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { match resolve_to_literal(predicate_node) { Some(literal) => PredicateOrLiteral::Literal(literal), None => PredicateOrLiteral::Predicate(Predicate { - node: PredicateNode::Expression(expression), + node: if matches!(context, ExpressionContext::Condition) + && match predicate_node { + ast::Expr::BoolOp(_) | ast::Expr::If(_) => true, + ast::Expr::UnaryOp(unary) => unary.op == ast::UnaryOp::Not, + ast::Expr::Compare(compare) => compare.ops.len() > 1, + _ => false, + } { + PredicateNode::Condition(expression) + } else { + PredicateNode::Expression(expression) + }, is_positive: true, }), } @@ -2245,7 +2270,8 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { let place_table = self.current_place_table(); match pred.node { - PredicateNode::Expression(expression) => { + PredicateNode::Expression(expression) + | PredicateNode::Condition(expression) => { let expression_node = expression.node_ref(self.db).node(self.module); let mut places = PossiblyNarrowedPlacesBuilder::new(self.db, place_table) .expression(expression_node); @@ -3004,7 +3030,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { /// print(last) /// ``` fn visit_comprehension_filter(&mut self, if_expr: &'ast ast::Expr) -> FlowSnapshot { - self.visit_expr(if_expr); + self.visit_expr_with_context(if_expr, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(if_expr); let filtered_out = if let Some(snapshots) = condition_flow_snapshot.into_branches() { self.flow_restore(snapshots.truthy); @@ -3269,13 +3295,18 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { .get_or_init(|| source_text(self.db, self.file.file(self.db))) } + fn visit_expr_with_context(&mut self, expr: &'ast ast::Expr, context: ExpressionContext) { + self.next_expression_context = context; + self.visit_expr(expr); + } + /// Visits a conditional expression without reserving its flow snapshots in every recursive /// expression-visitor frame. This matters for deeply nested expressions in unoptimized builds. - fn visit_if_expression(&mut self, node: &'ast ast::ExprIf) { + fn visit_if_expression(&mut self, node: &'ast ast::ExprIf, context: ExpressionContext) { let ast::ExprIf { body, test, orelse, .. } = node; - self.visit_expr(test); + self.visit_expr_with_context(test, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(test); let falsy = if let Some(snapshots) = condition_flow_snapshot.into_branches() { self.flow_restore(snapshots.truthy); @@ -3288,7 +3319,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { let in_type_checking_block = self.in_type_checking_block; self.current_use_def_map_mut() .record_range_reachability(body.range(), in_type_checking_block); - self.visit_expr(body); + self.visit_expr_with_context(body, context); let post_body = self.flow_snapshot(); self.flow_restore(falsy); @@ -3297,12 +3328,12 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { let in_type_checking_block = self.in_type_checking_block; self.current_use_def_map_mut() .record_range_reachability(orelse.range(), in_type_checking_block); - self.visit_expr(orelse); + self.visit_expr_with_context(orelse, context); self.flow_merge(post_body); } /// Keeps short-circuit flow snapshots out of the common recursive expression-visitor frame. - fn visit_bool_expression(&mut self, node: &'ast ast::ExprBoolOp) { + fn visit_bool_expression(&mut self, node: &'ast ast::ExprBoolOp, context: ExpressionContext) { let ast::ExprBoolOp { values, op, .. } = node; let mut snapshots = vec![]; let mut reachability_constraints = vec![]; @@ -3317,7 +3348,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { let in_type_checking_block = self.in_type_checking_block; self.current_use_def_map_mut() .record_range_reachability(value.range(), in_type_checking_block); - self.visit_expr(value); + self.visit_expr_with_context(value, context); // Only non-final values can short-circuit this boolean operation. The final // value can still have its own outcome-specific flow if it is nested. @@ -3326,7 +3357,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { value, )); let condition_flow_snapshots = self.take_condition_flow_snapshots(value); - let predicate = self.build_predicate(value); + let predicate = self.build_predicate(value, context); let possibly_narrowed = self.compute_possibly_narrowed_places(&predicate); let predicate_id = match op { ast::BoolOp::And => self.add_predicate(predicate), @@ -3866,9 +3897,9 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { // `msg` branch back into the following flow, since there is no way of getting out // of that branch. Code after the assertion starts from the condition's truthy flow. - self.visit_expr(test); + self.visit_expr_with_context(test, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(test); - let predicate = self.build_predicate(test); + let predicate = self.build_predicate(test, ExpressionContext::Condition); if msg.is_some() || self @@ -4039,7 +4070,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { } } ast::Stmt::If(node) => { - self.visit_expr(&node.test); + self.visit_expr_with_context(&node.test, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(&node.test); let mut falsy = if let Some(snapshots) = condition_flow_snapshot.into_branches() { self.flow_restore(snapshots.truthy); @@ -4093,7 +4124,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { self.record_negated_reachability_constraint(last_reachability_constraint); let next_falsy = if let Some(elif_test) = clause_test { - self.visit_expr(elif_test); + self.visit_expr_with_context(elif_test, ExpressionContext::Condition); // A test expression is evaluated whether the branch is taken or not let condition_flow_snapshot = self.flow_snapshot_for_condition(elif_test); let next_falsy = @@ -4176,7 +4207,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { // Visit the test expression after creating loop headers, so that loop-back values // are visible. - self.visit_expr(test); + self.visit_expr_with_context(test, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(test); // Take the pre_loop snapshot from the post-test fallback flow before restoring the @@ -4554,7 +4585,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { // while the next case is reached through `!P || (P && !G)`. Save `P && !G` // separately so it can be merged with the pattern-failure state after the body. let match_success_guard_failure = case.guard.as_ref().map(|guard| { - self.visit_expr(guard); + self.visit_expr_with_context(guard, ExpressionContext::Condition); let condition_flow_snapshot = self.flow_snapshot_for_condition(guard); let falsy = if let Some(snapshots) = condition_flow_snapshot.into_branches() { @@ -5266,6 +5297,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { } fn visit_expr(&mut self, expr: &'ast ast::Expr) { + let context = std::mem::take(&mut self.next_expression_context); self.with_semantic_checker(|semantic, context| semantic.visit_expr(expr, context)); self.scopes_by_expression @@ -5399,7 +5431,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.visit_expr(lambda.body.as_ref()); self.pop_scope(); } - ast::Expr::If(node) => self.visit_if_expression(node), + ast::Expr::If(node) => self.visit_if_expression(node, context), ast::Expr::ListComp( list_comprehension @ ast::ExprListComp { elt, generators, .. @@ -5466,7 +5498,14 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.record_exception_checkpoint(); } ast::Expr::UnaryOp(unary) => { - walk_expr(self, expr); + self.visit_expr_with_context( + &unary.operand, + if unary.op == ast::UnaryOp::Not { + context + } else { + ExpressionContext::Value + }, + ); self.record_exception_checkpoint_if( unary.op != ast::UnaryOp::Not || !Self::condition_evaluation_is_known_safe(&unary.operand), @@ -5487,7 +5526,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { )); } } - ast::Expr::BoolOp(node) => self.visit_bool_expression(node), + ast::Expr::BoolOp(node) => self.visit_bool_expression(node, context), ast::Expr::StringLiteral(_) => { walk_expr(self, expr); } diff --git a/crates/ty_python_core/src/predicate.rs b/crates/ty_python_core/src/predicate.rs index 68ffc529421301..c2a7b6e9bb198c 100644 --- a/crates/ty_python_core/src/predicate.rs +++ b/crates/ty_python_core/src/predicate.rs @@ -114,7 +114,14 @@ pub struct CallableAndCallExpr<'db> { #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, get_size2::GetSize, salsa::SalsaValue)] pub enum PredicateNode<'db> { + /// The truthiness of an expression's resulting value. Expression(Expression<'db>), + /// A short-circuit expression evaluated directly as a condition: a boolean operation, `not`, + /// a chained comparison, or a conditional expression. + /// + /// In `if x and False`, the truthy branch is unreachable. But after `y = x and False`, + /// `if y` may be truthy: it can call `x.__bool__` a second time and get a different result. + Condition(Expression<'db>), /// Whether a context manager's exit return type allows an exception to be suppressed. /// /// Resolved during type inference because the context manager's type is unavailable during diff --git a/crates/ty_python_semantic/resources/mdtest/annotations/annotated.md b/crates/ty_python_semantic/resources/mdtest/annotations/annotated.md index 846c92e4ca1c8d..6290a26b0fd9be 100644 --- a/crates/ty_python_semantic/resources/mdtest/annotations/annotated.md +++ b/crates/ty_python_semantic/resources/mdtest/annotations/annotated.md @@ -37,6 +37,15 @@ def convert(value: "Annotated[str, dict(**{'name': 'value'})]") -> "Annotated[in return 1 ``` +Conditional expressions are also valid metadata and do not affect the annotated type. + +```py +def flag() -> bool: + return True + +conditional_value: "Annotated[int, 1 if flag() else 2]" = 1 +``` + ## Inside `type[...]` `Annotated` can wrap a class or specialized generic class inside `type[...]` without changing the diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index b64b7ae96db9f2..97f87b819ee2d9 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/crates/ty_python_semantic/resources/mdtest/attributes.md @@ -4128,6 +4128,44 @@ class Foo: ... reveal_type(Foo.__class__) # revealed: ``` +## Classes of recursive intersections + +Mutually recursive aliases can describe the same union of classes. Computing the class of their +intersection preserves every possible class, regardless of the order in which the aliases are +expanded. + +```toml +[environment] +python-version = "3.12" +``` + +```py +from ty_extensions import Intersection + +type First = Second | int +type Second = First | str + +def recursive(value: Intersection[First, Second]): + reveal_type(type(value)) # revealed: type[int | str] +``` + +## Classes of recursive generic aliases + +Recursive specialization can introduce classes beyond the initial type argument. Class inference +terminates even when the type arguments keep growing, conservatively returning `type`. + +```toml +[environment] +python-version = "3.12" +``` + +```py +type Growing[T] = T | Growing[list[T]] + +def growing(value: Growing[int]): + reveal_type(type(value)) # revealed: type +``` + ## Module attributes ### Basic diff --git a/crates/ty_python_semantic/resources/mdtest/boolean/short_circuit.md b/crates/ty_python_semantic/resources/mdtest/boolean/short_circuit.md index 4c314e3cba76d2..f222b354327ba2 100644 --- a/crates/ty_python_semantic/resources/mdtest/boolean/short_circuit.md +++ b/crates/ty_python_semantic/resources/mdtest/boolean/short_circuit.md @@ -192,3 +192,168 @@ def match_guard(flag: bool, subject: object): def comprehension_filter(flag: bool): [reveal_type(x) for _ in range(1) if flag and (x := 1)] # revealed: Literal[1] ``` + +## Reachability of compound conditions + +An `and` condition with an always-falsy operand cannot take the truthy branch. Similarly, an `or` +condition with an always-truthy operand cannot take the falsy branch. This holds even when another +operand has mutable truthiness, and when conditions contain nested boolean operations or `not`. + +```py +def conditions(value: object): + if value and False: + "".missing + + if value or True: + pass + else: + "".missing + + if not (value and False): + pass + else: + "".missing + + if (value and False) or not (value or True): + "".missing +``` + +A comparison can also return a value with unknown truthiness. That does not make a subsequent +always-falsy operand optional when deciding whether to enter the branch. + +```py +def comparison(value): + if value is not None and value != "" and False: + "".missing +``` + +Short-circuiting also skips later operands within a condition, including after nested boolean +operations. + +```py +def nested_operands(value): + if value is not None and (value != "" and False) and "".missing: + pass + + if value is None or (value != "" or True) or "".missing: + pass + + if value is not None and not (value != "" or True) and "".missing: + pass +``` + +The same short-circuit rules apply to loop conditions, assertions, conditional expressions, +comprehension filters, and match guards. + +```py +def other_conditions(value: object): + while value and False: + "".missing + + assert value or True, value.missing + + "".missing if value and False else None + + [item.missing for item in range(1) if value and False] + + match value: + case _ if value and False: + "".missing + + assert value and False + "".missing +``` + +## Conditional expressions used as conditions + +When a conditional expression is itself a condition, its selected branch is evaluated as a condition +too. The unselected branch does not affect whether the condition is truthy. + +```py +def conditional_expressions(value: object, flag: bool): + if (value and False) if flag else False: + "".missing + + if True if flag else (value or True): + pass + else: + "".missing + + if True if value and False else False: + "".missing +``` + +## Chained comparison conditions + +A comparison chain used as a condition is falsy if any comparison is always falsy, even if an +earlier comparison can return a value with mutable truthiness. + +```py +def comparisons(value): + if value is not None and value == 1 < 0: + "".missing + + if value is None: + return + + if value == 1 < 0: + "".missing + + if value == 1 < 0 < value: + "".missing + + if (value == 1 < 0) and True: + "".missing + + if (value == 1 < 0) and "".missing: + pass + + if not (value == 1 < 0): + pass + else: + "".missing +``` + +## Re-testing boolean expression results + +Saving the result of `value and False` and then testing it can call `value.__bool__` twice. The +second call may return a different result, so the truthy branch remains reachable. Assignment +expressions and nested boolean operations in value contexts can also cause this extra test. + +```py +class MutableTruthiness: + truthy: bool = False + + def __bool__(self) -> bool: + self.truthy = not self.truthy + return self.truthy + +def expressions(value: MutableTruthiness, flag: bool): + saved = value and False + if saved: + "".missing # error: [unresolved-attribute] + + if saved := value and False: + "".missing # error: [unresolved-attribute] + + saved = (value and False) if flag else False + if saved: + "".missing # error: [unresolved-attribute] + + result = (value and False) and "".missing # error: [unresolved-attribute] + result = (not (value or True)) or "".missing # error: [unresolved-attribute] +``` + +Saving a comparison chain's result can likewise lead to re-testing a non-boolean comparison result. + +```py +def saved_comparison(value): + if value is None: + return + result = value == 1 < 0 + if result: + "".missing # error: [unresolved-attribute] + + if result := value == 1 < 0: + "".missing # error: [unresolved-attribute] +``` diff --git a/crates/ty_python_semantic/resources/mdtest/comparison/intersections.md b/crates/ty_python_semantic/resources/mdtest/comparison/intersections.md index f485531b048acb..da57275bbc1084 100644 --- a/crates/ty_python_semantic/resources/mdtest/comparison/intersections.md +++ b/crates/ty_python_semantic/resources/mdtest/comparison/intersections.md @@ -171,6 +171,267 @@ def f(value: object, other_b: B) -> None: value.does_not_exist # no error (unreachable branch) ``` +## Non-boolean comparison results + +Rich comparisons preserve their declared return types after narrowing an operand to an intersection. +A return type that is disjoint from `bool` does not make the comparison unreachable, and an `int` +return type is not narrowed to its `bool` subtype. + +```py +class Comparison: + def __eq__(self, other: object) -> str: # error: [invalid-method-override] + return "equal" + + def __ne__(self, other: object) -> bytes: # error: [invalid-method-override] + return b"different" + + def __lt__(self, other: object) -> int: + return 42 + + def __contains__(self, other: object) -> str: + return "contained" + +class Excluded: ... + +def compare(value: Comparison): + if not isinstance(value, Excluded): + reveal_type(value == 0) # revealed: str + reveal_type(value != 0) # revealed: bytes + reveal_type(value < 0) # revealed: int +``` + +Membership tests still convert their result to `bool`, even when `__contains__` returns another +type. + +```py +def membership(value: Comparison): + if not isinstance(value, Excluded): + reveal_type(0 in value) # revealed: bool + reveal_type(0 not in value) # revealed: bool +``` + +A comparison that always raises still has type `Never` when another component inherits +`object.__eq__`. + +```py +from typing_extensions import Never + +class NonReturning: + def __eq__(self, other: object) -> Never: + raise RuntimeError + +def never_returns(value: NonReturning): + if isinstance(value, Excluded): + reveal_type(value == 0) # revealed: Never +``` + +## Conditionally defined comparison methods + +A conditional comparison method can fall back to the inherited `object` method. Narrowing its +receiver preserves both the custom result and the boolean fallback. + +```py +def enabled() -> bool: + return True + +class Conditional: + if enabled(): + def __eq__(self, other: object) -> str: # error: [invalid-method-override] + return "equal" + +class Excluded: ... + +def compare(value: Conditional): + if not isinstance(value, Excluded): + reveal_type(value == 0) # revealed: str | bool +``` + +The conditional method must not be discarded in favor of a reflected method that returns a different +boolean literal. The left operand can return `True` without calling the right operand's method. + +```py +from typing_extensions import Literal + +class ConditionalTrue: + if enabled(): + def __eq__(self, other: object) -> Literal[True]: + return True + +class ReflectedFalse: + def __eq__(self, other: object) -> Literal[False]: + return False + +def reflected(left: ConditionalTrue, right: ReflectedFalse): + if not isinstance(left, Excluded): + reveal_type(left == right) # revealed: bool +``` + +An intersection with another class that defines a boolean comparison still permits either result of +the conditional method. + +```py +class BooleanComparison: + def __eq__(self, other: object) -> bool: + return False + +def positive(left: ConditionalTrue): + if isinstance(left, BooleanComparison): + reveal_type(left == 0) # revealed: bool +``` + +The same applies when both classes define their comparison methods conditionally. + +```py +class ConditionalBoolean: + if enabled(): + def __eq__(self, other: object) -> bool: + return False + +def both_conditional(left: ConditionalTrue): + if isinstance(left, ConditionalBoolean): + reveal_type(left == 0) # revealed: bool +``` + +## Comparison methods returning `Self` + +A comparison method annotated with `Self` returns the full intersection receiver, just like an +explicit method call. Excluding a class from the receiver also excludes it from the comparison +result. + +```py +from __future__ import annotations +from typing_extensions import Self + +class Index: + def __eq__(self, other: object) -> Self: # error: [invalid-method-override] + return self + + def __lt__(self, other: object) -> Self: + return self + + def __gt__(self, other: object) -> Index: + return Index() + +class MultiIndex: ... + +def equality(index: Index): + if not isinstance(index, MultiIndex): + reveal_type(index.__eq__("")) # revealed: Index & ~MultiIndex + reveal_type(index == "") # revealed: Index & ~MultiIndex +``` + +An `and` expression can return either `False` or the comparison result; it does not convert the +comparison result to a boolean. + +```py +def conjunction(index: Index): + reveal_type(not isinstance(index, MultiIndex) and index == "") # revealed: Literal[False] | (Index & ~MultiIndex) +``` + +Positive intersection components are preserved as well. An inherited `object.__eq__` on another +component does not restrict a custom comparison's result to `bool`. + +```py +def positive(index: Index): + if isinstance(index, MultiIndex): + reveal_type(index == "") # revealed: Index & MultiIndex +``` + +Reflected comparisons bind `Self` to the right-hand receiver. In contrast, a concrete return +annotation does not inherit constraints from the receiver: `__gt__` can return a different `Index`. + +```py +def reflected(index: Index): + if not isinstance(index, MultiIndex): + reveal_type(0 > index) # revealed: Index & ~MultiIndex + reveal_type(index > 0) # revealed: Index +``` + +## Comparison results containing `Self` + +Receiver binding also applies when `Self` occurs inside the comparison's return type, rather than +being the entire return type. + +```py +from typing_extensions import Self + +class Comparison: + def __eq__(self, other: object) -> tuple[Self]: # error: [invalid-method-override] + return (self,) + +class Excluded: ... + +def equality(value: Comparison): + if not isinstance(value, Excluded): + reveal_type(value.__eq__(0)) # revealed: tuple[Comparison & ~Excluded] + reveal_type(value == 0) # revealed: tuple[Comparison & ~Excluded] +``` + +## Reflected comparisons with narrowed receivers + +Excluding an unrelated class from the left operand does not prevent the right operand from being a +subclass with a reflected comparison method. Since the runtime classes of these operands are not +known exactly, either method can supply the result. + +```py +class Base: + def __lt__(self, other: object) -> int: + return 42 + +class Child(Base): + def __gt__(self, other: object) -> str: + return "reflected" + +class Excluded: ... + +def compare(left: Base, right: Child): + if not isinstance(left, Excluded): + reveal_type(left < right) # revealed: int | str +``` + +## NewTypes in intersection comparisons + +Narrowing a `NewType` of `float` preserves the comparison operations supported by its base type, +including comparisons where both operands are intersections. + +```py +from typing import NewType + +Float = NewType("Float", float) + +class Excluded: ... + +def compare(left: Float, right: Float): + if not isinstance(left, Excluded) and not isinstance(right, Excluded): + reveal_type(left < right) # revealed: bool +``` + +## Comparisons with multiple union return types + +A comparison can have different union return types on its positive intersection components. When +distributing those unions exceeds the complexity limit, we keep a wider union of their return types +instead of losing their non-boolean results. + +```py +class A: ... +class B: ... +class C: ... +class D: ... +class E: ... + +class First: + def __lt__(self, other: object) -> A | B | C: + return A() + +class Second: + def __lt__(self, other: object) -> D | E: + return D() + +def compare(value: First): + if isinstance(value, Second): + reveal_type(value < 0) # revealed: A | B | C | D | E +``` + ## Diagnostics ### Unsupported operators for positive contributions @@ -230,6 +491,8 @@ def _(x: object): reveal_type(2 in x) # revealed: bool reveal_type(2 is x) # revealed: bool + reveal_type(x == 0) # revealed: bool + x < 0 # error: [unsupported-operator] ``` ```snapshot diff --git a/crates/ty_python_semantic/resources/mdtest/conditional/if_expression.md b/crates/ty_python_semantic/resources/mdtest/conditional/if_expression.md index 082e0d43dbbfdc..4964e9eadd9555 100644 --- a/crates/ty_python_semantic/resources/mdtest/conditional/if_expression.md +++ b/crates/ty_python_semantic/resources/mdtest/conditional/if_expression.md @@ -36,6 +36,31 @@ def _(flag: bool): reveal_type(x) # revealed: Literal[1] | None ``` +## Statically known compound conditions + +Short-circuit conditions can select a single branch even when an operand has mutable truthiness. +Saving the condition's value and testing it again does not provide the same guarantee. + +```py +def _(value: object): + reveal_type(1 if value and False else 2) # revealed: Literal[2] + reveal_type(1 if value or True else 2) # revealed: Literal[1] + + saved = value and False + reveal_type(1 if saved else 2) # revealed: Literal[1, 2] +``` + +This also applies when comparisons return values with unknown truthiness, including in comparison +chains. + +```py +def _(value): + if value is None: + return + reveal_type(True if value != "" and False else False) # revealed: Literal[False] + reveal_type(False if value == 1 < 0 else True) # revealed: Literal[True] +``` + ## Condition with object that implements `__bool__` incorrectly ```py diff --git a/crates/ty_python_semantic/resources/mdtest/with/sync.md b/crates/ty_python_semantic/resources/mdtest/with/sync.md index 7e0f781e52f451..92fd50dbd57baa 100644 --- a/crates/ty_python_semantic/resources/mdtest/with/sync.md +++ b/crates/ty_python_semantic/resources/mdtest/with/sync.md @@ -558,6 +558,28 @@ def _(context_expr: Manager1 | Manager2): reveal_type(f) # revealed: str | int ``` +## Context managers returning narrowed `Self` + +A context manager whose `__enter__` returns `Self` preserves narrowing of the context expression in +the bound value. + +```py +from typing_extensions import Self + +class Manager: + def __enter__(self) -> Self: + return self + + def __exit__(self, *args: object) -> None: ... + +class Excluded: ... + +def use(manager: Manager): + if not isinstance(manager, Excluded): + with manager as value: + reveal_type(value) # revealed: Manager & ~Excluded +``` + ## Type aliases preserve context manager behavior ```toml diff --git a/crates/ty_python_semantic/src/reachability.rs b/crates/ty_python_semantic/src/reachability.rs index fed33131357f05..da3c93494e4554 100644 --- a/crates/ty_python_semantic/src/reachability.rs +++ b/crates/ty_python_semantic/src/reachability.rs @@ -204,12 +204,14 @@ use crate::{ CallableTypes, ComparisonSoundnessPolicy, EnumClassLiteral, KnownInstanceType, NarrowingConstraint, SpecialFormType, Type, TypeContext, UnionType, callable_pattern_type, definite_match_pattern_type, definite_match_pattern_type_for_subject, equality_truthiness, - expand_type, infer_narrowing_constraints, infer_same_file_expression_type, - mapping_pattern_type, pattern_binding_fallthrough_type, sequence_pattern_type_builder, - singleton_pattern_type, + expand_type, infer_expression_types, infer_narrowing_constraints, + infer_same_file_expression_type, mapping_pattern_type, pattern_binding_fallthrough_type, + sequence_pattern_type_builder, singleton_pattern_type, }, }; +use ruff_db::parsed::parsed_module; use ruff_index::{Idx, IndexSlice}; +use ruff_python_ast as ast; use ruff_python_ast::name::Name; use ruff_text_size::TextRange; use rustc_hash::{FxHashMap, FxHashSet}; @@ -544,6 +546,7 @@ const REACHABILITY_EVALUATION_CHUNK_SIZE: usize = 256; fn predicate_scope<'db>(db: &'db dyn Db, predicate: &Predicate<'db>) -> ScopeId<'db> { match predicate.node { PredicateNode::Expression(expression) + | PredicateNode::Condition(expression) | PredicateNode::ContextManagerSuppresses { expression, .. } => expression.scope(db), PredicateNode::IsNonTerminalCall(CallableAndCallExpr { callable, .. }) => { callable.scope(db) @@ -1589,6 +1592,65 @@ fn analyze_non_empty_iterable(db: &dyn Db, iterable: Expression) -> Truthiness { } } +/// Evaluate a condition without re-testing intermediate short-circuit results. +pub(crate) fn analyze_condition_expression( + node: &ast::Expr, + leaf_truthiness: &impl Fn(&ast::Expr) -> Truthiness, +) -> Truthiness { + match node { + ast::Expr::BoolOp(ast::ExprBoolOp { op, values, .. }) => { + let short_circuit = Truthiness::from(*op == ast::BoolOp::Or); + let mut result = short_circuit.negate(); + for value in values { + let truthiness = analyze_condition_expression(value, leaf_truthiness); + if truthiness == short_circuit { + return short_circuit; + } + if truthiness.is_ambiguous() { + result = Truthiness::Ambiguous; + } + } + result + } + ast::Expr::UnaryOp(ast::ExprUnaryOp { + op: ast::UnaryOp::Not, + operand, + .. + }) => analyze_condition_expression(operand, leaf_truthiness).negate(), + ast::Expr::If(ast::ExprIf { + test, body, orelse, .. + }) => match analyze_condition_expression(test, leaf_truthiness) { + Truthiness::AlwaysTrue => analyze_condition_expression(body, leaf_truthiness), + Truthiness::AlwaysFalse => analyze_condition_expression(orelse, leaf_truthiness), + Truthiness::Ambiguous => { + let body_truthiness = analyze_condition_expression(body, leaf_truthiness); + if body_truthiness == analyze_condition_expression(orelse, leaf_truthiness) { + body_truthiness + } else { + Truthiness::Ambiguous + } + } + }, + _ => leaf_truthiness(node), + } +} + +#[salsa::tracked( + returns(copy), + cycle_initial = |_, _, _| Truthiness::Ambiguous, + heap_size = get_size2::GetSize::get_heap_size +)] +fn analyze_condition<'db>(db: &'db dyn Db, expression: Expression<'db>) -> Truthiness { + let env = ProgramEnvironment::from_scope(expression.scope(db)); + let module = parsed_module(db, expression.python_file(db)).load(db); + let inference = infer_expression_types(db, expression, TypeContext::default()); + analyze_condition_expression(expression.node_ref(db).node(&module), &|node| { + inference + .comparison_truthiness(node) + .unwrap_or_else(|| inference.expression_type(node).bool(db, &env)) + }) +} + fn analyze_single(db: &dyn Db, env: &ProgramEnvironment<'_>, predicate: &Predicate) -> Truthiness { let _span = tracing::trace_span!("analyze_single", ?predicate).entered(); @@ -1598,6 +1660,9 @@ fn analyze_single(db: &dyn Db, env: &ProgramEnvironment<'_>, predicate: &Predica .bool(db, env) .negate_if(!predicate.is_positive) } + PredicateNode::Condition(test_expr) => { + analyze_condition(db, test_expr).negate_if(!predicate.is_positive) + } PredicateNode::ContextManagerSuppresses { expression, is_async, diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 9cfdf94f125b43..3acecd6a87e096 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -19,16 +19,16 @@ use ruff_db::parsed::parsed_module; use ruff_python_ast as ast; use ruff_python_ast::name::Name; use ruff_text_size::Ranged; -use smallvec::smallvec_inline; +use smallvec::{SmallVec, smallvec_inline}; use ty_module_resolver::{ ImportingFile, KnownModule, Module, ModuleName, file_to_module, resolve_module, }; pub(crate) use self::callable::UpcastPolicy; use self::class::ClassInstanceFlags; -use self::cyclic::ActiveRecursionDetector; pub use self::cyclic::CycleDetector; pub(crate) use self::cyclic::TypeTransformer; +use self::cyclic::{ActiveRecursionDetector, TypeIdentity}; pub use self::dedicated::pytest::{FixtureBinding, fixture_bindings_for_parameter}; pub(crate) use self::diagnostic::TypeCheckDiagnostics; pub(crate) use self::diagnostic::register_lints; @@ -398,6 +398,38 @@ fn definition_expression_annotation<'db>( } } +#[derive(Default)] +struct MetaTypeVisitor<'db> { + active_types: ActiveRecursionDetector>, + active_identities: ActiveRecursionDetector>, +} + +impl<'db> MetaTypeVisitor<'db> { + fn visit( + &self, + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + ty: Type<'db>, + project: impl FnOnce() -> Type<'db>, + ) -> Type<'db> { + // A repeated specialization adds no new classes to a recursive union. Changing + // type arguments can introduce other classes, so use an unconstrained metatype. + // Do not cache results: a projection made while another alias is active can omit + // classes that are only encountered later in that alias's union. + self.active_types.visit( + &ty, + || Type::Never, + || { + self.active_identities.visit( + &ty.to_type_identity(db), + || KnownClass::Type.to_instance(db, env), + project, + ) + }, + ) + } +} + struct ApplyTypeMappingTag; struct ApplyMaterializationEquivalence; @@ -6862,14 +6894,9 @@ impl<'db> Type<'db> { policy: MemberLookupPolicy, ) -> Result, CallDunderError<'db>> { if let Type::Intersection(intersection) = self { - return intersection.try_call_dunder_with_policy( - db, - env, - name, - argument_types, - tcx, - policy, - ); + return intersection + .try_call_dunder_with_policy(db, env, name, argument_types, tcx, policy) + .map(|bindings| bindings.into_bindings(self)); } if let Type::Union(union) = self { @@ -6879,7 +6906,23 @@ impl<'db> Type<'db> { // Implicit calls to dunder methods never access instance members, so we pass // `NO_INSTANCE_FALLBACK` here in addition to other policies: let policy = policy | MemberLookupPolicy::NO_INSTANCE_FALLBACK; - match self.member_lookup_with_policy(db, env, name, policy).place { + Self::try_call_dunder_member( + db, + env, + self.member_lookup_with_policy(db, env, name, policy).place, + argument_types, + tcx, + ) + } + + fn try_call_dunder_member( + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + member: Place<'db>, + argument_types: &CallArguments<'_, 'db>, + tcx: TypeContext<'db>, + ) -> Result, CallDunderError<'db>> { + match member { Place::Defined(DefinedPlace { ty: dunder_callable, definedness: boundness, @@ -6926,36 +6969,13 @@ impl<'db> Type<'db> { argument_types: &CallArguments<'_, 'db>, tcx: TypeContext<'db>, ) -> Result, CallDunderError<'db>> { - match self.member(db, env, name).place { - Place::Defined(DefinedPlace { - ty: dunder_callable, - definedness: boundness, - provenance, - .. - }) => { - let constraints = ConstraintSetBuilder::new(); - let bindings = dunder_callable - .bindings(db, env) - .match_parameters(db, env, argument_types) - .check_types(db, env, &constraints, argument_types, tcx, &[]); - - let bindings = match bindings { - Ok(bindings) => bindings, - Err(CallError(kind, bindings)) => { - return Err(CallDunderError::CallError(kind, bindings, provenance)); - } - }; - - if boundness == Definedness::PossiblyUndefined { - return Err(CallDunderError::PossiblyUnbound { - bindings: Box::new(bindings), - unbound_on: None, - }); - } - Ok(bindings) - } - Place::Undefined => Err(CallDunderError::MethodNotAvailable), - } + Self::try_call_dunder_member( + db, + env, + self.member(db, env, name).place, + argument_types, + tcx, + ) } /// Return whether a custom `__getattribute__` could affect this lookup. @@ -7709,6 +7729,15 @@ impl<'db> Type<'db> { /// See `Self::dunder_class` for more details. #[must_use] fn to_meta_type(self, db: &'db dyn Db, env: &ProgramEnvironment<'db>) -> Type<'db> { + self.to_meta_type_impl(db, env, &MetaTypeVisitor::default()) + } + + fn to_meta_type_impl( + self, + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + visitor: &MetaTypeVisitor<'db>, + ) -> Type<'db> { match self { Type::Never => Type::Never, Type::NominalInstance(instance) => instance.to_meta_type(db, env), @@ -7718,9 +7747,9 @@ impl<'db> Type<'db> { property.instance_class(db).to_class_literal(db, env) } Type::SlotDescriptor(_) => KnownClass::MemberDescriptorType.to_class_literal(db, env), - Type::Union(union) => union.map(db, env, |ty| ty.to_meta_type(db, env)), + Type::Union(union) => union.map(db, env, |ty| ty.to_meta_type_impl(db, env, visitor)), Type::TypeIs(_) | Type::TypeGuard(_) => KnownClass::Bool.to_class_literal(db, env), - Type::TypeForm(_) => Type::object().to_meta_type(db, env), + Type::TypeForm(_) => Type::object().to_meta_type_impl(db, env, visitor), Type::LiteralValue(literal) => match literal.kind() { LiteralValueTypeKind::Bool(_) => KnownClass::Bool.to_class_literal(db, env), LiteralValueTypeKind::Bytes(_) => KnownClass::Bytes.to_class_literal(db, env), @@ -7758,13 +7787,13 @@ impl<'db> Type<'db> { Type::Divergent(_) => self, Type::Intersection(intersection) => { if let Some(alternatives) = intersection.finite_alternative_union(db, env) { - alternatives.to_meta_type(db, env) + alternatives.to_meta_type_impl(db, env, visitor) } else { // Negative constraints do not generally constrain classes: `int & ~Literal[0]` // still has meta-type `type[int]`. Pure negations are bounded by `object`. let mut builder = IntersectionBuilder::new(db, env); for positive in intersection.positive_elements_or_object(db) { - builder.add_positive_in_place(positive.to_meta_type(db, env)); + builder.add_positive_in_place(positive.to_meta_type_impl(db, env, visitor)); } // An exclusion can narrow a type variable's union bound to a definite class: @@ -7789,7 +7818,9 @@ impl<'db> Type<'db> { _ => None, } { - builder.add_positive_in_place(narrowed_bound.to_meta_type(db, env)); + builder.add_positive_in_place( + narrowed_bound.to_meta_type_impl(db, env, visitor), + ); } builder.build() @@ -7797,7 +7828,7 @@ impl<'db> Type<'db> { } Type::EnumComplement(complement) => complement .remaining_literal_union(db, env) - .to_meta_type(db, env), + .to_meta_type_impl(db, env, visitor), Type::AlwaysTruthy | Type::AlwaysFalsy => KnownClass::Type.to_instance(db, env), Type::BoundSuper(_) => KnownClass::Super.to_class_literal(db, env), // Class-member lookup on a protocol instance must use the protocol's nominal class. @@ -7814,8 +7845,14 @@ impl<'db> Type<'db> { todo_type!("TypedDict synthesized meta-type").expect_dynamic(), ), }, - Type::TypeAlias(alias) => alias.value_type(db).to_meta_type(db, env), - Type::NewTypeInstance(newtype) => newtype.concrete_base_type(db).to_meta_type(db, env), + Type::TypeAlias(alias) => visitor.visit(db, env, self, || { + alias.value_type(db).to_meta_type_impl(db, env, visitor) + }), + Type::NewTypeInstance(newtype) => visitor.visit(db, env, self, || { + newtype + .concrete_base_type(db) + .to_meta_type_impl(db, env, visitor) + }), } } @@ -9106,6 +9143,41 @@ impl<'db> Type<'db> { } } +/// Checked dunder calls, retaining union alternatives within each intersection component. +enum DunderBindings<'db> { + /// A complete call result, including finite alternatives or an `object` fallback. + Single(Box>), + /// Successful calls on positive intersection components. + Intersection(Vec>), +} + +impl<'db> DunderBindings<'db> { + fn into_bindings(self, receiver: Type<'db>) -> Bindings<'db> { + match self { + Self::Single(bindings) => *bindings, + Self::Intersection(bindings) => Bindings::from_intersection(receiver, bindings), + } + } + + fn return_type(&self, db: &'db dyn Db, env: &ProgramEnvironment<'db>) -> Type<'db> { + match self { + Self::Single(bindings) => bindings.return_type(db, env), + Self::Intersection(bindings) => { + let return_types: SmallVec<[Type<'db>; 1]> = bindings + .iter() + .map(|bindings| bindings.return_type(db, env)) + .collect(); + IntersectionType::bounded_from_elements(db, env, return_types.iter().copied()) + .unwrap_or_else(|| { + // If exact distribution exceeds the type budget, preserve every possible + // return type in a conservative union instead. + UnionType::from_elements(db, env, return_types) + }) + } + } + } +} + impl<'db> IntersectionType<'db> { /// Return whether the negation of this intersection is a subtype of `target`. /// @@ -9127,9 +9199,9 @@ impl<'db> IntersectionType<'db> { .all(|negative| negative.is_subtype_of(db, env, target)) } - // Calls the dunder on each element separately and combines the results. + // Calls the dunder on each element separately before combining the results. // This avoids intersecting bound methods (which often collapses to Never) - // and instead intersects the return types. + // and lets callers intersect return types without expanding complete call bindings. // // TODO: we might be able to remove this after fixing // https://github.com/astral-sh/ty/issues/2428. @@ -9141,29 +9213,44 @@ impl<'db> IntersectionType<'db> { argument_types: &mut CallArguments<'_, 'db>, tcx: TypeContext<'db>, policy: MemberLookupPolicy, - ) -> Result, CallDunderError<'db>> { + ) -> Result, CallDunderError<'db>> { if let Some(alternatives) = self.finite_alternative_union(db, env) { - return alternatives.try_call_dunder_with_policy( - db, - env, - name, - argument_types, - tcx, - policy, - ); + return alternatives + .try_call_dunder_with_policy(db, env, name, argument_types, tcx, policy) + .map(|bindings| DunderBindings::Single(Box::new(bindings))); } - // Using `positive()` rather than `positive_elements_or_object()` is safe - // here because `object` does not define any of the dunders that are called - // through this path without `MRO_NO_OBJECT_FALLBACK` (e.g. `__await__`, - // `__iter__`, `__enter__`, `__bool__`). + // Search components separately, but bind descriptors and `Self` to the full receiver. + // An inherited `object` method on an otherwise undefined component is only a fallback + // for the whole intersection: `object.__eq__` must not restrict another component's + // custom comparison result to `bool`. + let receiver = Type::Intersection(self); + let policy = policy | MemberLookupPolicy::NO_INSTANCE_FALLBACK; + let component_policy = policy | MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK; + let lookup = |element: Type<'db>, policy| { + element + .member_lookup_with_policy_and_receiver(db, env, name, policy, Some(receiver)) + .unwrap_or_else(|error| error.fallback_member(db)) + .place + }; let positive = self.positive(db); let mut successful_bindings = Vec::with_capacity(positive.len()); let mut last_error = None; let mut error_provenance = Provenance::Unknown; + let mut any_defined = false; for element in positive { - match element.try_call_dunder_with_policy(db, env, name, argument_types, tcx, policy) { + let mut member = lookup(*element, component_policy); + if let Place::Defined(defined) = member + && !defined.is_definitely_defined() + && !policy.mro_no_object_fallback() + { + // A conditional override can still fall back to `object`; include both + // possibilities instead of discarding a possibly undefined call. + member = lookup(*element, policy); + } + any_defined |= !member.is_undefined(); + match Type::try_call_dunder_member(db, env, member, argument_types, tcx) { Ok(bindings) => successful_bindings.push(bindings), Err(err) => { error_provenance = error_provenance.or(err.provenance()); @@ -9172,6 +9259,12 @@ impl<'db> IntersectionType<'db> { } } + if !any_defined && !policy.mro_no_object_fallback() { + let member = lookup(Type::object(), policy); + return Type::try_call_dunder_member(db, env, member, argument_types, tcx) + .map(|bindings| DunderBindings::Single(Box::new(bindings))); + } + if successful_bindings.is_empty() { // TODO we are only showing one of the errors here; should we aggregate // them somehow or show all of them? @@ -9180,10 +9273,7 @@ impl<'db> IntersectionType<'db> { .with_provenance(error_provenance)); } - Ok(Bindings::from_intersection( - Type::Intersection(self), - successful_bindings, - )) + Ok(DunderBindings::Intersection(successful_bindings)) } } diff --git a/crates/ty_python_semantic/src/types/call.rs b/crates/ty_python_semantic/src/types/call.rs index 763acacb6255ac..e4cd967a2edaf7 100644 --- a/crates/ty_python_semantic/src/types/call.rs +++ b/crates/ty_python_semantic/src/types/call.rs @@ -19,7 +19,7 @@ pub(super) use bind::{ /// /// `Possibly` requires preserving both dispatch results because the static types admit runtime /// pairs for which either method has priority. -#[derive(PartialEq, Eq)] +#[derive(PartialEq, Eq, PartialOrd, Ord)] enum ReflectedMethodPriority { Never, Possibly, @@ -82,6 +82,27 @@ fn reflected_method_priority<'db>( return ReflectedMethodPriority::Never; } + // Positive intersection components retain their nominal subclass relationships, even + // when a negative component prevents a subtype check against the full intersection. + // Any component can establish possible or definite priority for the reflected method. + match (left_ty, right_ty) { + (Type::Intersection(intersection), _) => { + return intersection + .positive_elements_or_object(db) + .map(|left| reflected_method_priority(db, env, left, right_ty)) + .max() + .unwrap_or(ReflectedMethodPriority::Never); + } + (_, Type::Intersection(intersection)) => { + return intersection + .positive_elements_or_object(db) + .map(|right| reflected_method_priority(db, env, left_ty, right)) + .max() + .unwrap_or(ReflectedMethodPriority::Never); + } + _ => {} + } + if let (Some(left_class), Some(right_class)) = ( operator_dispatch_class(db, env, left_ty), operator_dispatch_class(db, env, right_ty), @@ -116,12 +137,27 @@ impl<'db> Type<'db> { policy: MemberLookupPolicy, ) -> Option> { let call_dunder = |name, receiver: Type<'db>, argument: Type<'db>| { + let mut arguments = CallArguments::positional([argument]); + if let Type::Intersection(intersection) = receiver { + return intersection + .try_call_dunder_with_policy( + db, + env, + name, + &mut arguments, + TypeContext::default(), + policy, + ) + .map(|outcome| outcome.return_type(db, env)) + .ok(); + } + receiver .try_call_dunder_with_policy( db, env, name, - &mut CallArguments::positional([argument]), + &mut arguments, TypeContext::default(), policy, ) diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index a9eec2fa1e1c43..c1fa0b09d50346 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -70,7 +70,7 @@ use ty_python_core::expression::Expression; use ty_python_core::scope::ScopeId; use ty_python_core::statement::StatementInner; use ty_python_core::unpack::Unpack; -use ty_python_core::{ExpressionNodeKey, SemanticIndex, Statement, semantic_index}; +use ty_python_core::{ExpressionNodeKey, SemanticIndex, Statement, Truthiness, semantic_index}; mod builder; mod comparisons; @@ -1703,6 +1703,10 @@ struct ExpressionInferenceExtra<'db> { /// Metadata for type expressions in this region. type_expression_flags: FrozenMap, + /// Truthiness of chained comparisons evaluated directly as conditions, before their + /// potentially stateful comparison results can be saved and tested again. + comparison_truthiness: FrozenMap, + /// The constraints on any collection initializers that are accessed in this region. collection_use_constraints: CollectionUseConstraints<'db>, @@ -1755,6 +1759,16 @@ impl<'db> ExpressionInference<'db> { *binding_ty = binding_ty.recursive_type_normalized(db, env, cycle); } } + + if cycle.iteration() > crate::TAINTED_CYCLES { + // Widening operand types must not make reachability alternate between + // definite outcomes during cycle recovery. + for (expr, truthiness) in &mut extra.comparison_truthiness { + if previous.comparison_truthiness(*expr) != Some(*truthiness) { + *truthiness = Truthiness::Ambiguous; + } + } + } } for (expr, ty) in &mut self.expressions { @@ -1788,6 +1802,17 @@ impl<'db> ExpressionInference<'db> { .unwrap_or_else(Type::unknown) } + pub(crate) fn comparison_truthiness( + &self, + expression: impl Into, + ) -> Option { + self.extra + .as_deref()? + .comparison_truthiness + .get(&expression.into()) + .copied() + } + fn collection_use_constraints( &self, collection_def: Definition<'db>, diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index b2e8c2bf46331d..76f619a1da99c5 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -45,7 +45,9 @@ use crate::place_load::{ ImplicitPlaceLoad, PlaceExprPrefixLoad, PlaceExprPrefixLoads, PlaceLoadFailure, PlaceLoadMode, PlaceLoadResolutionStep, PlaceLoadSource, PlaceLoadSourceKind, resolve_place_load, }; -use crate::reachability::{ReachabilityEvaluationCache, evaluate_reachability_with_cache}; +use crate::reachability::{ + ReachabilityEvaluationCache, analyze_condition_expression, evaluate_reachability_with_cache, +}; use crate::types::add_inferred_python_version_hint_to_diagnostic; use crate::types::attribute_write::{AssignmentAttributeMembers, assignment_attribute_members}; use crate::types::call::bind::{ @@ -261,6 +263,10 @@ pub(super) struct TypeInferenceBuilder<'db, 'ast> { /// The types of every expression in this region. expressions: FxHashMap>, + /// Direct-condition truthiness for chained comparisons. Other expressions can be + /// analyzed from their inferred operand types without repeating comparison inference. + comparison_truthiness: FxHashMap, + /// An expression cache shared across builders during multi-inference. expression_cache: Option>>>, @@ -482,6 +488,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { called_functions: FxIndexSet::default(), deferred_state: DeferredExpressionState::None, expressions: FxHashMap::default(), + comparison_truthiness: FxHashMap::default(), expression_cache: None, reachability_cache: OnceCell::new(), qualifiers: FxHashMap::default(), @@ -691,6 +698,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .extend(inference.expressions.iter().copied()); if let Some(extra) = &inference.extra { + self.comparison_truthiness + .extend(extra.comparison_truthiness.iter().copied()); self.context.extend(&extra.diagnostics); self.extend_cycle_recovery(extra.cycle_recovery); self.called_functions @@ -721,6 +730,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { self.expressions .extend(inference.expressions.iter().map(|(key, ty)| (*key, *ty))); + self.comparison_truthiness.extend( + inference + .comparison_truthiness + .iter() + .map(|(key, truthiness)| (*key, *truthiness)), + ); self.context.extend(&inference.diagnostics); self.extend_cycle_recovery(inference.cycle_recovery); self.called_functions @@ -8498,10 +8513,20 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { (body_ty, orelse_ty) }; - match test_ty.try_bool(db, env).unwrap_or_else(|err| { - err.report_diagnostic(&self.context, &**test); - err.fallback_truthiness() - }) { + let test_truthiness = match test_ty.try_bool(db, env) { + Ok(Truthiness::Ambiguous) => analyze_condition_expression(test, &|node| { + self.comparison_truthiness + .get(&node.into()) + .copied() + .unwrap_or_else(|| self.expression_type(node).bool(db, env)) + }), + Ok(truthiness) => truthiness, + Err(err) => { + err.report_diagnostic(&self.context, &**test); + err.fallback_truthiness() + } + }; + match test_truthiness { Truthiness::AlwaysTrue => body_ty, Truthiness::AlwaysFalse => orelse_ty, Truthiness::Ambiguous => UnionType::from_two_elements(db, env, body_ty, orelse_ty), @@ -11043,12 +11068,15 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { (ty, value.range()) }, ) + .0 } /// Computes the output of a chain of (one) boolean operation, consuming as input an iterator /// of operations and calling the `infer_ty` for each to infer their types. /// The iterator is consumed even if the boolean evaluation can be short-circuited, /// in order to ensure the invariant that all expressions are evaluated when inferring types. + /// Returns the value type and the combined truthiness of all but the final operand, which + /// is not converted to a boolean when evaluating the chain as a value. /// /// `infer_ty` receives the unguarded union of previous operand types that may contribute to the /// result. This can be used as a type context without losing generic specialization information @@ -11060,16 +11088,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { track_peer_types: bool, operations: Iterator, needs_peer_type: NeedsPeerType, - infer_ty: InferType, - ) -> Type<'db> + mut infer_ty: InferType, + ) -> (Type<'db>, Truthiness) where Iterator: IntoIterator, NeedsPeerType: Fn(&Item) -> bool, - InferType: Fn(&mut Self, Item, Option>) -> (Type<'db>, TextRange), + InferType: FnMut(&mut Self, Item, Option>) -> (Type<'db>, TextRange), { let db = self.db(); let env = self.program_environment(); let mut done = false; + let mut preceding_truthiness = Truthiness::from(op == ast::BoolOp::And); let mut peer_types: Option> = None; let elements = operations @@ -11094,6 +11123,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { err.report_diagnostic(&self.context, range); err.fallback_truthiness() }); + preceding_truthiness = match op { + ast::BoolOp::And => preceding_truthiness + .negate() + .or(truthiness.negate()) + .negate(), + ast::BoolOp::Or => preceding_truthiness.or(truthiness), + }; if done { return Type::Never; @@ -11128,7 +11164,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } }); - UnionType::from_elements(db, env, elements) + let ty = UnionType::from_elements(db, env, elements); + (ty, preceding_truthiness) } fn infer_compare_expression(&mut self, compare: &ast::ExprCompare) -> Type<'db> { @@ -11142,6 +11179,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } = compare; self.infer_expression(left, TypeContext::default()); + let mut last_comparison_ty = Type::unknown(); // https://docs.python.org/3/reference/expressions.html#comparisons // > Formally, if `a, b, c, …, y, z` are expressions and `op1, op2, …, opN` are comparison @@ -11150,7 +11188,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // // As some operators (==, !=, <, <=, >, >=) *can* return an arbitrary type, the logic below // is shared with the one in `infer_binary_type_comparison`. - self.infer_chained_boolean_types( + let (ty, preceding_truthiness) = self.infer_chained_boolean_types( ast::BoolOp::And, false, std::iter::once(&**left) @@ -11192,9 +11230,25 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } }); + last_comparison_ty = ty; (ty, range) }, - ) + ); + + if ops.len() > 1 { + let truthiness = preceding_truthiness + .negate() + .or_else(|| { + last_comparison_ty + .bool(db, self.program_environment()) + .negate() + }) + .negate(); + self.comparison_truthiness + .insert(ast::ExprRef::Compare(compare).into(), truthiness); + } + + ty } fn infer_type_parameters(&mut self, type_parameters: &ast::TypeParams) { @@ -11229,6 +11283,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let Self { context, expressions, + comparison_truthiness, qualifiers: _, type_expression_flags, collection_use_constraints, @@ -11270,6 +11325,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { FullExpressionCacheEntry { expressions, + comparison_truthiness, type_expression_flags, collection_use_constraints, string_annotations, @@ -11289,6 +11345,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let Self { context, expressions, + comparison_truthiness: _, qualifiers, type_expression_flags, mut collection_use_constraints, @@ -11402,6 +11459,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let Self { context, expressions, + comparison_truthiness: _, bindings, called_functions, expression_cache: _, @@ -11450,6 +11508,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let Self { context, expressions, + comparison_truthiness: _, qualifiers, type_expression_flags, mut collection_use_constraints, @@ -11591,6 +11650,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { type_expression_flags, mut collection_use_constraints, expressions, + comparison_truthiness: _, scope, cycle_recovery, qualifiers, @@ -11672,6 +11732,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { context: _, collection_use_constraints: _, expressions: _, + comparison_truthiness: _, string_annotations: _, expected_types: _, scope: _, @@ -11731,6 +11792,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let Self { context, expressions, + comparison_truthiness, type_expression_flags, collection_use_constraints, string_annotations, @@ -11771,6 +11833,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ); self.expressions.extend(expressions.iter()); + self.comparison_truthiness.extend(comparison_truthiness); self.context.extend(&diagnostics); self.extend_cycle_recovery(cycle_recovery); self.string_annotations @@ -11883,6 +11946,7 @@ enum ExpressionCacheEntry<'db> { /// that is otherwise performed for Salsa results. struct FullExpressionCacheEntry<'db> { expressions: FxHashMap>, + comparison_truthiness: FxHashMap, type_expression_flags: FxHashMap, collection_use_constraints: CollectionUseConstraints<'db>, string_annotations: FxHashSet, @@ -11907,6 +11971,7 @@ impl<'db> FullExpressionCacheEntry<'db> { fn is_single_expression(&self, expression: ExpressionNodeKey, ty: Type<'db>) -> bool { self.expressions.len() == 1 && self.expressions.get(&expression) == Some(&ty) + && self.comparison_truthiness.is_empty() && self.type_expression_flags.is_empty() && self.collection_use_constraints.is_empty() && self.string_annotations.is_empty() @@ -11922,6 +11987,7 @@ impl<'db> FullExpressionCacheEntry<'db> { region: InferenceRegion<'db>, ) -> ExpressionInference<'db> { let extra = (!self.string_annotations.is_empty() + || !self.comparison_truthiness.is_empty() || !self.type_expression_flags.is_empty() || !self.collection_use_constraints.is_empty() || !self.expected_types.is_empty() @@ -11943,6 +12009,7 @@ impl<'db> FullExpressionCacheEntry<'db> { self.diagnostics.shrink_to_fit(); Box::new(ExpressionInferenceExtra { string_annotations: FrozenSet::from(self.string_annotations), + comparison_truthiness: FrozenMap::from(self.comparison_truthiness), expected_types: FrozenMap::from(self.expected_types), type_expression_flags: FrozenMap::from(self.type_expression_flags), bindings: self.bindings.into_boxed_slice(), diff --git a/crates/ty_python_semantic/src/types/infer/comparisons.rs b/crates/ty_python_semantic/src/types/infer/comparisons.rs index 573029e8caa674..aa04ef244fdf79 100644 --- a/crates/ty_python_semantic/src/types/infer/comparisons.rs +++ b/crates/ty_python_semantic/src/types/infer/comparisons.rs @@ -264,6 +264,27 @@ enum NonIdentityOperator { Membership(MembershipOperator), } +impl NonIdentityOperator { + fn truthiness<'db>( + self, + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + left: Type<'db>, + right: Type<'db>, + soundness_policy: ComparisonSoundnessPolicy, + ) -> Truthiness { + match self { + Self::Rich(RichCompareOperator::Eq) => { + equality_truthiness(db, env, left, right, soundness_policy) + } + Self::Rich(RichCompareOperator::Ne) => { + inequality_truthiness(db, env, left, right, soundness_policy) + } + _ => Truthiness::Ambiguous, + } + } +} + impl From for ast::CmpOp { fn from(value: NonIdentityOperator) -> Self { match value { @@ -402,17 +423,13 @@ fn infer_binary_type_comparison_inner<'db>( }); } - let comparison_truthiness = match op { - NonIdentityOperator::Rich(RichCompareOperator::Eq) => { - equality_truthiness(db, env, left, right, soundness_policy) + // An intersection can supply a custom comparison even when another component inherits + // `object.__eq__`. Check its return type before using boolean-only simplifications. + if !left.is_intersection() && !right.is_intersection() { + let comparison_truthiness = op.truthiness(db, env, left, right, soundness_policy); + if comparison_truthiness != Truthiness::Ambiguous { + return Ok(Type::from_truthiness(db, env, comparison_truthiness)); } - NonIdentityOperator::Rich(RichCompareOperator::Ne) => { - inequality_truthiness(db, env, left, right, soundness_policy) - } - _ => Truthiness::Ambiguous, - }; - if comparison_truthiness != Truthiness::Ambiguous { - return Ok(Type::from_truthiness(db, env, comparison_truthiness)); } let comparison_result = match (left, right) { @@ -880,6 +897,33 @@ fn infer_binary_intersection_type_comparison<'db>( }; } + let (left, right) = match intersection_on { + IntersectionOn::Left => (Type::Intersection(intersection), other), + IntersectionOn::Right => (other, Type::Intersection(intersection)), + }; + // Rich comparisons can return arbitrary objects. Use the full receiver to bind `Self`. + // A failed call can still succeed via component-specific inference below, such as the + // concrete-base fallback for NewTypes of `float`. + let rich_result = if let NonIdentityOperator::Rich(rich_op) = op + && let Ok(result) = + infer_rich_comparison(context, left, right, rich_op, MemberLookupPolicy::default()) + { + // Gradual results such as `bool & Any` still permit boolean simplifications. + // Preserve narrower results, including boolean literals and `Never`. + if result.top_materialization(db, env) != KnownClass::Bool.to_instance(db, env) { + return Ok(result); + } + let soundness_policy = + ComparisonSoundnessPolicy::from_analysis_settings(db.analysis_settings(context.file())); + let comparison_truthiness = op.truthiness(db, env, left, right, soundness_policy); + if comparison_truthiness != Truthiness::Ambiguous { + return Ok(Type::from_truthiness(db, env, comparison_truthiness)); + } + Some(result) + } else { + None + }; + // If a comparison yields a definitive true/false answer on a (positive) part // of an intersection type, it will also yield a definitive answer on the full // intersection type, which is even more specific. @@ -902,6 +946,10 @@ fn infer_binary_intersection_type_comparison<'db>( } } + if let Some(result) = rich_result { + return Ok(result); + } + // If none of the simplifications above apply, we still need to return *some* // result type for the comparison 'T_inter `op` T_other' (or reversed), where // @@ -942,8 +990,6 @@ fn infer_binary_intersection_type_comparison<'db>( // let mut builder = IntersectionBuilder::new(db, env); - builder.add_positive_in_place(KnownClass::Bool.to_instance(db, env)); - let mut state = State::NoPositiveElements; for pos in intersection.positive(db) { diff --git a/crates/ty_python_semantic/src/types/narrow.rs b/crates/ty_python_semantic/src/types/narrow.rs index 1373731a0b1f7f..077f23e0a58269 100644 --- a/crates/ty_python_semantic/src/types/narrow.rs +++ b/crates/ty_python_semantic/src/types/narrow.rs @@ -78,7 +78,7 @@ pub(crate) fn infer_narrowing_constraints<'db>( Option>, ) { let constraints = match predicate.node { - PredicateNode::Expression(expression) => { + PredicateNode::Expression(expression) | PredicateNode::Condition(expression) => { let constraints = all_narrowing_constraints_for_expression(db, expression); ( constraints.get(place, true).cloned(), @@ -1505,7 +1505,7 @@ impl<'db, 'ast> NarrowingConstraintsBuilder<'db, 'ast> { fn finish(mut self) -> Option> { let constraints: Option> = match self.predicate { - PredicateNode::Expression(expression) => { + PredicateNode::Expression(expression) | PredicateNode::Condition(expression) => { self.evaluate_expression_predicate(expression, self.is_positive) } PredicateNode::Pattern(pattern) => { @@ -3209,6 +3209,7 @@ impl<'db> NarrowingConstraintsBuilder<'db, '_> { let db = self.db; match self.predicate { PredicateNode::Expression(expression) + | PredicateNode::Condition(expression) | PredicateNode::ContextManagerSuppresses { expression, .. } => expression.scope(db), PredicateNode::Pattern(pattern) => pattern.scope(db), PredicateNode::FinallyNormalPathImpossible { scope, .. } => scope,