diff options
author | Josh Mcguigan <[email protected]> | 2020-04-07 23:32:14 +0100 |
---|---|---|
committer | Josh Mcguigan <[email protected]> | 2020-04-08 18:47:05 +0100 |
commit | 941615748d9000e66ff4400ae519dc60410a11f7 (patch) | |
tree | 05606f6755f1ac7d46e38784fb8b2952a7153f99 /crates/ra_hir_ty | |
parent | 4762c6d9c66dc1b6be9b9010dbe787ef8d69530a (diff) |
fix panic in match checking when tuple enum missing pattern
Diffstat (limited to 'crates/ra_hir_ty')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 60 |
1 files changed, 45 insertions, 15 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index f29a25505..037db5cf8 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,26 @@ mod tests { | |||
1325 | } | 1335 | } |
1326 | 1336 | ||
1327 | #[test] | 1337 | #[test] |
1338 | fn malformed_match_arm_tuple_enum_missing_pattern() { | ||
1339 | let content = r" | ||
1340 | enum Either { | ||
1341 | A, | ||
1342 | B(u32), | ||
1343 | } | ||
1344 | fn test_fn() { | ||
1345 | match Either::A { | ||
1346 | Either::A => (), | ||
1347 | Either::B() => (), | ||
1348 | } | ||
1349 | } | ||
1350 | "; | ||
1351 | |||
1352 | // We are testing to be sure we don't panic here when the match | ||
1353 | // arm `Either::B` is missing its pattern. | ||
1354 | check_no_diagnostic(content); | ||
1355 | } | ||
1356 | |||
1357 | #[test] | ||
1328 | fn enum_not_in_scope() { | 1358 | fn enum_not_in_scope() { |
1329 | let content = r" | 1359 | let content = r" |
1330 | fn test_fn() { | 1360 | fn test_fn() { |