diff options
Diffstat (limited to 'crates/ra_hir_ty')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 68 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/expr.rs | 30 |
2 files changed, 57 insertions, 41 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index de291c1f6..91206adc3 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs | |||
@@ -250,17 +250,13 @@ pub enum Usefulness { | |||
250 | 250 | ||
251 | pub struct MatchCheckCtx<'a> { | 251 | pub struct MatchCheckCtx<'a> { |
252 | pub body: Arc<Body>, | 252 | pub body: Arc<Body>, |
253 | pub match_expr: &'a Expr, | ||
254 | pub infer: Arc<InferenceResult>, | 253 | pub infer: Arc<InferenceResult>, |
255 | pub db: &'a dyn HirDatabase, | 254 | pub db: &'a dyn HirDatabase, |
256 | } | 255 | } |
257 | 256 | ||
258 | // see src/librustc_mir_build/hair/pattern/_match.rs | 257 | /// Given a set of patterns `matrix`, and pattern to consider `v`, determines |
259 | // It seems the rustc version of this method is able to assume that all the match arm | 258 | /// whether `v` is useful. A pattern is useful if it covers cases which were |
260 | // patterns are valid (they are valid given a particular match expression), but I | 259 | /// not previously covered. |
261 | // don't think we can make that assumption here. How should that be handled? | ||
262 | // | ||
263 | // Perhaps check that validity before passing the patterns into this method? | ||
264 | pub(crate) fn is_useful( | 260 | pub(crate) fn is_useful( |
265 | cx: &MatchCheckCtx, | 261 | cx: &MatchCheckCtx, |
266 | matrix: &Matrix, | 262 | matrix: &Matrix, |
@@ -518,6 +514,19 @@ mod tests { | |||
518 | } | 514 | } |
519 | 515 | ||
520 | #[test] | 516 | #[test] |
517 | fn empty_tuple_wild() { | ||
518 | let content = r" | ||
519 | fn test_fn() { | ||
520 | match () { | ||
521 | _ => {} | ||
522 | } | ||
523 | } | ||
524 | "; | ||
525 | |||
526 | check_no_diagnostic(content); | ||
527 | } | ||
528 | |||
529 | #[test] | ||
521 | fn empty_tuple_no_diagnostic() { | 530 | fn empty_tuple_no_diagnostic() { |
522 | let content = r" | 531 | let content = r" |
523 | fn test_fn() { | 532 | fn test_fn() { |
@@ -976,21 +985,6 @@ mod tests { | |||
976 | 985 | ||
977 | check_no_diagnostic(content); | 986 | check_no_diagnostic(content); |
978 | } | 987 | } |
979 | } | ||
980 | |||
981 | #[cfg(test)] | ||
982 | mod false_negatives { | ||
983 | //! The implementation of match checking here is a work in progress. As we roll this out, we | ||
984 | //! prefer false negatives to false positives (ideally there would be no false positives). This | ||
985 | //! test module should document known false negatives. Eventually we will have a complete | ||
986 | //! implementation of match checking and this module will be empty. | ||
987 | //! | ||
988 | //! The reasons for documenting known false negatives: | ||
989 | //! | ||
990 | //! 1. It acts as a backlog of work that can be done to improve the behavior of the system. | ||
991 | //! 2. It ensures the code doesn't panic when handling these cases. | ||
992 | |||
993 | use super::tests::*; | ||
994 | 988 | ||
995 | #[test] | 989 | #[test] |
996 | fn mismatched_types() { | 990 | fn mismatched_types() { |
@@ -1011,10 +1005,8 @@ mod false_negatives { | |||
1011 | } | 1005 | } |
1012 | "; | 1006 | "; |
1013 | 1007 | ||
1014 | // This is a false negative. | 1008 | // Match arms with the incorrect type are filtered out. |
1015 | // We don't currently check that the match arms actually | 1009 | check_diagnostic_with_no_fix(content); |
1016 | // match the type of the match expression. | ||
1017 | check_no_diagnostic(content); | ||
1018 | } | 1010 | } |
1019 | 1011 | ||
1020 | #[test] | 1012 | #[test] |
@@ -1028,14 +1020,24 @@ mod false_negatives { | |||
1028 | } | 1020 | } |
1029 | "; | 1021 | "; |
1030 | 1022 | ||
1031 | // This is a false negative. | 1023 | // Match arms with the incorrect type are filtered out. |
1032 | // We don't currently check that the match arms actually | 1024 | check_diagnostic_with_no_fix(content); |
1033 | // match the type of the match expression. This test | ||
1034 | // checks to ensure we don't panic when the code we are | ||
1035 | // checking is malformed in such a way that the arity of the | ||
1036 | // constructors doesn't match. | ||
1037 | check_no_diagnostic(content); | ||
1038 | } | 1025 | } |
1026 | } | ||
1027 | |||
1028 | #[cfg(test)] | ||
1029 | mod false_negatives { | ||
1030 | //! The implementation of match checking here is a work in progress. As we roll this out, we | ||
1031 | //! prefer false negatives to false positives (ideally there would be no false positives). This | ||
1032 | //! test module should document known false negatives. Eventually we will have a complete | ||
1033 | //! implementation of match checking and this module will be empty. | ||
1034 | //! | ||
1035 | //! The reasons for documenting known false negatives: | ||
1036 | //! | ||
1037 | //! 1. It acts as a backlog of work that can be done to improve the behavior of the system. | ||
1038 | //! 2. It ensures the code doesn't panic when handling these cases. | ||
1039 | |||
1040 | use super::tests::*; | ||
1039 | 1041 | ||
1040 | #[test] | 1042 | #[test] |
1041 | fn integers() { | 1043 | fn integers() { |
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 7498d04dc..8c7d1a98e 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs | |||
@@ -67,7 +67,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
67 | fn validate_match( | 67 | fn validate_match( |
68 | &mut self, | 68 | &mut self, |
69 | id: ExprId, | 69 | id: ExprId, |
70 | expr: ExprId, | 70 | match_expr: ExprId, |
71 | arms: &[MatchArm], | 71 | arms: &[MatchArm], |
72 | db: &dyn HirDatabase, | 72 | db: &dyn HirDatabase, |
73 | infer: Arc<InferenceResult>, | 73 | infer: Arc<InferenceResult>, |
@@ -75,18 +75,32 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
75 | let (body, source_map): (Arc<Body>, Arc<BodySourceMap>) = | 75 | let (body, source_map): (Arc<Body>, Arc<BodySourceMap>) = |
76 | db.body_with_source_map(self.func.into()); | 76 | db.body_with_source_map(self.func.into()); |
77 | 77 | ||
78 | let match_expr: &hir_def::expr::Expr = &body[expr]; | 78 | let match_expr_ty = match infer.type_of_expr.get(match_expr) { |
79 | Some(ty) => ty, | ||
80 | // If we can't resolve the type of the match expression | ||
81 | // we cannot perform exhaustiveness checks. | ||
82 | None => return, | ||
83 | }; | ||
79 | 84 | ||
80 | let cx = MatchCheckCtx { body: body.clone(), match_expr, infer, db }; | 85 | let cx = MatchCheckCtx { body, infer: infer.clone(), db }; |
81 | let pats = arms.iter().map(|arm| arm.pat); | 86 | let pats = arms.iter().map(|arm| arm.pat); |
82 | 87 | ||
83 | let mut seen = Matrix::empty(); | 88 | let mut seen = Matrix::empty(); |
84 | for pat in pats { | 89 | for pat in pats { |
85 | // If we had a NotUsefulMatchArm diagnostic, we could | 90 | // We skip any patterns whose type we cannot resolve. |
86 | // check the usefulness of each pattern as we added it | 91 | if let Some(pat_ty) = infer.type_of_pat.get(pat) { |
87 | // to the matrix here. | 92 | // We only include patterns whose type matches the type |
88 | let v = PatStack::from_pattern(pat); | 93 | // of the match expression. If we had a InvalidMatchArmPattern |
89 | seen.push(&cx, v); | 94 | // diagnostic or similar we could raise that in an else |
95 | // block here. | ||
96 | if pat_ty == match_expr_ty { | ||
97 | // If we had a NotUsefulMatchArm diagnostic, we could | ||
98 | // check the usefulness of each pattern as we added it | ||
99 | // to the matrix here. | ||
100 | let v = PatStack::from_pattern(pat); | ||
101 | seen.push(&cx, v); | ||
102 | } | ||
103 | } | ||
90 | } | 104 | } |
91 | 105 | ||
92 | match is_useful(&cx, &seen, &PatStack::from_wild()) { | 106 | match is_useful(&cx, &seen, &PatStack::from_wild()) { |