aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-04-18 19:42:07 +0100
committerGitHub <[email protected]>2020-04-18 19:42:07 +0100
commit6f60e646fc2aa50eb53e9c644f0f92651b04cc5b (patch)
tree507f92918f72c37e6ff3adf24b9ddefa83df42e1
parent98819d89199c5138cc5018b036b0ec5d3fade77e (diff)
parent7adb681b1f8dbdc0205efbe22698c28ab9da6378 (diff)
Merge #3894
3894: Match check enum record r=flodiebold a=JoshMcguigan This PR implements match statement exhaustiveness checking for record type enums. It also make a minor addition to the test infrastructure to allow testing against a single diagnostic, so you can be sure your test is triggering (or not) whichever diagnostic you expect. Co-authored-by: Josh Mcguigan <[email protected]>
-rw-r--r--crates/ra_hir_ty/src/_match.rs336
-rw-r--r--crates/ra_hir_ty/src/test_db.rs36
2 files changed, 331 insertions, 41 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs
index 688026a04..779e78574 100644
--- a/crates/ra_hir_ty/src/_match.rs
+++ b/crates/ra_hir_ty/src/_match.rs
@@ -235,10 +235,19 @@ impl From<PatId> for PatIdOrWild {
235 } 235 }
236} 236}
237 237
238impl From<&PatId> for PatIdOrWild {
239 fn from(pat_id: &PatId) -> Self {
240 Self::PatId(*pat_id)
241 }
242}
243
238#[derive(Debug, Clone, Copy, PartialEq)] 244#[derive(Debug, Clone, Copy, PartialEq)]
239pub enum MatchCheckErr { 245pub enum MatchCheckErr {
240 NotImplemented, 246 NotImplemented,
241 MalformedMatchArm, 247 MalformedMatchArm,
248 /// Used when type inference cannot resolve the type of
249 /// a pattern or expression.
250 Unknown,
242} 251}
243 252
244/// The return type of `is_useful` is either an indication of usefulness 253/// The return type of `is_useful` is either an indication of usefulness
@@ -290,10 +299,14 @@ impl PatStack {
290 Self::from_slice(&self.0[1..]) 299 Self::from_slice(&self.0[1..])
291 } 300 }
292 301
293 fn replace_head_with<T: Into<PatIdOrWild> + Copy>(&self, pat_ids: &[T]) -> PatStack { 302 fn replace_head_with<I, T>(&self, pats: I) -> PatStack
303 where
304 I: Iterator<Item = T>,
305 T: Into<PatIdOrWild>,
306 {
294 let mut patterns: PatStackInner = smallvec![]; 307 let mut patterns: PatStackInner = smallvec![];
295 for pat in pat_ids { 308 for pat in pats {
296 patterns.push((*pat).into()); 309 patterns.push(pat.into());
297 } 310 }
298 for pat in &self.0[1..] { 311 for pat in &self.0[1..] {
299 patterns.push(*pat); 312 patterns.push(*pat);
@@ -330,7 +343,7 @@ impl PatStack {
330 return Err(MatchCheckErr::NotImplemented); 343 return Err(MatchCheckErr::NotImplemented);
331 } 344 }
332 345
333 Some(self.replace_head_with(pat_ids)) 346 Some(self.replace_head_with(pat_ids.iter()))
334 } 347 }
335 (Pat::Lit(lit_expr), Constructor::Bool(constructor_val)) => { 348 (Pat::Lit(lit_expr), Constructor::Bool(constructor_val)) => {
336 match cx.body.exprs[lit_expr] { 349 match cx.body.exprs[lit_expr] {
@@ -382,7 +395,7 @@ impl PatStack {
382 new_patterns.push((*pat_id).into()); 395 new_patterns.push((*pat_id).into());
383 } 396 }
384 397
385 Some(self.replace_head_with(&new_patterns)) 398 Some(self.replace_head_with(new_patterns.into_iter()))
386 } else { 399 } else {
387 return Err(MatchCheckErr::MalformedMatchArm); 400 return Err(MatchCheckErr::MalformedMatchArm);
388 } 401 }
@@ -390,13 +403,41 @@ impl PatStack {
390 // If there is no ellipsis in the tuple pattern, the number 403 // If there is no ellipsis in the tuple pattern, the number
391 // of patterns must equal the constructor arity. 404 // of patterns must equal the constructor arity.
392 if pat_ids.len() == constructor_arity { 405 if pat_ids.len() == constructor_arity {
393 Some(self.replace_head_with(pat_ids)) 406 Some(self.replace_head_with(pat_ids.into_iter()))
394 } else { 407 } else {
395 return Err(MatchCheckErr::MalformedMatchArm); 408 return Err(MatchCheckErr::MalformedMatchArm);
396 } 409 }
397 } 410 }
398 } 411 }
399 } 412 }
413 (Pat::Record { args: ref arg_patterns, .. }, Constructor::Enum(e)) => {
414 let pat_id = self.head().as_id().expect("we know this isn't a wild");
415 if !enum_variant_matches(cx, pat_id, *e) {
416 None
417 } else {
418 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() {
419 VariantData::Record(struct_field_arena) => {
420 // Here we treat any missing fields in the record as the wild pattern, as
421 // if the record has ellipsis. We want to do this here even if the
422 // record does not contain ellipsis, because it allows us to continue
423 // enforcing exhaustiveness for the rest of the match statement.
424 //
425 // Creating the diagnostic for the missing field in the pattern
426 // should be done in a different diagnostic.
427 let patterns = struct_field_arena.iter().map(|(_, struct_field)| {
428 arg_patterns
429 .iter()
430 .find(|pat| pat.name == struct_field.name)
431 .map(|pat| PatIdOrWild::from(pat.pat))
432 .unwrap_or(PatIdOrWild::Wild)
433 });
434
435 Some(self.replace_head_with(patterns))
436 }
437 _ => return Err(MatchCheckErr::Unknown),
438 }
439 }
440 }
400 (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), 441 (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented),
401 (_, _) => return Err(MatchCheckErr::NotImplemented), 442 (_, _) => return Err(MatchCheckErr::NotImplemented),
402 }; 443 };
@@ -655,8 +696,8 @@ impl Constructor {
655 Constructor::Enum(e) => { 696 Constructor::Enum(e) => {
656 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() { 697 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() {
657 VariantData::Tuple(struct_field_data) => struct_field_data.len(), 698 VariantData::Tuple(struct_field_data) => struct_field_data.len(),
699 VariantData::Record(struct_field_data) => struct_field_data.len(),
658 VariantData::Unit => 0, 700 VariantData::Unit => 0,
659 _ => return Err(MatchCheckErr::NotImplemented),
660 } 701 }
661 } 702 }
662 }; 703 };
@@ -695,10 +736,10 @@ fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Opt
695 Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), 736 Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)),
696 _ => return Err(MatchCheckErr::NotImplemented), 737 _ => return Err(MatchCheckErr::NotImplemented),
697 }, 738 },
698 Pat::TupleStruct { .. } | Pat::Path(_) => { 739 Pat::TupleStruct { .. } | Pat::Path(_) | Pat::Record { .. } => {
699 let pat_id = pat.as_id().expect("we already know this pattern is not a wild"); 740 let pat_id = pat.as_id().expect("we already know this pattern is not a wild");
700 let variant_id = 741 let variant_id =
701 cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckErr::NotImplemented)?; 742 cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckErr::Unknown)?;
702 match variant_id { 743 match variant_id {
703 VariantId::EnumVariantId(enum_variant_id) => { 744 VariantId::EnumVariantId(enum_variant_id) => {
704 Some(Constructor::Enum(enum_variant_id)) 745 Some(Constructor::Enum(enum_variant_id))
@@ -759,20 +800,22 @@ mod tests {
759 pub(super) use insta::assert_snapshot; 800 pub(super) use insta::assert_snapshot;
760 pub(super) use ra_db::fixture::WithFixture; 801 pub(super) use ra_db::fixture::WithFixture;
761 802
762 pub(super) use crate::test_db::TestDB; 803 pub(super) use crate::{diagnostics::MissingMatchArms, test_db::TestDB};
763 804
764 pub(super) fn check_diagnostic_message(content: &str) -> String { 805 pub(super) fn check_diagnostic_message(content: &str) -> String {
765 TestDB::with_single_file(content).0.diagnostics().0 806 TestDB::with_single_file(content).0.diagnostic::<MissingMatchArms>().0
766 } 807 }
767 808
768 pub(super) fn check_diagnostic(content: &str) { 809 pub(super) fn check_diagnostic(content: &str) {
769 let diagnostic_count = TestDB::with_single_file(content).0.diagnostics().1; 810 let diagnostic_count =
811 TestDB::with_single_file(content).0.diagnostic::<MissingMatchArms>().1;
770 812
771 assert_eq!(1, diagnostic_count, "no diagnostic reported"); 813 assert_eq!(1, diagnostic_count, "no diagnostic reported");
772 } 814 }
773 815
774 pub(super) fn check_no_diagnostic(content: &str) { 816 pub(super) fn check_no_diagnostic(content: &str) {
775 let diagnostic_count = TestDB::with_single_file(content).0.diagnostics().1; 817 let diagnostic_count =
818 TestDB::with_single_file(content).0.diagnostic::<MissingMatchArms>().1;
776 819
777 assert_eq!(0, diagnostic_count, "expected no diagnostic, found one"); 820 assert_eq!(0, diagnostic_count, "expected no diagnostic, found one");
778 } 821 }
@@ -1532,6 +1575,236 @@ mod tests {
1532 } 1575 }
1533 1576
1534 #[test] 1577 #[test]
1578 fn enum_record_no_arms() {
1579 let content = r"
1580 enum Either {
1581 A { foo: bool },
1582 B,
1583 }
1584 fn test_fn() {
1585 let a = Either::A { foo: true };
1586 match a {
1587 }
1588 }
1589 ";
1590
1591 check_diagnostic(content);
1592 }
1593
1594 #[test]
1595 fn enum_record_missing_arms() {
1596 let content = r"
1597 enum Either {
1598 A { foo: bool },
1599 B,
1600 }
1601 fn test_fn() {
1602 let a = Either::A { foo: true };
1603 match a {
1604 Either::A { foo: true } => (),
1605 }
1606 }
1607 ";
1608
1609 check_diagnostic(content);
1610 }
1611
1612 #[test]
1613 fn enum_record_no_diagnostic() {
1614 let content = r"
1615 enum Either {
1616 A { foo: bool },
1617 B,
1618 }
1619 fn test_fn() {
1620 let a = Either::A { foo: true };
1621 match a {
1622 Either::A { foo: true } => (),
1623 Either::A { foo: false } => (),
1624 Either::B => (),
1625 }
1626 }
1627 ";
1628
1629 check_no_diagnostic(content);
1630 }
1631
1632 #[test]
1633 fn enum_record_missing_field_no_diagnostic() {
1634 let content = r"
1635 enum Either {
1636 A { foo: bool },
1637 B,
1638 }
1639 fn test_fn() {
1640 let a = Either::B;
1641 match a {
1642 Either::A { } => (),
1643 Either::B => (),
1644 }
1645 }
1646 ";
1647
1648 // When `Either::A` is missing a struct member, we don't want
1649 // to fire the missing match arm diagnostic. This should fire
1650 // some other diagnostic.
1651 check_no_diagnostic(content);
1652 }
1653
1654 #[test]
1655 fn enum_record_missing_field_missing_match_arm() {
1656 let content = r"
1657 enum Either {
1658 A { foo: bool },
1659 B,
1660 }
1661 fn test_fn() {
1662 let a = Either::B;
1663 match a {
1664 Either::A { } => (),
1665 }
1666 }
1667 ";
1668
1669 // Even though `Either::A` is missing fields, we still want to fire
1670 // the missing arm diagnostic here, since we know `Either::B` is missing.
1671 check_diagnostic(content);
1672 }
1673
1674 #[test]
1675 fn enum_record_no_diagnostic_wild() {
1676 let content = r"
1677 enum Either {
1678 A { foo: bool },
1679 B,
1680 }
1681 fn test_fn() {
1682 let a = Either::A { foo: true };
1683 match a {
1684 Either::A { foo: _ } => (),
1685 Either::B => (),
1686 }
1687 }
1688 ";
1689
1690 check_no_diagnostic(content);
1691 }
1692
1693 #[test]
1694 fn enum_record_fields_out_of_order_missing_arm() {
1695 let content = r"
1696 enum Either {
1697 A { foo: bool, bar: () },
1698 B,
1699 }
1700 fn test_fn() {
1701 let a = Either::A { foo: true };
1702 match a {
1703 Either::A { bar: (), foo: false } => (),
1704 Either::A { foo: true, bar: () } => (),
1705 }
1706 }
1707 ";
1708
1709 check_diagnostic(content);
1710 }
1711
1712 #[test]
1713 fn enum_record_fields_out_of_order_no_diagnostic() {
1714 let content = r"
1715 enum Either {
1716 A { foo: bool, bar: () },
1717 B,
1718 }
1719 fn test_fn() {
1720 let a = Either::A { foo: true };
1721 match a {
1722 Either::A { bar: (), foo: false } => (),
1723 Either::A { foo: true, bar: () } => (),
1724 Either::B => (),
1725 }
1726 }
1727 ";
1728
1729 check_no_diagnostic(content);
1730 }
1731
1732 #[test]
1733 fn enum_record_ellipsis_missing_arm() {
1734 let content = r"
1735 enum Either {
1736 A { foo: bool, bar: bool },
1737 B,
1738 }
1739 fn test_fn() {
1740 match Either::B {
1741 Either::A { foo: true, .. } => (),
1742 Either::B => (),
1743 }
1744 }
1745 ";
1746
1747 check_diagnostic(content);
1748 }
1749
1750 #[test]
1751 fn enum_record_ellipsis_no_diagnostic() {
1752 let content = r"
1753 enum Either {
1754 A { foo: bool, bar: bool },
1755 B,
1756 }
1757 fn test_fn() {
1758 let a = Either::A { foo: true };
1759 match a {
1760 Either::A { foo: true, .. } => (),
1761 Either::A { foo: false, .. } => (),
1762 Either::B => (),
1763 }
1764 }
1765 ";
1766
1767 check_no_diagnostic(content);
1768 }
1769
1770 #[test]
1771 fn enum_record_ellipsis_all_fields_missing_arm() {
1772 let content = r"
1773 enum Either {
1774 A { foo: bool, bar: bool },
1775 B,
1776 }
1777 fn test_fn() {
1778 let a = Either::B;
1779 match a {
1780 Either::A { .. } => (),
1781 }
1782 }
1783 ";
1784
1785 check_diagnostic(content);
1786 }
1787
1788 #[test]
1789 fn enum_record_ellipsis_all_fields_no_diagnostic() {
1790 let content = r"
1791 enum Either {
1792 A { foo: bool, bar: bool },
1793 B,
1794 }
1795 fn test_fn() {
1796 let a = Either::B;
1797 match a {
1798 Either::A { .. } => (),
1799 Either::B => (),
1800 }
1801 }
1802 ";
1803
1804 check_no_diagnostic(content);
1805 }
1806
1807 #[test]
1535 fn enum_tuple_partial_ellipsis_no_diagnostic() { 1808 fn enum_tuple_partial_ellipsis_no_diagnostic() {
1536 let content = r" 1809 let content = r"
1537 enum Either { 1810 enum Either {
@@ -1689,25 +1962,6 @@ mod false_negatives {
1689 } 1962 }
1690 1963
1691 #[test] 1964 #[test]
1692 fn enum_record() {
1693 let content = r"
1694 enum Either {
1695 A { foo: u32 },
1696 B,
1697 }
1698 fn test_fn() {
1699 match Either::B {
1700 Either::A { foo: 5 } => (),
1701 }
1702 }
1703 ";
1704
1705 // This is a false negative.
1706 // We don't currently handle enum record types.
1707 check_no_diagnostic(content);
1708 }
1709
1710 #[test]
1711 fn internal_or() { 1965 fn internal_or() {
1712 let content = r" 1966 let content = r"
1713 fn test_fn() { 1967 fn test_fn() {
@@ -1796,4 +2050,22 @@ mod false_negatives {
1796 // We don't currently handle tuple patterns with ellipsis. 2050 // We don't currently handle tuple patterns with ellipsis.
1797 check_no_diagnostic(content); 2051 check_no_diagnostic(content);
1798 } 2052 }
2053
2054 #[test]
2055 fn struct_missing_arm() {
2056 let content = r"
2057 struct Foo {
2058 a: bool,
2059 }
2060 fn test_fn(f: Foo) {
2061 match f {
2062 Foo { a: true } => {},
2063 }
2064 }
2065 ";
2066
2067 // This is a false negative.
2068 // We don't currently handle structs.
2069 check_no_diagnostic(content);
2070 }
1799} 2071}
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index 3a4d58bf9..8498d3d96 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -12,7 +12,7 @@ use ra_db::{
12}; 12};
13use stdx::format_to; 13use stdx::format_to;
14 14
15use crate::{db::HirDatabase, expr::ExprValidator}; 15use crate::{db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator};
16 16
17#[salsa::database( 17#[salsa::database(
18 ra_db::SourceDatabaseExtStorage, 18 ra_db::SourceDatabaseExtStorage,
@@ -104,10 +104,7 @@ impl TestDB {
104 panic!("Can't find module for file") 104 panic!("Can't find module for file")
105 } 105 }
106 106
107 // FIXME: don't duplicate this 107 fn diag<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) {
108 pub fn diagnostics(&self) -> (String, u32) {
109 let mut buf = String::new();
110 let mut count = 0;
111 let crate_graph = self.crate_graph(); 108 let crate_graph = self.crate_graph();
112 for krate in crate_graph.iter() { 109 for krate in crate_graph.iter() {
113 let crate_def_map = self.crate_def_map(krate); 110 let crate_def_map = self.crate_def_map(krate);
@@ -132,15 +129,36 @@ impl TestDB {
132 129
133 for f in fns { 130 for f in fns {
134 let infer = self.infer(f.into()); 131 let infer = self.infer(f.into());
135 let mut sink = DiagnosticSink::new(|d| { 132 let mut sink = DiagnosticSink::new(&mut cb);
136 format_to!(buf, "{:?}: {}\n", d.syntax_node(self).text(), d.message());
137 count += 1;
138 });
139 infer.add_diagnostics(self, f, &mut sink); 133 infer.add_diagnostics(self, f, &mut sink);
140 let mut validator = ExprValidator::new(f, infer, &mut sink); 134 let mut validator = ExprValidator::new(f, infer, &mut sink);
141 validator.validate_body(self); 135 validator.validate_body(self);
142 } 136 }
143 } 137 }
138 }
139
140 pub fn diagnostics(&self) -> (String, u32) {
141 let mut buf = String::new();
142 let mut count = 0;
143 self.diag(|d| {
144 format_to!(buf, "{:?}: {}\n", d.syntax_node(self).text(), d.message());
145 count += 1;
146 });
147 (buf, count)
148 }
149
150 /// Like `diagnostics`, but filtered for a single diagnostic.
151 pub fn diagnostic<D: Diagnostic>(&self) -> (String, u32) {
152 let mut buf = String::new();
153 let mut count = 0;
154 self.diag(|d| {
155 // We want to filter diagnostics by the particular one we are testing for, to
156 // avoid surprising results in tests.
157 if d.downcast_ref::<D>().is_some() {
158 format_to!(buf, "{:?}: {}\n", d.syntax_node(self).text(), d.message());
159 count += 1;
160 };
161 });
144 (buf, count) 162 (buf, count)
145 } 163 }
146} 164}