diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-04-12 09:30:24 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-04-12 09:30:24 +0100 |
commit | 268b7987290143550461090c2c0e23371813dbec (patch) | |
tree | 5dcdab30670131d405d34d0c20a8785bbf1ee181 /crates/ra_hir_ty/src | |
parent | a8e032820f14cad3630299a1ae16c62dcf59f358 (diff) | |
parent | bb2e5308b77e5d115df17411aaa2dd26be171b0a (diff) |
Merge #3938
3938: fix missing match arm false positive r=flodiebold a=JoshMcguigan
This fixes #3932 by skipping the missing match arm diagnostic in the case any of the match arms don't type check properly against the match expression.
I think this is the appropriate behavior for this diagnostic, since `is_useful` relies on all match arms being well formed, and the case of a malformed match arm should probably be handled by a different diagnostic.
Co-authored-by: Josh Mcguigan <[email protected]>
Diffstat (limited to 'crates/ra_hir_ty/src')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 252 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/expr.rs | 13 |
2 files changed, 253 insertions, 12 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 9e9a9d047..c482cf619 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs | |||
@@ -973,6 +973,47 @@ mod tests { | |||
973 | } | 973 | } |
974 | 974 | ||
975 | #[test] | 975 | #[test] |
976 | fn tuple_of_bools_with_ellipsis_at_end_no_diagnostic() { | ||
977 | let content = r" | ||
978 | fn test_fn() { | ||
979 | match (false, true, false) { | ||
980 | (false, ..) => {}, | ||
981 | (true, ..) => {}, | ||
982 | } | ||
983 | } | ||
984 | "; | ||
985 | |||
986 | check_no_diagnostic(content); | ||
987 | } | ||
988 | |||
989 | #[test] | ||
990 | fn tuple_of_bools_with_ellipsis_at_beginning_no_diagnostic() { | ||
991 | let content = r" | ||
992 | fn test_fn() { | ||
993 | match (false, true, false) { | ||
994 | (.., false) => {}, | ||
995 | (.., true) => {}, | ||
996 | } | ||
997 | } | ||
998 | "; | ||
999 | |||
1000 | check_no_diagnostic(content); | ||
1001 | } | ||
1002 | |||
1003 | #[test] | ||
1004 | fn tuple_of_bools_with_ellipsis_no_diagnostic() { | ||
1005 | let content = r" | ||
1006 | fn test_fn() { | ||
1007 | match (false, true, false) { | ||
1008 | (..) => {}, | ||
1009 | } | ||
1010 | } | ||
1011 | "; | ||
1012 | |||
1013 | check_no_diagnostic(content); | ||
1014 | } | ||
1015 | |||
1016 | #[test] | ||
976 | fn tuple_of_tuple_and_bools_no_arms() { | 1017 | fn tuple_of_tuple_and_bools_no_arms() { |
977 | let content = r" | 1018 | let content = r" |
978 | fn test_fn() { | 1019 | fn test_fn() { |
@@ -1315,8 +1356,9 @@ mod tests { | |||
1315 | } | 1356 | } |
1316 | "; | 1357 | "; |
1317 | 1358 | ||
1318 | // Match arms with the incorrect type are filtered out. | 1359 | // Match statements with arms that don't match the |
1319 | check_diagnostic(content); | 1360 | // expression pattern do not fire this diagnostic. |
1361 | check_no_diagnostic(content); | ||
1320 | } | 1362 | } |
1321 | 1363 | ||
1322 | #[test] | 1364 | #[test] |
@@ -1330,8 +1372,9 @@ mod tests { | |||
1330 | } | 1372 | } |
1331 | "; | 1373 | "; |
1332 | 1374 | ||
1333 | // Match arms with the incorrect type are filtered out. | 1375 | // Match statements with arms that don't match the |
1334 | check_diagnostic(content); | 1376 | // expression pattern do not fire this diagnostic. |
1377 | check_no_diagnostic(content); | ||
1335 | } | 1378 | } |
1336 | 1379 | ||
1337 | #[test] | 1380 | #[test] |
@@ -1344,8 +1387,9 @@ mod tests { | |||
1344 | } | 1387 | } |
1345 | "; | 1388 | "; |
1346 | 1389 | ||
1347 | // Match arms with the incorrect type are filtered out. | 1390 | // Match statements with arms that don't match the |
1348 | check_diagnostic(content); | 1391 | // expression pattern do not fire this diagnostic. |
1392 | check_no_diagnostic(content); | ||
1349 | } | 1393 | } |
1350 | 1394 | ||
1351 | #[test] | 1395 | #[test] |
@@ -1383,6 +1427,102 @@ mod tests { | |||
1383 | // we don't create a diagnostic). | 1427 | // we don't create a diagnostic). |
1384 | check_no_diagnostic(content); | 1428 | check_no_diagnostic(content); |
1385 | } | 1429 | } |
1430 | |||
1431 | #[test] | ||
1432 | fn expr_diverges() { | ||
1433 | let content = r" | ||
1434 | enum Either { | ||
1435 | A, | ||
1436 | B, | ||
1437 | } | ||
1438 | fn test_fn() { | ||
1439 | match loop {} { | ||
1440 | Either::A => (), | ||
1441 | Either::B => (), | ||
1442 | } | ||
1443 | } | ||
1444 | "; | ||
1445 | |||
1446 | check_no_diagnostic(content); | ||
1447 | } | ||
1448 | |||
1449 | #[test] | ||
1450 | fn expr_loop_with_break() { | ||
1451 | let content = r" | ||
1452 | enum Either { | ||
1453 | A, | ||
1454 | B, | ||
1455 | } | ||
1456 | fn test_fn() { | ||
1457 | match loop { break Foo::A } { | ||
1458 | Either::A => (), | ||
1459 | Either::B => (), | ||
1460 | } | ||
1461 | } | ||
1462 | "; | ||
1463 | |||
1464 | check_no_diagnostic(content); | ||
1465 | } | ||
1466 | |||
1467 | #[test] | ||
1468 | fn expr_partially_diverges() { | ||
1469 | let content = r" | ||
1470 | enum Either<T> { | ||
1471 | A(T), | ||
1472 | B, | ||
1473 | } | ||
1474 | fn foo() -> Either<!> { | ||
1475 | Either::B | ||
1476 | } | ||
1477 | fn test_fn() -> u32 { | ||
1478 | match foo() { | ||
1479 | Either::A(val) => val, | ||
1480 | Either::B => 0, | ||
1481 | } | ||
1482 | } | ||
1483 | "; | ||
1484 | |||
1485 | check_no_diagnostic(content); | ||
1486 | } | ||
1487 | |||
1488 | #[test] | ||
1489 | fn enum_tuple_partial_ellipsis_no_diagnostic() { | ||
1490 | let content = r" | ||
1491 | enum Either { | ||
1492 | A(bool, bool, bool, bool), | ||
1493 | B, | ||
1494 | } | ||
1495 | fn test_fn() { | ||
1496 | match Either::B { | ||
1497 | Either::A(true, .., true) => {}, | ||
1498 | Either::A(true, .., false) => {}, | ||
1499 | Either::A(false, .., true) => {}, | ||
1500 | Either::A(false, .., false) => {}, | ||
1501 | Either::B => {}, | ||
1502 | } | ||
1503 | } | ||
1504 | "; | ||
1505 | |||
1506 | check_no_diagnostic(content); | ||
1507 | } | ||
1508 | |||
1509 | #[test] | ||
1510 | fn enum_tuple_ellipsis_no_diagnostic() { | ||
1511 | let content = r" | ||
1512 | enum Either { | ||
1513 | A(bool, bool, bool, bool), | ||
1514 | B, | ||
1515 | } | ||
1516 | fn test_fn() { | ||
1517 | match Either::B { | ||
1518 | Either::A(..) => {}, | ||
1519 | Either::B => {}, | ||
1520 | } | ||
1521 | } | ||
1522 | "; | ||
1523 | |||
1524 | check_no_diagnostic(content); | ||
1525 | } | ||
1386 | } | 1526 | } |
1387 | 1527 | ||
1388 | #[cfg(test)] | 1528 | #[cfg(test)] |
@@ -1452,4 +1592,104 @@ mod false_negatives { | |||
1452 | // We do not currently handle patterns with internal `or`s. | 1592 | // We do not currently handle patterns with internal `or`s. |
1453 | check_no_diagnostic(content); | 1593 | check_no_diagnostic(content); |
1454 | } | 1594 | } |
1595 | |||
1596 | #[test] | ||
1597 | fn expr_diverges_missing_arm() { | ||
1598 | let content = r" | ||
1599 | enum Either { | ||
1600 | A, | ||
1601 | B, | ||
1602 | } | ||
1603 | fn test_fn() { | ||
1604 | match loop {} { | ||
1605 | Either::A => (), | ||
1606 | } | ||
1607 | } | ||
1608 | "; | ||
1609 | |||
1610 | // This is a false negative. | ||
1611 | // Even though the match expression diverges, rustc fails | ||
1612 | // to compile here since `Either::B` is missing. | ||
1613 | check_no_diagnostic(content); | ||
1614 | } | ||
1615 | |||
1616 | #[test] | ||
1617 | fn expr_loop_missing_arm() { | ||
1618 | let content = r" | ||
1619 | enum Either { | ||
1620 | A, | ||
1621 | B, | ||
1622 | } | ||
1623 | fn test_fn() { | ||
1624 | match loop { break Foo::A } { | ||
1625 | Either::A => (), | ||
1626 | } | ||
1627 | } | ||
1628 | "; | ||
1629 | |||
1630 | // This is a false negative. | ||
1631 | // We currently infer the type of `loop { break Foo::A }` to `!`, which | ||
1632 | // causes us to skip the diagnostic since `Either::A` doesn't type check | ||
1633 | // with `!`. | ||
1634 | check_no_diagnostic(content); | ||
1635 | } | ||
1636 | |||
1637 | #[test] | ||
1638 | fn tuple_of_bools_with_ellipsis_at_end_missing_arm() { | ||
1639 | let content = r" | ||
1640 | fn test_fn() { | ||
1641 | match (false, true, false) { | ||
1642 | (false, ..) => {}, | ||
1643 | } | ||
1644 | } | ||
1645 | "; | ||
1646 | |||
1647 | // This is a false negative. | ||
1648 | // The `..` pattern is currently lowered to a single `Pat::Wild` | ||
1649 | // no matter how many fields the `..` pattern is covering. This | ||
1650 | // causes the match arm in this test not to type check against | ||
1651 | // the match expression, which causes this diagnostic not to | ||
1652 | // fire. | ||
1653 | check_no_diagnostic(content); | ||
1654 | } | ||
1655 | |||
1656 | #[test] | ||
1657 | fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() { | ||
1658 | let content = r" | ||
1659 | fn test_fn() { | ||
1660 | match (false, true, false) { | ||
1661 | (.., false) => {}, | ||
1662 | } | ||
1663 | } | ||
1664 | "; | ||
1665 | |||
1666 | // This is a false negative. | ||
1667 | // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. | ||
1668 | check_no_diagnostic(content); | ||
1669 | } | ||
1670 | |||
1671 | #[test] | ||
1672 | fn enum_tuple_partial_ellipsis_missing_arm() { | ||
1673 | let content = r" | ||
1674 | enum Either { | ||
1675 | A(bool, bool, bool, bool), | ||
1676 | B, | ||
1677 | } | ||
1678 | fn test_fn() { | ||
1679 | match Either::B { | ||
1680 | Either::A(true, .., true) => {}, | ||
1681 | Either::A(true, .., false) => {}, | ||
1682 | Either::A(false, .., false) => {}, | ||
1683 | Either::B => {}, | ||
1684 | } | ||
1685 | } | ||
1686 | "; | ||
1687 | |||
1688 | // This is a false negative. | ||
1689 | // The `..` pattern is currently lowered to a single `Pat::Wild` | ||
1690 | // no matter how many fields the `..` pattern is covering. This | ||
1691 | // causes us to return a `MatchCheckErr::MalformedMatchArm` in | ||
1692 | // `Pat::specialize_constructor`. | ||
1693 | check_no_diagnostic(content); | ||
1694 | } | ||
1455 | } | 1695 | } |
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 69b527f74..21abbcf1e 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs | |||
@@ -161,12 +161,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
161 | 161 | ||
162 | let mut seen = Matrix::empty(); | 162 | let mut seen = Matrix::empty(); |
163 | for pat in pats { | 163 | for pat in pats { |
164 | // We skip any patterns whose type we cannot resolve. | ||
165 | // | ||
166 | // This could lead to false positives in this diagnostic, so | ||
167 | // it might be better to skip the entire diagnostic if we either | ||
168 | // cannot resolve a match arm or determine that the match arm has | ||
169 | // the wrong type. | ||
170 | if let Some(pat_ty) = infer.type_of_pat.get(pat) { | 164 | if let Some(pat_ty) = infer.type_of_pat.get(pat) { |
171 | // We only include patterns whose type matches the type | 165 | // We only include patterns whose type matches the type |
172 | // of the match expression. If we had a InvalidMatchArmPattern | 166 | // of the match expression. If we had a InvalidMatchArmPattern |
@@ -189,8 +183,15 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
189 | // to the matrix here. | 183 | // to the matrix here. |
190 | let v = PatStack::from_pattern(pat); | 184 | let v = PatStack::from_pattern(pat); |
191 | seen.push(&cx, v); | 185 | seen.push(&cx, v); |
186 | continue; | ||
192 | } | 187 | } |
193 | } | 188 | } |
189 | |||
190 | // If we can't resolve the type of a pattern, or the pattern type doesn't | ||
191 | // fit the match expression, we skip this diagnostic. Skipping the entire | ||
192 | // diagnostic rather than just not including this match arm is preferred | ||
193 | // to avoid the chance of false positives. | ||
194 | return; | ||
194 | } | 195 | } |
195 | 196 | ||
196 | match is_useful(&cx, &seen, &PatStack::from_wild()) { | 197 | match is_useful(&cx, &seen, &PatStack::from_wild()) { |