diff options
| author | Josh Mcguigan <[email protected]> | 2020-04-05 18:16:34 +0100 |
|---|---|---|
| committer | Josh Mcguigan <[email protected]> | 2020-04-07 13:12:08 +0100 |
| commit | 43dfd894934cf7c9161e473495a4e24965239475 (patch) | |
| tree | 323ddd9e91ea46713b1b5c30d40cd6474310c6e0 /crates/ra_hir_ty | |
| parent | b87b7a088f34ff794fc19e57ee2ae1cfe81a12df (diff) | |
handle non matching enum pattern types
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()) { |