diff options
author | Benjamin Coenen <[email protected]> | 2020-04-09 08:39:17 +0100 |
---|---|---|
committer | Benjamin Coenen <[email protected]> | 2020-04-09 08:53:53 +0100 |
commit | 585bb83e2aec9c79dae8c2e031e9165f40937003 (patch) | |
tree | 3dda062f3deb768b211e7e091dd5b29b9b6fae84 /crates/ra_hir_ty | |
parent | 8f1dba6f9ae1d8d314dd9d007e4c582ed1403e8d (diff) | |
parent | 080c983498afcac3eb54028af5c9f8bfe7f2c826 (diff) |
feat: add attributes support on struct fields and method #3870
Signed-off-by: Benjamin Coenen <[email protected]>
Diffstat (limited to 'crates/ra_hir_ty')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 74 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/expr.rs | 158 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/tests.rs | 2 |
3 files changed, 151 insertions, 83 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index f29a25505..9e9a9d047 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs | |||
@@ -235,7 +235,10 @@ impl From<PatId> for PatIdOrWild { | |||
235 | } | 235 | } |
236 | 236 | ||
237 | #[derive(Debug, Clone, Copy, PartialEq)] | 237 | #[derive(Debug, Clone, Copy, PartialEq)] |
238 | pub struct MatchCheckNotImplemented; | 238 | pub enum MatchCheckErr { |
239 | NotImplemented, | ||
240 | MalformedMatchArm, | ||
241 | } | ||
239 | 242 | ||
240 | /// The return type of `is_useful` is either an indication of usefulness | 243 | /// The return type of `is_useful` is either an indication of usefulness |
241 | /// of the match arm, or an error in the case the match statement | 244 | /// of the match arm, or an error in the case the match statement |
@@ -244,7 +247,7 @@ pub struct MatchCheckNotImplemented; | |||
244 | /// | 247 | /// |
245 | /// The `std::result::Result` type is used here rather than a custom enum | 248 | /// The `std::result::Result` type is used here rather than a custom enum |
246 | /// to allow the use of `?`. | 249 | /// to allow the use of `?`. |
247 | pub type MatchCheckResult<T> = Result<T, MatchCheckNotImplemented>; | 250 | pub type MatchCheckResult<T> = Result<T, MatchCheckErr>; |
248 | 251 | ||
249 | #[derive(Debug)] | 252 | #[derive(Debug)] |
250 | /// A row in a Matrix. | 253 | /// A row in a Matrix. |
@@ -335,12 +338,12 @@ impl PatStack { | |||
335 | Expr::Literal(Literal::Bool(_)) => None, | 338 | Expr::Literal(Literal::Bool(_)) => None, |
336 | // perhaps this is actually unreachable given we have | 339 | // perhaps this is actually unreachable given we have |
337 | // already checked that these match arms have the appropriate type? | 340 | // already checked that these match arms have the appropriate type? |
338 | _ => return Err(MatchCheckNotImplemented), | 341 | _ => return Err(MatchCheckErr::NotImplemented), |
339 | } | 342 | } |
340 | } | 343 | } |
341 | (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?), | 344 | (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?), |
342 | (Pat::Path(_), Constructor::Enum(constructor)) => { | 345 | (Pat::Path(_), Constructor::Enum(constructor)) => { |
343 | // enums with no associated data become `Pat::Path` | 346 | // unit enum variants become `Pat::Path` |
344 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 347 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); |
345 | if !enum_variant_matches(cx, pat_id, *constructor) { | 348 | if !enum_variant_matches(cx, pat_id, *constructor) { |
346 | None | 349 | None |
@@ -348,16 +351,23 @@ impl PatStack { | |||
348 | Some(self.to_tail()) | 351 | Some(self.to_tail()) |
349 | } | 352 | } |
350 | } | 353 | } |
351 | (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(constructor)) => { | 354 | (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => { |
352 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 355 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); |
353 | if !enum_variant_matches(cx, pat_id, *constructor) { | 356 | if !enum_variant_matches(cx, pat_id, *enum_constructor) { |
354 | None | 357 | None |
355 | } else { | 358 | } else { |
359 | // If the enum variant matches, then we need to confirm | ||
360 | // that the number of patterns aligns with the expected | ||
361 | // number of patterns for that enum variant. | ||
362 | if pat_ids.len() != constructor.arity(cx)? { | ||
363 | return Err(MatchCheckErr::MalformedMatchArm); | ||
364 | } | ||
365 | |||
356 | Some(self.replace_head_with(pat_ids)) | 366 | Some(self.replace_head_with(pat_ids)) |
357 | } | 367 | } |
358 | } | 368 | } |
359 | (Pat::Or(_), _) => return Err(MatchCheckNotImplemented), | 369 | (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), |
360 | (_, _) => return Err(MatchCheckNotImplemented), | 370 | (_, _) => return Err(MatchCheckErr::NotImplemented), |
361 | }; | 371 | }; |
362 | 372 | ||
363 | Ok(result) | 373 | Ok(result) |
@@ -514,7 +524,7 @@ pub(crate) fn is_useful( | |||
514 | return if any_useful { | 524 | return if any_useful { |
515 | Ok(Usefulness::Useful) | 525 | Ok(Usefulness::Useful) |
516 | } else if found_unimplemented { | 526 | } else if found_unimplemented { |
517 | Err(MatchCheckNotImplemented) | 527 | Err(MatchCheckErr::NotImplemented) |
518 | } else { | 528 | } else { |
519 | Ok(Usefulness::NotUseful) | 529 | Ok(Usefulness::NotUseful) |
520 | }; | 530 | }; |
@@ -567,7 +577,7 @@ pub(crate) fn is_useful( | |||
567 | } | 577 | } |
568 | 578 | ||
569 | if found_unimplemented { | 579 | if found_unimplemented { |
570 | Err(MatchCheckNotImplemented) | 580 | Err(MatchCheckErr::NotImplemented) |
571 | } else { | 581 | } else { |
572 | Ok(Usefulness::NotUseful) | 582 | Ok(Usefulness::NotUseful) |
573 | } | 583 | } |
@@ -604,7 +614,7 @@ impl Constructor { | |||
604 | match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() { | 614 | match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() { |
605 | VariantData::Tuple(struct_field_data) => struct_field_data.len(), | 615 | VariantData::Tuple(struct_field_data) => struct_field_data.len(), |
606 | VariantData::Unit => 0, | 616 | VariantData::Unit => 0, |
607 | _ => return Err(MatchCheckNotImplemented), | 617 | _ => return Err(MatchCheckErr::NotImplemented), |
608 | } | 618 | } |
609 | } | 619 | } |
610 | }; | 620 | }; |
@@ -637,20 +647,20 @@ fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Opt | |||
637 | Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), | 647 | Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), |
638 | Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { | 648 | Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { |
639 | Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), | 649 | Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), |
640 | _ => return Err(MatchCheckNotImplemented), | 650 | _ => return Err(MatchCheckErr::NotImplemented), |
641 | }, | 651 | }, |
642 | Pat::TupleStruct { .. } | Pat::Path(_) => { | 652 | Pat::TupleStruct { .. } | Pat::Path(_) => { |
643 | let pat_id = pat.as_id().expect("we already know this pattern is not a wild"); | 653 | let pat_id = pat.as_id().expect("we already know this pattern is not a wild"); |
644 | let variant_id = | 654 | let variant_id = |
645 | cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckNotImplemented)?; | 655 | cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckErr::NotImplemented)?; |
646 | match variant_id { | 656 | match variant_id { |
647 | VariantId::EnumVariantId(enum_variant_id) => { | 657 | VariantId::EnumVariantId(enum_variant_id) => { |
648 | Some(Constructor::Enum(enum_variant_id)) | 658 | Some(Constructor::Enum(enum_variant_id)) |
649 | } | 659 | } |
650 | _ => return Err(MatchCheckNotImplemented), | 660 | _ => return Err(MatchCheckErr::NotImplemented), |
651 | } | 661 | } |
652 | } | 662 | } |
653 | _ => return Err(MatchCheckNotImplemented), | 663 | _ => return Err(MatchCheckErr::NotImplemented), |
654 | }; | 664 | }; |
655 | 665 | ||
656 | Ok(res) | 666 | Ok(res) |
@@ -1325,6 +1335,40 @@ mod tests { | |||
1325 | } | 1335 | } |
1326 | 1336 | ||
1327 | #[test] | 1337 | #[test] |
1338 | fn malformed_match_arm_tuple_missing_pattern() { | ||
1339 | let content = r" | ||
1340 | fn test_fn() { | ||
1341 | match (0) { | ||
1342 | () => (), | ||
1343 | } | ||
1344 | } | ||
1345 | "; | ||
1346 | |||
1347 | // Match arms with the incorrect type are filtered out. | ||
1348 | check_diagnostic(content); | ||
1349 | } | ||
1350 | |||
1351 | #[test] | ||
1352 | fn malformed_match_arm_tuple_enum_missing_pattern() { | ||
1353 | let content = r" | ||
1354 | enum Either { | ||
1355 | A, | ||
1356 | B(u32), | ||
1357 | } | ||
1358 | fn test_fn() { | ||
1359 | match Either::A { | ||
1360 | Either::A => (), | ||
1361 | Either::B() => (), | ||
1362 | } | ||
1363 | } | ||
1364 | "; | ||
1365 | |||
1366 | // We are testing to be sure we don't panic here when the match | ||
1367 | // arm `Either::B` is missing its pattern. | ||
1368 | check_no_diagnostic(content); | ||
1369 | } | ||
1370 | |||
1371 | #[test] | ||
1328 | fn enum_not_in_scope() { | 1372 | fn enum_not_in_scope() { |
1329 | let content = r" | 1373 | let content = r" |
1330 | fn test_fn() { | 1374 | fn test_fn() { |
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index fb779cbef..e45e9ea14 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs | |||
@@ -2,12 +2,8 @@ | |||
2 | 2 | ||
3 | use std::sync::Arc; | 3 | use std::sync::Arc; |
4 | 4 | ||
5 | use hir_def::{ | 5 | use hir_def::{path::path, resolver::HasResolver, AdtId, FunctionId}; |
6 | path::{path, Path}, | 6 | use hir_expand::diagnostics::DiagnosticSink; |
7 | resolver::HasResolver, | ||
8 | AdtId, FunctionId, | ||
9 | }; | ||
10 | use hir_expand::{diagnostics::DiagnosticSink, name::Name}; | ||
11 | use ra_syntax::{ast, AstPtr}; | 7 | use ra_syntax::{ast, AstPtr}; |
12 | use rustc_hash::FxHashSet; | 8 | use rustc_hash::FxHashSet; |
13 | 9 | ||
@@ -28,7 +24,7 @@ pub use hir_def::{ | |||
28 | ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, | 24 | ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, |
29 | MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, | 25 | MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, |
30 | }, | 26 | }, |
31 | VariantId, | 27 | LocalStructFieldId, VariantId, |
32 | }; | 28 | }; |
33 | 29 | ||
34 | pub struct ExprValidator<'a, 'b: 'a> { | 30 | pub struct ExprValidator<'a, 'b: 'a> { |
@@ -49,14 +45,37 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
49 | pub fn validate_body(&mut self, db: &dyn HirDatabase) { | 45 | pub fn validate_body(&mut self, db: &dyn HirDatabase) { |
50 | let body = db.body(self.func.into()); | 46 | let body = db.body(self.func.into()); |
51 | 47 | ||
52 | for e in body.exprs.iter() { | 48 | for (id, expr) in body.exprs.iter() { |
53 | if let (id, Expr::RecordLit { path, fields, spread }) = e { | 49 | if let Some((variant_def, missed_fields, true)) = |
54 | self.validate_record_literal(id, path, fields, *spread, db); | 50 | record_literal_missing_fields(db, &self.infer, id, expr) |
55 | } else if let (id, Expr::Match { expr, arms }) = e { | 51 | { |
52 | // XXX: only look at source_map if we do have missing fields | ||
53 | let (_, source_map) = db.body_with_source_map(self.func.into()); | ||
54 | |||
55 | if let Ok(source_ptr) = source_map.expr_syntax(id) { | ||
56 | if let Some(expr) = source_ptr.value.left() { | ||
57 | let root = source_ptr.file_syntax(db.upcast()); | ||
58 | if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { | ||
59 | if let Some(field_list) = record_lit.record_field_list() { | ||
60 | let variant_data = variant_data(db.upcast(), variant_def); | ||
61 | let missed_fields = missed_fields | ||
62 | .into_iter() | ||
63 | .map(|idx| variant_data.fields()[idx].name.clone()) | ||
64 | .collect(); | ||
65 | self.sink.push(MissingFields { | ||
66 | file: source_ptr.file_id, | ||
67 | field_list: AstPtr::new(&field_list), | ||
68 | missed_fields, | ||
69 | }) | ||
70 | } | ||
71 | } | ||
72 | } | ||
73 | } | ||
74 | } | ||
75 | if let Expr::Match { expr, arms } = expr { | ||
56 | self.validate_match(id, *expr, arms, db, self.infer.clone()); | 76 | self.validate_match(id, *expr, arms, db, self.infer.clone()); |
57 | } | 77 | } |
58 | } | 78 | } |
59 | |||
60 | let body_expr = &body[body.body_expr]; | 79 | let body_expr = &body[body.body_expr]; |
61 | if let Expr::Block { tail: Some(t), .. } = body_expr { | 80 | if let Expr::Block { tail: Some(t), .. } = body_expr { |
62 | self.validate_results_in_tail_expr(body.body_expr, *t, db); | 81 | self.validate_results_in_tail_expr(body.body_expr, *t, db); |
@@ -145,61 +164,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
145 | } | 164 | } |
146 | } | 165 | } |
147 | 166 | ||
148 | fn validate_record_literal( | ||
149 | &mut self, | ||
150 | id: ExprId, | ||
151 | _path: &Option<Path>, | ||
152 | fields: &[RecordLitField], | ||
153 | spread: Option<ExprId>, | ||
154 | db: &dyn HirDatabase, | ||
155 | ) { | ||
156 | if spread.is_some() { | ||
157 | return; | ||
158 | }; | ||
159 | let variant_def: VariantId = match self.infer.variant_resolution_for_expr(id) { | ||
160 | Some(VariantId::UnionId(_)) | None => return, | ||
161 | Some(it) => it, | ||
162 | }; | ||
163 | if let VariantId::UnionId(_) = variant_def { | ||
164 | return; | ||
165 | } | ||
166 | |||
167 | let variant_data = variant_data(db.upcast(), variant_def); | ||
168 | |||
169 | let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); | ||
170 | let missed_fields: Vec<Name> = variant_data | ||
171 | .fields() | ||
172 | .iter() | ||
173 | .filter_map(|(_f, d)| { | ||
174 | let name = d.name.clone(); | ||
175 | if lit_fields.contains(&name) { | ||
176 | None | ||
177 | } else { | ||
178 | Some(name) | ||
179 | } | ||
180 | }) | ||
181 | .collect(); | ||
182 | if missed_fields.is_empty() { | ||
183 | return; | ||
184 | } | ||
185 | let (_, source_map) = db.body_with_source_map(self.func.into()); | ||
186 | |||
187 | if let Ok(source_ptr) = source_map.expr_syntax(id) { | ||
188 | if let Some(expr) = source_ptr.value.left() { | ||
189 | let root = source_ptr.file_syntax(db.upcast()); | ||
190 | if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { | ||
191 | if let Some(field_list) = record_lit.record_field_list() { | ||
192 | self.sink.push(MissingFields { | ||
193 | file: source_ptr.file_id, | ||
194 | field_list: AstPtr::new(&field_list), | ||
195 | missed_fields, | ||
196 | }) | ||
197 | } | ||
198 | } | ||
199 | } | ||
200 | } | ||
201 | } | ||
202 | |||
203 | fn validate_results_in_tail_expr(&mut self, body_id: ExprId, id: ExprId, db: &dyn HirDatabase) { | 167 | fn validate_results_in_tail_expr(&mut self, body_id: ExprId, id: ExprId, db: &dyn HirDatabase) { |
204 | // the mismatch will be on the whole block currently | 168 | // the mismatch will be on the whole block currently |
205 | let mismatch = match self.infer.type_mismatch_for_expr(body_id) { | 169 | let mismatch = match self.infer.type_mismatch_for_expr(body_id) { |
@@ -232,3 +196,63 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
232 | } | 196 | } |
233 | } | 197 | } |
234 | } | 198 | } |
199 | |||
200 | pub fn record_literal_missing_fields( | ||
201 | db: &dyn HirDatabase, | ||
202 | infer: &InferenceResult, | ||
203 | id: ExprId, | ||
204 | expr: &Expr, | ||
205 | ) -> Option<(VariantId, Vec<LocalStructFieldId>, /*exhaustive*/ bool)> { | ||
206 | let (fields, exhausitve) = match expr { | ||
207 | Expr::RecordLit { path: _, fields, spread } => (fields, spread.is_none()), | ||
208 | _ => return None, | ||
209 | }; | ||
210 | |||
211 | let variant_def = infer.variant_resolution_for_expr(id)?; | ||
212 | if let VariantId::UnionId(_) = variant_def { | ||
213 | return None; | ||
214 | } | ||
215 | |||
216 | let variant_data = variant_data(db.upcast(), variant_def); | ||
217 | |||
218 | let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); | ||
219 | let missed_fields: Vec<LocalStructFieldId> = variant_data | ||
220 | .fields() | ||
221 | .iter() | ||
222 | .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) }) | ||
223 | .collect(); | ||
224 | if missed_fields.is_empty() { | ||
225 | return None; | ||
226 | } | ||
227 | Some((variant_def, missed_fields, exhausitve)) | ||
228 | } | ||
229 | |||
230 | pub fn record_pattern_missing_fields( | ||
231 | db: &dyn HirDatabase, | ||
232 | infer: &InferenceResult, | ||
233 | id: PatId, | ||
234 | pat: &Pat, | ||
235 | ) -> Option<(VariantId, Vec<LocalStructFieldId>)> { | ||
236 | let fields = match pat { | ||
237 | Pat::Record { path: _, args } => args, | ||
238 | _ => return None, | ||
239 | }; | ||
240 | |||
241 | let variant_def = infer.variant_resolution_for_pat(id)?; | ||
242 | if let VariantId::UnionId(_) = variant_def { | ||
243 | return None; | ||
244 | } | ||
245 | |||
246 | let variant_data = variant_data(db.upcast(), variant_def); | ||
247 | |||
248 | let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); | ||
249 | let missed_fields: Vec<LocalStructFieldId> = variant_data | ||
250 | .fields() | ||
251 | .iter() | ||
252 | .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) }) | ||
253 | .collect(); | ||
254 | if missed_fields.is_empty() { | ||
255 | return None; | ||
256 | } | ||
257 | Some((variant_def, missed_fields)) | ||
258 | } | ||
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 060814e53..608408d88 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs | |||
@@ -336,7 +336,7 @@ fn no_such_field_with_feature_flag_diagnostics() { | |||
336 | pub(crate) fn new(my_val: usize, bar: bool) -> Self { | 336 | pub(crate) fn new(my_val: usize, bar: bool) -> Self { |
337 | Self { my_val, bar } | 337 | Self { my_val, bar } |
338 | } | 338 | } |
339 | 339 | ||
340 | #[cfg(not(feature = "foo"))] | 340 | #[cfg(not(feature = "foo"))] |
341 | pub(crate) fn new(my_val: usize, _bar: bool) -> Self { | 341 | pub(crate) fn new(my_val: usize, _bar: bool) -> Self { |
342 | Self { my_val } | 342 | Self { my_val } |