diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index de291c1f6074..91206adc3e6f 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -250,17 +250,13 @@ pub enum Usefulness { pub struct MatchCheckCtx<'a> { pub body: Arc, - pub match_expr: &'a Expr, pub infer: Arc, pub db: &'a dyn HirDatabase, } -// see src/librustc_mir_build/hair/pattern/_match.rs -// It seems the rustc version of this method is able to assume that all the match arm -// patterns are valid (they are valid given a particular match expression), but I -// don't think we can make that assumption here. How should that be handled? -// -// Perhaps check that validity before passing the patterns into this method? +/// Given a set of patterns `matrix`, and pattern to consider `v`, determines +/// whether `v` is useful. A pattern is useful if it covers cases which were +/// not previously covered. pub(crate) fn is_useful( cx: &MatchCheckCtx, matrix: &Matrix, @@ -517,6 +513,19 @@ mod tests { check_diagnostic_with_no_fix(content); } + #[test] + fn empty_tuple_wild() { + let content = r" + fn test_fn() { + match () { + _ => {} + } + } + "; + + check_no_diagnostic(content); + } + #[test] fn empty_tuple_no_diagnostic() { let content = r" @@ -976,21 +985,6 @@ mod tests { check_no_diagnostic(content); } -} - -#[cfg(test)] -mod false_negatives { - //! The implementation of match checking here is a work in progress. As we roll this out, we - //! prefer false negatives to false positives (ideally there would be no false positives). This - //! test module should document known false negatives. Eventually we will have a complete - //! implementation of match checking and this module will be empty. - //! - //! The reasons for documenting known false negatives: - //! - //! 1. It acts as a backlog of work that can be done to improve the behavior of the system. - //! 2. It ensures the code doesn't panic when handling these cases. - - use super::tests::*; #[test] fn mismatched_types() { @@ -1011,10 +1005,8 @@ mod false_negatives { } "; - // This is a false negative. - // We don't currently check that the match arms actually - // match the type of the match expression. - check_no_diagnostic(content); + // Match arms with the incorrect type are filtered out. + check_diagnostic_with_no_fix(content); } #[test] @@ -1028,14 +1020,24 @@ mod false_negatives { } "; - // This is a false negative. - // We don't currently check that the match arms actually - // match the type of the match expression. This test - // checks to ensure we don't panic when the code we are - // checking is malformed in such a way that the arity of the - // constructors doesn't match. - check_no_diagnostic(content); + // Match arms with the incorrect type are filtered out. + check_diagnostic_with_no_fix(content); } +} + +#[cfg(test)] +mod false_negatives { + //! The implementation of match checking here is a work in progress. As we roll this out, we + //! prefer false negatives to false positives (ideally there would be no false positives). This + //! test module should document known false negatives. Eventually we will have a complete + //! implementation of match checking and this module will be empty. + //! + //! The reasons for documenting known false negatives: + //! + //! 1. It acts as a backlog of work that can be done to improve the behavior of the system. + //! 2. It ensures the code doesn't panic when handling these cases. + + use super::tests::*; #[test] fn integers() { diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 7498d04dcb4d..8c7d1a98e550 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> { fn validate_match( &mut self, id: ExprId, - expr: ExprId, + match_expr: ExprId, arms: &[MatchArm], db: &dyn HirDatabase, infer: Arc, @@ -75,18 +75,32 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let (body, source_map): (Arc, Arc) = db.body_with_source_map(self.func.into()); - let match_expr: &hir_def::expr::Expr = &body[expr]; + let match_expr_ty = match infer.type_of_expr.get(match_expr) { + Some(ty) => ty, + // If we can't resolve the type of the match expression + // we cannot perform exhaustiveness checks. + None => return, + }; - let cx = MatchCheckCtx { body: body.clone(), match_expr, infer, db }; + let cx = MatchCheckCtx { body, infer: infer.clone(), db }; let pats = arms.iter().map(|arm| arm.pat); let mut seen = Matrix::empty(); for pat in pats { - // If we had a NotUsefulMatchArm diagnostic, we could - // check the usefulness of each pattern as we added it - // to the matrix here. - let v = PatStack::from_pattern(pat); - seen.push(&cx, v); + // We skip any patterns whose type we cannot resolve. + if let Some(pat_ty) = infer.type_of_pat.get(pat) { + // We only include patterns whose type matches the type + // of the match expression. If we had a InvalidMatchArmPattern + // diagnostic or similar we could raise that in an else + // block here. + if pat_ty == match_expr_ty { + // If we had a NotUsefulMatchArm diagnostic, we could + // check the usefulness of each pattern as we added it + // to the matrix here. + let v = PatStack::from_pattern(pat); + seen.push(&cx, v); + } + } } match is_useful(&cx, &seen, &PatStack::from_wild()) {