From 8f14bcf34c6aaa2d0b2711a6cfffab5bf8dd71ac Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:30:38 +0530 Subject: [PATCH] Report unreachable match cases after earlier negations Narrow by the current case pattern before applying earlier unguarded case negations. This lets an Any subject become its current class first, so subtracting an earlier superclass produces Never and the case is reported unreachable. Keep object subjects on the existing early return because structural patterns can narrow object to Never spuriously. The check is limited to the Any case tracked by #3338. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- pyrefly/lib/alt/narrow.rs | 45 +++++++++++++--------- pyrefly/lib/alt/solve.rs | 4 +- pyrefly/lib/binding/binding.rs | 7 +++- pyrefly/lib/binding/pattern.rs | 25 +++++++----- pyrefly/lib/test/pattern_match.rs | 63 +++++++++++++++++++++++++++++++ 5 files changed, 116 insertions(+), 28 deletions(-) diff --git a/pyrefly/lib/alt/narrow.rs b/pyrefly/lib/alt/narrow.rs index 2c5f958e33..3dc7e513a7 100644 --- a/pyrefly/lib/alt/narrow.rs +++ b/pyrefly/lib/alt/narrow.rs @@ -2703,7 +2703,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { &self, subject_idx: &Idx, narrowing_subject: Option<&NarrowingSubject>, - narrow_ops_for_case: &(Box, TextRange), + narrow_ops_for_case: &(NarrowOp, TextRange), + prev_case_negations: Option<&(NarrowOp, TextRange)>, case_range: &TextRange, errors: &ErrorCollector, ) { @@ -2713,17 +2714,21 @@ 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() { + // #3338 is limited to `Any`. Keep the existing `object` behavior: + // structural patterns such as tuples can narrow `object` to `Never` + // spuriously. + if subject_ty.is_object() { return; } let ignore_errors = self.error_swallower(); - let (mut narrowed_ty, has_never_trigger_facet) = match narrowing_subject { + let mut has_never_trigger_facet = false; + let mut resolved_facet_chain = None; + let mut narrowed_info = match narrowing_subject { None | Some(NarrowingSubject::Name(_)) => { - let narrowed = - self.narrow(&subject_info, op.as_ref(), *narrow_range, &ignore_errors); - let has_never_trigger_facet = + let narrowed = self.narrow(&subject_info, op, *narrow_range, &ignore_errors); + has_never_trigger_facet = self.has_never_match_trigger_facet(&narrowed, op, *case_range); - (narrowed.ty().clone(), has_never_trigger_facet) + narrowed } Some(NarrowingSubject::Facets(_, facets)) => { let Some(resolved_chain) = self.resolve_facet_chain(facets.chain.clone()) else { @@ -2732,19 +2737,25 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let type_info = TypeInfo::of_ty(self.heap.mk_any_implicit()); let narrowing_subject_info = type_info.with_narrow(resolved_chain.facets(), subject_ty.clone()); - let narrowed = self.narrow( - &narrowing_subject_info, - op.as_ref(), - *narrow_range, - &ignore_errors, - ); + let narrowed = + self.narrow(&narrowing_subject_info, op, *narrow_range, &ignore_errors); + resolved_facet_chain = Some(resolved_chain); + narrowed + } + }; + // Narrow by the current pattern first, then apply the earlier cases' + // negations. On an `Any` subject this lets `bool` become `bool` before + // subtracting an earlier `int`, which yields `Never`. + if let Some((prev_op, prev_range)) = prev_case_negations { + narrowed_info = self.narrow(&narrowed_info, prev_op, *prev_range, &ignore_errors); + } + let mut narrowed_ty = match resolved_facet_chain { + Some(resolved_chain) => { // No sub-facet check needed: the narrowed type already represents // the facet's type directly, so `is_never()` below is sufficient. - ( - self.get_facet_chain_type(&narrowed, &resolved_chain, *case_range), - false, - ) + self.get_facet_chain_type(&narrowed_info, &resolved_chain, *case_range) } + None => narrowed_info.ty().clone(), }; self.expand_mut(&mut narrowed_ty); if !narrowed_ty.is_never() && !has_never_trigger_facet { diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 13ba551a25..ddff472bf1 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3166,11 +3166,13 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { subject_idx, narrowing_subject, narrow_ops_for_case, + prev_case_negations, case_range, } => self.check_match_case_reachability( subject_idx, narrowing_subject.as_ref(), - narrow_ops_for_case, + narrow_ops_for_case.as_ref(), + prev_case_negations.as_deref(), case_range, errors, ), diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index d183ea4fd6..37a1a1ae2c 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1325,7 +1325,12 @@ pub enum BindingExpect { MatchCaseReachability { subject_idx: Idx, narrowing_subject: Option, - narrow_ops_for_case: (Box, TextRange), + narrow_ops_for_case: Box<(NarrowOp, TextRange)>, + /// Negations from earlier unguarded cases in the same match, in the + /// same subject coordinates as `narrow_ops_for_case`. Reachability + /// narrows by the current pattern first, then applies these, so an + /// `Any` subject can first become `bool` and then subtract `int`. + prev_case_negations: Option>, 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..915535be38 100644 --- a/pyrefly/lib/binding/pattern.rs +++ b/pyrefly/lib/binding/pattern.rs @@ -935,35 +935,41 @@ impl<'a> BindingsBuilder<'a> { // match subject. This is required for cases like `match self.a`, where the // carried narrow is stored as a facet on `self` but the branch-local subject // is already the projected `self.a` expression. - let case_subject_idx = if let Some(narrowing_subject) = match_subject.as_single() + let (case_subject_idx, prev_case_negations) = if let Some(narrowing_subject) = + match_subject.as_single() && let Some((narrow_op, op_range)) = negated_prev_ops.0.get(narrowing_subject.name()) && let Some(projected_narrow_op) = narrow_op.rebase_onto_subject(narrowing_subject) { - self.insert_binding( + let prev_case_negations = Some(Box::new((projected_narrow_op.clone(), *op_range))); + let case_subject_idx = self.insert_binding( Key::PatternNarrow(case_range), Binding::Narrow( subject_idx, Box::new(projected_narrow_op), NarrowUseLocation::Start(*op_range), ), - ) + ); + (case_subject_idx, prev_case_negations) } else if let Some((narrow_op, op_range)) = &negated_prev_subject { - self.insert_binding( + let prev_case_negations = Some(Box::new((narrow_op.clone(), *op_range))); + let case_subject_idx = self.insert_binding( Key::PatternNarrow(case_range), Binding::Narrow( subject_idx, Box::new(narrow_op.clone()), NarrowUseLocation::Start(*op_range), ), - ) + ); + (case_subject_idx, prev_case_negations) } else if match_subject.as_single().is_some() && !negated_prev_ops.0.is_empty() { - self.insert_binding( + let case_subject_idx = self.insert_binding( Key::PatternNarrow(case_range), Binding::Expr(None, Box::new(*subject_expr.clone())), - ) + ); + (case_subject_idx, None) } else { - subject_idx + (subject_idx, None) }; let mut new_narrow_ops = self.bind_pattern(match_subject.clone(), pattern, case_subject_idx); @@ -1000,7 +1006,8 @@ 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, range)), + prev_case_negations, case_range, }, ); diff --git a/pyrefly/lib/test/pattern_match.rs b/pyrefly/lib/test/pattern_match.rs index cc8e732b79..21cb36ca32 100644 --- a/pyrefly/lib/test/pattern_match.rs +++ b/pyrefly/lib/test/pattern_match.rs @@ -197,6 +197,69 @@ def no_cascade_after_wildcard(x: int) -> None: "#, ); +testcase!( + test_match_case_unreachable_subsumed_by_sibling_pattern, + 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 subject of type + 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 + +class A: + x: int + +class B(A): + pass + +def ok_object_subclass_order(x: object) -> None: + match x: + case int(): + pass + case bool(): + pass + +# A partial class pattern does not subsume its subclass pattern. +def ok_partial_class_predecessor_any(x: Any) -> None: + match x: + case A(x=1): + pass + case B(): + pass + +def duplicate_literal_any(x: Any) -> None: + match x: + case 1: + pass + case 1: # E: Case pattern can never match subject of type + pass + +def or_pattern_any(x: Any) -> None: + match x: + case int() | str(): + pass + case bool(): # E: Case pattern can never match subject of type + pass +"#, +); + testcase!( test_pattern_dict_key_enum, r#"