diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-04-07 13:53:47 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-04-07 13:53:47 +0100 |
commit | 97b963b44b9da1fca4229da4c8744fa88c25780b (patch) | |
tree | f25e0db350072cf8d084ce81adcaeeeaac3f4c42 /crates/ra_hir_ty/src/expr.rs | |
parent | b7e5d94bda362ffc21174a79aa0be113c3288e1e (diff) | |
parent | 9fc1f51b7ade2cda7d410450ab1347311eb074af (diff) |
Merge #3706
3706: missing match arms diagnostic r=flodiebold a=JoshMcguigan
Following up on https://github.com/rust-analyzer/rust-analyzer/pull/3689#issuecomment-602718222, this PR creates a missing match arms diagnostic.
At the moment this is a very early draft, but I wanted to open it just to get some initial feedback.
Initial questions:
* Have I roughly created the correct boilerplate?
* Inside the new `validate_match` function:
* Am I correct in thinking I want to do validation by comparing the match arms against `match_expr`? And when analyzing `match_expr` I should be looking at it as a `hir_def::expr::Expr`?
* I mostly copied the chained if-let statements from the struct validation. Shouldn't there be a non-failable way to get an AstPtr from the hir data structures?
Thanks for all the guidance.
Co-authored-by: Josh Mcguigan <[email protected]>
Diffstat (limited to 'crates/ra_hir_ty/src/expr.rs')
-rw-r--r-- | crates/ra_hir_ty/src/expr.rs | 89 |
1 files changed, 87 insertions, 2 deletions
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index b7b476b4c..1e7395b16 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs | |||
@@ -14,9 +14,10 @@ use rustc_hash::FxHashSet; | |||
14 | 14 | ||
15 | use crate::{ | 15 | use crate::{ |
16 | db::HirDatabase, | 16 | db::HirDatabase, |
17 | diagnostics::{MissingFields, MissingOkInTailExpr}, | 17 | diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr}, |
18 | utils::variant_data, | 18 | utils::variant_data, |
19 | ApplicationTy, InferenceResult, Ty, TypeCtor, | 19 | ApplicationTy, InferenceResult, Ty, TypeCtor, |
20 | _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, | ||
20 | }; | 21 | }; |
21 | 22 | ||
22 | pub use hir_def::{ | 23 | pub use hir_def::{ |
@@ -52,15 +53,99 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
52 | for e in body.exprs.iter() { | 53 | for e in body.exprs.iter() { |
53 | if let (id, Expr::RecordLit { path, fields, spread }) = e { | 54 | if let (id, Expr::RecordLit { path, fields, spread }) = e { |
54 | self.validate_record_literal(id, path, fields, *spread, db); | 55 | self.validate_record_literal(id, path, fields, *spread, db); |
56 | } else if let (id, Expr::Match { expr, arms }) = e { | ||
57 | self.validate_match(id, *expr, arms, db, self.infer.clone()); | ||
55 | } | 58 | } |
56 | } | 59 | } |
57 | 60 | ||
58 | let body_expr = &body[body.body_expr]; | 61 | let body_expr = &body[body.body_expr]; |
59 | if let Expr::Block { statements: _, tail: Some(t) } = body_expr { | 62 | if let Expr::Block { tail: Some(t), .. } = body_expr { |
60 | self.validate_results_in_tail_expr(body.body_expr, *t, db); | 63 | self.validate_results_in_tail_expr(body.body_expr, *t, db); |
61 | } | 64 | } |
62 | } | 65 | } |
63 | 66 | ||
67 | fn validate_match( | ||
68 | &mut self, | ||
69 | id: ExprId, | ||
70 | match_expr: ExprId, | ||
71 | arms: &[MatchArm], | ||
72 | db: &dyn HirDatabase, | ||
73 | infer: Arc<InferenceResult>, | ||
74 | ) { | ||
75 | let (body, source_map): (Arc<Body>, Arc<BodySourceMap>) = | ||
76 | db.body_with_source_map(self.func.into()); | ||
77 | |||
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 | }; | ||
84 | |||
85 | let cx = MatchCheckCtx { body, infer: infer.clone(), db }; | ||
86 | let pats = arms.iter().map(|arm| arm.pat); | ||
87 | |||
88 | let mut seen = Matrix::empty(); | ||
89 | for pat in pats { | ||
90 | // We skip any patterns whose type we cannot resolve. | ||
91 | // | ||
92 | // This could lead to false positives in this diagnostic, so | ||
93 | // it might be better to skip the entire diagnostic if we either | ||
94 | // cannot resolve a match arm or determine that the match arm has | ||
95 | // the wrong type. | ||
96 | if let Some(pat_ty) = infer.type_of_pat.get(pat) { | ||
97 | // We only include patterns whose type matches the type | ||
98 | // of the match expression. If we had a InvalidMatchArmPattern | ||
99 | // diagnostic or similar we could raise that in an else | ||
100 | // block here. | ||
101 | // | ||
102 | // When comparing the types, we also have to consider that rustc | ||
103 | // will automatically de-reference the match expression type if | ||
104 | // necessary. | ||
105 | // | ||
106 | // FIXME we should use the type checker for this. | ||
107 | if pat_ty == match_expr_ty | ||
108 | || match_expr_ty | ||
109 | .as_reference() | ||
110 | .map(|(match_expr_ty, _)| match_expr_ty == pat_ty) | ||
111 | .unwrap_or(false) | ||
112 | { | ||
113 | // If we had a NotUsefulMatchArm diagnostic, we could | ||
114 | // check the usefulness of each pattern as we added it | ||
115 | // to the matrix here. | ||
116 | let v = PatStack::from_pattern(pat); | ||
117 | seen.push(&cx, v); | ||
118 | } | ||
119 | } | ||
120 | } | ||
121 | |||
122 | match is_useful(&cx, &seen, &PatStack::from_wild()) { | ||
123 | Ok(Usefulness::Useful) => (), | ||
124 | // if a wildcard pattern is not useful, then all patterns are covered | ||
125 | Ok(Usefulness::NotUseful) => return, | ||
126 | // this path is for unimplemented checks, so we err on the side of not | ||
127 | // reporting any errors | ||
128 | _ => return, | ||
129 | } | ||
130 | |||
131 | if let Ok(source_ptr) = source_map.expr_syntax(id) { | ||
132 | if let Some(expr) = source_ptr.value.left() { | ||
133 | let root = source_ptr.file_syntax(db.upcast()); | ||
134 | if let ast::Expr::MatchExpr(match_expr) = expr.to_node(&root) { | ||
135 | if let (Some(match_expr), Some(arms)) = | ||
136 | (match_expr.expr(), match_expr.match_arm_list()) | ||
137 | { | ||
138 | self.sink.push(MissingMatchArms { | ||
139 | file: source_ptr.file_id, | ||
140 | match_expr: AstPtr::new(&match_expr), | ||
141 | arms: AstPtr::new(&arms), | ||
142 | }) | ||
143 | } | ||
144 | } | ||
145 | } | ||
146 | } | ||
147 | } | ||
148 | |||
64 | fn validate_record_literal( | 149 | fn validate_record_literal( |
65 | &mut self, | 150 | &mut self, |
66 | id: ExprId, | 151 | id: ExprId, |