diff --git a/pyrefly/lib/alt/narrow.rs b/pyrefly/lib/alt/narrow.rs index 7a5879ccd7..0d427a375a 100644 --- a/pyrefly/lib/alt/narrow.rs +++ b/pyrefly/lib/alt/narrow.rs @@ -23,6 +23,7 @@ use pyrefly_types::simplify::simplify_tuples; use pyrefly_types::type_alias::TypeAliasData; use pyrefly_types::type_info::JoinStyle; use pyrefly_types::typed_dict::ExtraItems; +use pyrefly_types::types::TArgs; use pyrefly_util::prelude::SliceExt; use pyrefly_util::visit::Visit; use ruff_python_ast::Arguments; @@ -2684,6 +2685,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { subject_idx: &Idx, narrowing_subject: Option<&NarrowingSubject>, narrow_ops_for_case: &(Box, TextRange), + prev_case_ops: &[(Box, TextRange)], case_range: &TextRange, errors: &ErrorCollector, ) { @@ -2694,6 +2696,15 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let subject_info = self.with_type_for_exhaustiveness_check(self.get_idx(*subject_idx)); let subject_ty = subject_info.ty().clone(); if subject_ty.is_any() || subject_ty.is_object() { + // Narrowing an `Any`/`object` subject never yields `Never`, so the + // sibling check is the only way to detect a case that an earlier + // unguarded sibling already matches. + self.check_match_case_sibling_subsumption( + narrow_ops_for_case, + prev_case_ops, + *case_range, + errors, + ); return; } let ignore_errors = self.error_swallower(); @@ -2727,17 +2738,90 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } }; self.expand_mut(&mut narrowed_ty); - if !narrowed_ty.is_never() && !has_never_trigger_facet { + if narrowed_ty.is_never() || has_never_trigger_facet { + let subject_display = self.for_display(subject_ty); + self.error( + errors, + *case_range, + ErrorKind::UnreachableMatchCase, + format!("Case pattern can never match subject of type `{subject_display}`"), + ); return; } - let subject_display = self.for_display(subject_ty); - self.error( - errors, + // The subject-based narrowing found the case reachable, but a precise + // subject can still leave sibling patterns that an earlier unguarded + // case fully subsumes. + self.check_match_case_sibling_subsumption( + narrow_ops_for_case, + prev_case_ops, *case_range, - ErrorKind::UnreachableMatchCase, - format!("Case pattern can never match subject of type `{subject_display}`"), + errors, ); } + /// Extract the matched class from a plain class pattern, if it is one. + /// + /// Only single atomic `isinstance` patterns qualify; anything else keeps + /// the existing behavior. Returns `None` unless exactly one class is + /// found, so ambiguous patterns never produce a verdict. + fn pattern_class_operand(&self, op: &NarrowOp, errors: &ErrorCollector) -> Option { + if let NarrowOp::Atomic(_, AtomicNarrowOp::IsInstance(v, NarrowSource::Pattern)) = op { + let mut classes = self.as_class_info(self.expr_infer(v, errors)); + if classes.len() == 1 { + let class_ty = classes.pop().expect("checked length above"); + if let Type::ClassDef(cls) = class_ty + && self.get_class_tparams(&cls).is_none() + && !self.get_metadata_for_class(&cls).has_base_any() + { + return Some( + self.heap + .mk_class_type(ClassType::new(cls, TArgs::default())), + ); + } + } + } + None + } + + /// Report when a class pattern is subsumed by an earlier sibling case. + /// + /// The subject-based check sees one case at a time, so it stays silent + /// when the subject is `Any`: narrowing `Any` never yields `Never`. But + /// `case bool():` after `case int():` can never match no matter the + /// subject, because every `bool` already matched above. Only plain atomic + /// class patterns on both sides participate; guarded predecessors were + /// never recorded, and `Or`/sequence patterns keep the existing behavior. + fn check_match_case_sibling_subsumption( + &self, + narrow_ops_for_case: &(Box, TextRange), + prev_case_ops: &[(Box, TextRange)], + case_range: TextRange, + errors: &ErrorCollector, + ) { + let ignore_errors = self.error_swallower(); + let Some(current) = self.pattern_class_operand(&narrow_ops_for_case.0, &ignore_errors) + else { + return; + }; + let previous: Vec = prev_case_ops + .iter() + .filter_map(|(prev_op, _)| self.pattern_class_operand(prev_op, &ignore_errors)) + .collect(); + if previous.is_empty() { + return; + } + let matched = self.unions(previous); + if self.is_subset_eq(¤t, &matched) { + let current_display = self.for_display(current); + self.error( + errors, + case_range, + ErrorKind::UnreachableMatchCase, + format!( + "Case pattern can never match: an earlier case already matches all `{current_display}` values" + ), + ); + } + } /// Check whether we can reliably determine reachability for this op. /// diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index d3b5c857f0..dbe0dc2c66 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3105,11 +3105,13 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { subject_idx, narrowing_subject, narrow_ops_for_case, + prev_case_ops, case_range, } => self.check_match_case_reachability( subject_idx, narrowing_subject.as_ref(), narrow_ops_for_case, + prev_case_ops, case_range, errors, ), diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 33c1e032cb..446794fb2f 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -128,7 +128,7 @@ assert_words!(KeyDecoratedFunction, 1); assert_words!(KeyUndecoratedFunction, 1); assert_words!(Binding, 4); -assert_words!(BindingExpect, 12); +assert_words!(BindingExpect, 13); assert_words!(BindingTypeAlias, 6); assert_words!(BindingAnnotation, 11); assert_words!(BindingClass, 11); @@ -1318,6 +1318,11 @@ pub enum BindingExpect { subject_idx: Idx, narrowing_subject: Option, narrow_ops_for_case: (Box, TextRange), + /// Narrow ops of preceding unguarded cases in the same match, in + /// source order. Lets the reachability check detect sibling + /// subsumption (a class already matched above), which subject + /// narrowing alone cannot see for `Any` subjects. + prev_case_ops: Vec<(Box, TextRange)>, case_range: TextRange, }, /// Code following one or more points in a suite that control passes only conditionally, such diff --git a/pyrefly/lib/binding/pattern.rs b/pyrefly/lib/binding/pattern.rs index c83568ee42..65640817e4 100644 --- a/pyrefly/lib/binding/pattern.rs +++ b/pyrefly/lib/binding/pattern.rs @@ -912,6 +912,12 @@ impl<'a> BindingsBuilder<'a> { // is carried over to the fallback case. let mut negated_prev_ops = NarrowOps::new(); let mut negated_prev_subject: Option<(NarrowOp, TextRange)> = None; + // Narrow ops of preceding unguarded cases, in source order, for the + // sibling-subsumption part of the reachability check. Guarded cases + // may fail to match, so only unguarded ones are recorded, and each + // case records after its own binding is built so it never sees + // itself. + let mut prev_case_ops: Vec<(Box, TextRange)> = Vec::new(); for case in x.cases { let MatchCase { pattern, @@ -1000,10 +1006,14 @@ impl<'a> BindingsBuilder<'a> { BindingExpect::MatchCaseReachability { subject_idx: case_subject_idx, narrowing_subject, - narrow_ops_for_case: (Box::new(op), range), + narrow_ops_for_case: (Box::new(op.clone()), range), + prev_case_ops: std::mem::take(&mut prev_case_ops), case_range, }, ); + if guard.is_none() { + prev_case_ops.push((Box::new(op), range)); + } } let has_guard = guard.is_some(); if let Some(mut guard) = guard { diff --git a/pyrefly/lib/test/pattern_match.rs b/pyrefly/lib/test/pattern_match.rs index ff4f5a2433..df457e22a1 100644 --- a/pyrefly/lib/test/pattern_match.rs +++ b/pyrefly/lib/test/pattern_match.rs @@ -197,6 +197,34 @@ def no_cascade_after_wildcard(x: int) -> None: "#, ); +testcase!( + test_match_case_unreachable_subsumed_by_sibling_any_subject, + r#" +from typing import Any + +def shadowed_after_parent_any(x: Any) -> None: + match x: + case int(): + pass + case bool(): # E: Case pattern can never match: an earlier case already matches all `bool` values + pass + +def ok_subclass_first_any(x: Any) -> None: + match x: + case bool(): + pass + case int(): + pass + +def ok_guarded_predecessor_any(x: Any, flag: bool) -> None: + match x: + case int() if flag: + pass + case bool(): + pass +"#, +); + testcase!( test_pattern_dict_key_enum, r#"