diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 9e9a9d04751..c482cf6190a 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -972,6 +972,47 @@ fn test_fn() { check_no_diagnostic(content); } + #[test] + fn tuple_of_bools_with_ellipsis_at_end_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (false, ..) => {}, + (true, ..) => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_at_beginning_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (.., false) => {}, + (.., true) => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (..) => {}, + } + } + "; + + check_no_diagnostic(content); + } + #[test] fn tuple_of_tuple_and_bools_no_arms() { let content = r" @@ -1315,8 +1356,9 @@ fn test_fn() { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] @@ -1330,8 +1372,9 @@ fn test_fn() { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] @@ -1344,8 +1387,9 @@ fn test_fn() { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] @@ -1383,6 +1427,102 @@ fn test_fn() { // we don't create a diagnostic). check_no_diagnostic(content); } + + #[test] + fn expr_diverges() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop {} { + Either::A => (), + Either::B => (), + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn expr_loop_with_break() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop { break Foo::A } { + Either::A => (), + Either::B => (), + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn expr_partially_diverges() { + let content = r" + enum Either { + A(T), + B, + } + fn foo() -> Either { + Either::B + } + fn test_fn() -> u32 { + match foo() { + Either::A(val) => val, + Either::B => 0, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., true) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_ellipsis_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(..) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } } #[cfg(test)] @@ -1452,4 +1592,104 @@ enum Either { // We do not currently handle patterns with internal `or`s. check_no_diagnostic(content); } + + #[test] + fn expr_diverges_missing_arm() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop {} { + Either::A => (), + } + } + "; + + // This is a false negative. + // Even though the match expression diverges, rustc fails + // to compile here since `Either::B` is missing. + check_no_diagnostic(content); + } + + #[test] + fn expr_loop_missing_arm() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop { break Foo::A } { + Either::A => (), + } + } + "; + + // This is a false negative. + // We currently infer the type of `loop { break Foo::A }` to `!`, which + // causes us to skip the diagnostic since `Either::A` doesn't type check + // with `!`. + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_at_end_missing_arm() { + let content = r" + fn test_fn() { + match (false, true, false) { + (false, ..) => {}, + } + } + "; + + // This is a false negative. + // The `..` pattern is currently lowered to a single `Pat::Wild` + // no matter how many fields the `..` pattern is covering. This + // causes the match arm in this test not to type check against + // the match expression, which causes this diagnostic not to + // fire. + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() { + let content = r" + fn test_fn() { + match (false, true, false) { + (.., false) => {}, + } + } + "; + + // This is a false negative. + // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + // This is a false negative. + // The `..` pattern is currently lowered to a single `Pat::Wild` + // no matter how many fields the `..` pattern is covering. This + // causes us to return a `MatchCheckErr::MalformedMatchArm` in + // `Pat::specialize_constructor`. + check_no_diagnostic(content); + } } diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 69b527f7449..21abbcf1e2d 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -161,12 +161,6 @@ fn validate_match( let mut seen = Matrix::empty(); for pat in pats { - // We skip any patterns whose type we cannot resolve. - // - // This could lead to false positives in this diagnostic, so - // it might be better to skip the entire diagnostic if we either - // cannot resolve a match arm or determine that the match arm has - // the wrong type. if let Some(pat_ty) = infer.type_of_pat.get(pat) { // We only include patterns whose type matches the type // of the match expression. If we had a InvalidMatchArmPattern @@ -189,8 +183,15 @@ fn validate_match( // to the matrix here. let v = PatStack::from_pattern(pat); seen.push(&cx, v); + continue; } } + + // If we can't resolve the type of a pattern, or the pattern type doesn't + // fit the match expression, we skip this diagnostic. Skipping the entire + // diagnostic rather than just not including this match arm is preferred + // to avoid the chance of false positives. + return; } match is_useful(&cx, &seen, &PatStack::from_wild()) {