From 6940cfed1e24a67e816e69e1093e04c0eb73e070 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 12 Jun 2021 19:28:19 +0300 Subject: Move some hir_ty diagnostics to hir --- crates/hir_ty/src/diagnostics.rs | 403 +-------- crates/hir_ty/src/diagnostics/expr.rs | 499 ++--------- crates/hir_ty/src/diagnostics/match_check.rs | 957 --------------------- .../src/diagnostics/match_check/deconstruct_pat.rs | 1 - 4 files changed, 97 insertions(+), 1763 deletions(-) (limited to 'crates/hir_ty') diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index 12131d9bc..f3236bc06 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -7,9 +7,8 @@ mod decl_check; use std::{any::Any, fmt}; use base_db::CrateId; -use hir_def::{DefWithBodyId, ModuleDefId}; -use hir_expand::{name::Name, HirFileId, InFile}; -use stdx::format_to; +use hir_def::ModuleDefId; +use hir_expand::{HirFileId, InFile}; use syntax::{ast, AstPtr, SyntaxNodePtr}; use crate::{ @@ -18,7 +17,9 @@ use crate::{ }; pub use crate::diagnostics::{ - expr::{record_literal_missing_fields, record_pattern_missing_fields}, + expr::{ + record_literal_missing_fields, record_pattern_missing_fields, BodyValidationDiagnostic, + }, unsafe_check::missing_unsafe, }; @@ -33,223 +34,6 @@ pub fn validate_module_item( validator.validate_item(owner); } -pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) { - let _p = profile::span("validate_body"); - let infer = db.infer(owner); - let mut validator = expr::ExprValidator::new(owner, infer.clone(), sink); - validator.validate_body(db); -} - -// Diagnostic: missing-structure-fields -// -// This diagnostic is triggered if record lacks some fields that exist in the corresponding structure. -// -// Example: -// -// ```rust -// struct A { a: u8, b: u8 } -// -// let a = A { a: 10 }; -// ``` -#[derive(Debug)] -pub struct MissingFields { - pub file: HirFileId, - pub field_list_parent: AstPtr, - pub field_list_parent_path: Option>, - pub missed_fields: Vec, -} - -impl Diagnostic for MissingFields { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("missing-structure-fields") - } - fn message(&self) -> String { - let mut buf = String::from("Missing structure fields:\n"); - for field in &self.missed_fields { - format_to!(buf, "- {}\n", field); - } - buf - } - - fn display_source(&self) -> InFile { - InFile { - file_id: self.file, - value: self - .field_list_parent_path - .clone() - .map(SyntaxNodePtr::from) - .unwrap_or_else(|| self.field_list_parent.clone().into()), - } - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - -// Diagnostic: missing-pat-fields -// -// This diagnostic is triggered if pattern lacks some fields that exist in the corresponding structure. -// -// Example: -// -// ```rust -// struct A { a: u8, b: u8 } -// -// let a = A { a: 10, b: 20 }; -// -// if let A { a } = a { -// // ... -// } -// ``` -#[derive(Debug)] -pub struct MissingPatFields { - pub file: HirFileId, - pub field_list_parent: AstPtr, - pub field_list_parent_path: Option>, - pub missed_fields: Vec, -} - -impl Diagnostic for MissingPatFields { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("missing-pat-fields") - } - fn message(&self) -> String { - let mut buf = String::from("Missing structure fields:\n"); - for field in &self.missed_fields { - format_to!(buf, "- {}\n", field); - } - buf - } - fn display_source(&self) -> InFile { - InFile { - file_id: self.file, - value: self - .field_list_parent_path - .clone() - .map(SyntaxNodePtr::from) - .unwrap_or_else(|| self.field_list_parent.clone().into()), - } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - -// Diagnostic: missing-match-arm -// -// This diagnostic is triggered if `match` block is missing one or more match arms. -#[derive(Debug)] -pub struct MissingMatchArms { - pub file: HirFileId, - pub match_expr: AstPtr, - pub arms: AstPtr, -} - -impl Diagnostic for MissingMatchArms { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("missing-match-arm") - } - fn message(&self) -> String { - String::from("Missing match arm") - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.match_expr.clone().into() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - -// Diagnostic: missing-ok-or-some-in-tail-expr -// -// This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`, -// or if a block that should return `Option` returns a value not wrapped in `Some`. -// -// Example: -// -// ```rust -// fn foo() -> Result { -// 10 -// } -// ``` -#[derive(Debug)] -pub struct MissingOkOrSomeInTailExpr { - pub file: HirFileId, - pub expr: AstPtr, - // `Some` or `Ok` depending on whether the return type is Result or Option - pub required: String, -} - -impl Diagnostic for MissingOkOrSomeInTailExpr { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("missing-ok-or-some-in-tail-expr") - } - fn message(&self) -> String { - format!("wrap return expression in {}", self.required) - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.expr.clone().into() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - -#[derive(Debug)] -pub struct RemoveThisSemicolon { - pub file: HirFileId, - pub expr: AstPtr, -} - -impl Diagnostic for RemoveThisSemicolon { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("remove-this-semicolon") - } - - fn message(&self) -> String { - "Remove this semicolon".to_string() - } - - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.expr.clone().into() } - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - -// Diagnostic: mismatched-arg-count -// -// This diagnostic is triggered if a function is invoked with an incorrect amount of arguments. -#[derive(Debug)] -pub struct MismatchedArgCount { - pub file: HirFileId, - pub call_expr: AstPtr, - pub expected: usize, - pub found: usize, -} - -impl Diagnostic for MismatchedArgCount { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("mismatched-arg-count") - } - fn message(&self) -> String { - let s = if self.expected == 1 { "" } else { "s" }; - format!("Expected {} argument{}, found {}", self.expected, s, self.found) - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.call_expr.clone().into() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - fn is_experimental(&self) -> bool { - true - } -} - #[derive(Debug)] pub enum CaseType { // `some_var` @@ -344,31 +128,6 @@ impl Diagnostic for IncorrectCase { } } -// Diagnostic: replace-filter-map-next-with-find-map -// -// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. -#[derive(Debug)] -pub struct ReplaceFilterMapNextWithFindMap { - pub file: HirFileId, - /// This expression is the whole method chain up to and including `.filter_map(..).next()`. - pub next_expr: AstPtr, -} - -impl Diagnostic for ReplaceFilterMapNextWithFindMap { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("replace-filter-map-next-with-find-map") - } - fn message(&self) -> String { - "replace filter_map(..).next() with find_map(..)".to_string() - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.next_expr.clone().into() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - #[cfg(test)] mod tests { use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; @@ -378,7 +137,7 @@ mod tests { use syntax::{TextRange, TextSize}; use crate::{ - diagnostics::{validate_body, validate_module_item}, + diagnostics::validate_module_item, diagnostics_sink::{Diagnostic, DiagnosticSinkBuilder}, test_db::TestDB, }; @@ -416,11 +175,6 @@ mod tests { } } } - - for f in fns { - let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); - validate_body(self, f.into(), &mut sink); - } } } } @@ -455,58 +209,6 @@ mod tests { assert_eq!(annotations, actual); } - #[test] - fn missing_record_pat_field_diagnostic() { - check_diagnostics( - r#" -struct S { foo: i32, bar: () } -fn baz(s: S) { - let S { foo: _ } = s; - //^ Missing structure fields: - //| - bar -} -"#, - ); - } - - #[test] - fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { - check_diagnostics( - r" -struct S { foo: i32, bar: () } -fn baz(s: S) -> i32 { - match s { - S { foo, .. } => foo, - } -} -", - ) - } - - #[test] - fn missing_record_pat_field_box() { - check_diagnostics( - r" -struct S { s: Box } -fn x(a: S) { - let S { box s } = a; -} -", - ) - } - - #[test] - fn missing_record_pat_field_ref() { - check_diagnostics( - r" -struct S { s: u32 } -fn x(a: S) { - let S { ref s } = a; -} -", - ) - } - #[test] fn import_extern_crate_clash_with_inner_item() { // This is more of a resolver test, but doesn't really work with the hir_def testsuite. @@ -535,97 +237,4 @@ pub struct Claims { "#, ); } - - #[test] - fn missing_semicolon() { - check_diagnostics( - r#" - fn test() -> i32 { 123; } - //^^^ Remove this semicolon - "#, - ); - } - - // Register the required standard library types to make the tests work - fn add_filter_map_with_find_next_boilerplate(body: &str) -> String { - let prefix = r#" - //- /main.rs crate:main deps:core - use core::iter::Iterator; - use core::option::Option::{self, Some, None}; - "#; - let suffix = r#" - //- /core/lib.rs crate:core - pub mod option { - pub enum Option { Some(T), None } - } - pub mod iter { - pub trait Iterator { - type Item; - fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } - fn next(&mut self) -> Option; - } - pub struct FilterMap {} - impl Iterator for FilterMap { - type Item = i32; - fn next(&mut self) -> i32 { 7 } - } - } - "#; - format!("{}{}{}", prefix, body, suffix) - } - - #[test] - fn replace_filter_map_next_with_find_map2() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .map(|x| x + 2) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }); - let n = m.next(); - } - "#, - )); - } } diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index a2a4d61db..c480ed352 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -9,19 +9,13 @@ use hir_def::{ }; use hir_expand::name; use rustc_hash::FxHashSet; -use syntax::{ast, AstPtr}; use crate::{ db::HirDatabase, - diagnostics::{ - match_check::{ - self, - usefulness::{compute_match_usefulness, expand_pattern, MatchCheckCtx, PatternArena}, - }, - MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, - MissingPatFields, RemoveThisSemicolon, + diagnostics::match_check::{ + self, + usefulness::{compute_match_usefulness, expand_pattern, MatchCheckCtx, PatternArena}, }, - diagnostics_sink::DiagnosticSink, AdtId, InferenceResult, Interner, TyExt, TyKind, }; @@ -31,38 +25,81 @@ pub(crate) use hir_def::{ LocalFieldId, VariantId, }; -use super::ReplaceFilterMapNextWithFindMap; +pub enum BodyValidationDiagnostic { + RecordLiteralMissingFields { + record_expr: ExprId, + variant: VariantId, + missed_fields: Vec, + }, + RecordPatMissingFields { + record_pat: PatId, + variant: VariantId, + missed_fields: Vec, + }, + ReplaceFilterMapNextWithFindMap { + method_call_expr: ExprId, + }, + MismatchedArgCount { + call_expr: ExprId, + expected: usize, + found: usize, + }, + RemoveThisSemicolon { + expr: ExprId, + }, + MissingOkOrSomeInTailExpr { + expr: ExprId, + required: String, + }, + MissingMatchArms { + match_expr: ExprId, + }, + InternalBailedOut { + pat: PatId, + }, +} -pub(super) struct ExprValidator<'a, 'b: 'a> { +impl BodyValidationDiagnostic { + pub fn collect( + db: &dyn HirDatabase, + owner: DefWithBodyId, + internal_diagnostics: bool, + ) -> Vec { + let _p = profile::span("BodyValidationDiagnostic::collect"); + let infer = db.infer(owner); + let mut validator = ExprValidator::new(owner, infer.clone()); + validator.internal_diagnostics = internal_diagnostics; + validator.validate_body(db); + validator.diagnostics + } +} + +struct ExprValidator { owner: DefWithBodyId, infer: Arc, - sink: &'a mut DiagnosticSink<'b>, + pub(super) diagnostics: Vec, + internal_diagnostics: bool, } -impl<'a, 'b> ExprValidator<'a, 'b> { - pub(super) fn new( - owner: DefWithBodyId, - infer: Arc, - sink: &'a mut DiagnosticSink<'b>, - ) -> ExprValidator<'a, 'b> { - ExprValidator { owner, infer, sink } +impl ExprValidator { + fn new(owner: DefWithBodyId, infer: Arc) -> ExprValidator { + ExprValidator { owner, infer, diagnostics: Vec::new(), internal_diagnostics: false } } - pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { + fn validate_body(&mut self, db: &dyn HirDatabase) { self.check_for_filter_map_next(db); let body = db.body(self.owner); for (id, expr) in body.exprs.iter() { - if let Some((variant_def, missed_fields, true)) = + if let Some((variant, missed_fields, true)) = record_literal_missing_fields(db, &self.infer, id, expr) { - self.create_record_literal_missing_fields_diagnostic( - id, - db, - variant_def, + self.diagnostics.push(BodyValidationDiagnostic::RecordLiteralMissingFields { + record_expr: id, + variant, missed_fields, - ); + }); } match expr { @@ -76,15 +113,14 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } for (id, pat) in body.pats.iter() { - if let Some((variant_def, missed_fields, true)) = + if let Some((variant, missed_fields, true)) = record_pattern_missing_fields(db, &self.infer, id, pat) { - self.create_record_pattern_missing_fields_diagnostic( - id, - db, - variant_def, + self.diagnostics.push(BodyValidationDiagnostic::RecordPatMissingFields { + record_pat: id, + variant, missed_fields, - ); + }); } } let body_expr = &body[body.body_expr]; @@ -92,71 +128,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { if let Some(t) = tail { self.validate_results_in_tail_expr(body.body_expr, *t, db); } else if let Some(Statement::Expr { expr: id, .. }) = statements.last() { - self.validate_missing_tail_expr(body.body_expr, *id, db); - } - } - } - - fn create_record_literal_missing_fields_diagnostic( - &mut self, - id: ExprId, - db: &dyn HirDatabase, - variant_def: VariantId, - missed_fields: Vec, - ) { - // XXX: only look at source_map if we do have missing fields - let (_, source_map) = db.body_with_source_map(self.owner); - - if let Ok(source_ptr) = source_map.expr_syntax(id) { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root) { - if let Some(_) = record_expr.record_expr_field_list() { - let variant_data = variant_def.variant_data(db.upcast()); - let missed_fields = missed_fields - .into_iter() - .map(|idx| variant_data.fields()[idx].name.clone()) - .collect(); - self.sink.push(MissingFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(&record_expr), - field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)), - missed_fields, - }) - } - } - } - } - - fn create_record_pattern_missing_fields_diagnostic( - &mut self, - id: PatId, - db: &dyn HirDatabase, - variant_def: VariantId, - missed_fields: Vec, - ) { - // XXX: only look at source_map if we do have missing fields - let (_, source_map) = db.body_with_source_map(self.owner); - - if let Ok(source_ptr) = source_map.pat_syntax(id) { - if let Some(expr) = source_ptr.value.as_ref().left() { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) { - if let Some(_) = record_pat.record_pat_field_list() { - let variant_data = variant_def.variant_data(db.upcast()); - let missed_fields = missed_fields - .into_iter() - .map(|idx| variant_data.fields()[idx].name.clone()) - .collect(); - self.sink.push(MissingPatFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(&record_pat), - field_list_parent_path: record_pat - .path() - .map(|path| AstPtr::new(&path)), - missed_fields, - }) - } - } + self.validate_missing_tail_expr(body.body_expr, *id); } } } @@ -199,13 +171,11 @@ impl<'a, 'b> ExprValidator<'a, 'b> { if function_id == *next_function_id { if let Some(filter_map_id) = prev { if *receiver == filter_map_id { - let (_, source_map) = db.body_with_source_map(self.owner); - if let Ok(next_source_ptr) = source_map.expr_syntax(id) { - self.sink.push(ReplaceFilterMapNextWithFindMap { - file: next_source_ptr.file_id, - next_expr: next_source_ptr.value, - }); - } + self.diagnostics.push( + BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { + method_call_expr: id, + }, + ); } } } @@ -266,19 +236,15 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let mut arg_count = args.len(); if arg_count != param_count { - let (_, source_map) = db.body_with_source_map(self.owner); - if let Ok(source_ptr) = source_map.expr_syntax(call_id) { - if is_method_call { - param_count -= 1; - arg_count -= 1; - } - self.sink.push(MismatchedArgCount { - file: source_ptr.file_id, - call_expr: source_ptr.value, - expected: param_count, - found: arg_count, - }); + if is_method_call { + param_count -= 1; + arg_count -= 1; } + self.diagnostics.push(BodyValidationDiagnostic::MismatchedArgCount { + call_expr: call_id, + expected: param_count, + found: arg_count, + }); } } @@ -346,8 +312,9 @@ impl<'a, 'b> ExprValidator<'a, 'b> { // fit the match expression, we skip this diagnostic. Skipping the entire // diagnostic rather than just not including this match arm is preferred // to avoid the chance of false positives. - #[cfg(test)] - match_check::tests::report_bail_out(db, self.owner, arm.pat, self.sink); + if self.internal_diagnostics { + self.diagnostics.push(BodyValidationDiagnostic::InternalBailedOut { pat: arm.pat }) + } return; } @@ -382,20 +349,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { // FIXME Report witnesses // eprintln!("compute_match_usefulness(..) -> {:?}", &witnesses); if !witnesses.is_empty() { - if let Ok(source_ptr) = source_map.expr_syntax(id) { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::MatchExpr(match_expr) = &source_ptr.value.to_node(&root) { - if let (Some(match_expr), Some(arms)) = - (match_expr.expr(), match_expr.match_arm_list()) - { - self.sink.push(MissingMatchArms { - file: source_ptr.file_id, - match_expr: AstPtr::new(&match_expr), - arms: AstPtr::new(&arms), - }) - } - } - } + self.diagnostics.push(BodyValidationDiagnostic::MissingMatchArms { match_expr: id }); } } @@ -453,24 +407,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> { if params.len(&Interner) > 0 && params.at(&Interner, 0).ty(&Interner) == Some(&mismatch.actual) { - let (_, source_map) = db.body_with_source_map(self.owner); - - if let Ok(source_ptr) = source_map.expr_syntax(id) { - self.sink.push(MissingOkOrSomeInTailExpr { - file: source_ptr.file_id, - expr: source_ptr.value, - required, - }); - } + self.diagnostics + .push(BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr: id, required }); } } - fn validate_missing_tail_expr( - &mut self, - body_id: ExprId, - possible_tail_id: ExprId, - db: &dyn HirDatabase, - ) { + fn validate_missing_tail_expr(&mut self, body_id: ExprId, possible_tail_id: ExprId) { let mismatch = match self.infer.type_mismatch_for_expr(body_id) { Some(m) => m, None => return, @@ -485,12 +427,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { return; } - let (_, source_map) = db.body_with_source_map(self.owner); - - if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { - self.sink - .push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value }); - } + self.diagnostics + .push(BodyValidationDiagnostic::RemoveThisSemicolon { expr: possible_tail_id }); } } @@ -568,258 +506,3 @@ fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResul walk(pat, body, infer, &mut has_type_mismatches); !has_type_mismatches } - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_diagnostics; - - #[test] - fn simple_free_fn_zero() { - check_diagnostics( - r#" -fn zero() {} -fn f() { zero(1); } - //^^^^^^^ Expected 0 arguments, found 1 -"#, - ); - - check_diagnostics( - r#" -fn zero() {} -fn f() { zero(); } -"#, - ); - } - - #[test] - fn simple_free_fn_one() { - check_diagnostics( - r#" -fn one(arg: u8) {} -fn f() { one(); } - //^^^^^ Expected 1 argument, found 0 -"#, - ); - - check_diagnostics( - r#" -fn one(arg: u8) {} -fn f() { one(1); } -"#, - ); - } - - #[test] - fn method_as_fn() { - check_diagnostics( - r#" -struct S; -impl S { fn method(&self) {} } - -fn f() { - S::method(); -} //^^^^^^^^^^^ Expected 1 argument, found 0 -"#, - ); - - check_diagnostics( - r#" -struct S; -impl S { fn method(&self) {} } - -fn f() { - S::method(&S); - S.method(); -} -"#, - ); - } - - #[test] - fn method_with_arg() { - check_diagnostics( - r#" -struct S; -impl S { fn method(&self, arg: u8) {} } - - fn f() { - S.method(); - } //^^^^^^^^^^ Expected 1 argument, found 0 - "#, - ); - - check_diagnostics( - r#" -struct S; -impl S { fn method(&self, arg: u8) {} } - -fn f() { - S::method(&S, 0); - S.method(1); -} -"#, - ); - } - - #[test] - fn method_unknown_receiver() { - // note: this is incorrect code, so there might be errors on this in the - // future, but we shouldn't emit an argument count diagnostic here - check_diagnostics( - r#" -trait Foo { fn method(&self, arg: usize) {} } - -fn f() { - let x; - x.method(); -} -"#, - ); - } - - #[test] - fn tuple_struct() { - check_diagnostics( - r#" -struct Tup(u8, u16); -fn f() { - Tup(0); -} //^^^^^^ Expected 2 arguments, found 1 -"#, - ) - } - - #[test] - fn enum_variant() { - check_diagnostics( - r#" -enum En { Variant(u8, u16), } -fn f() { - En::Variant(0); -} //^^^^^^^^^^^^^^ Expected 2 arguments, found 1 -"#, - ) - } - - #[test] - fn enum_variant_type_macro() { - check_diagnostics( - r#" -macro_rules! Type { - () => { u32 }; -} -enum Foo { - Bar(Type![]) -} -impl Foo { - fn new() { - Foo::Bar(0); - Foo::Bar(0, 1); - //^^^^^^^^^^^^^^ Expected 1 argument, found 2 - Foo::Bar(); - //^^^^^^^^^^ Expected 1 argument, found 0 - } -} - "#, - ); - } - - #[test] - fn varargs() { - check_diagnostics( - r#" -extern "C" { - fn fixed(fixed: u8); - fn varargs(fixed: u8, ...); - fn varargs2(...); -} - -fn f() { - unsafe { - fixed(0); - fixed(0, 1); - //^^^^^^^^^^^ Expected 1 argument, found 2 - varargs(0); - varargs(0, 1); - varargs2(); - varargs2(0); - varargs2(0, 1); - } -} - "#, - ) - } - - #[test] - fn arg_count_lambda() { - check_diagnostics( - r#" -fn main() { - let f = |()| (); - f(); - //^^^ Expected 1 argument, found 0 - f(()); - f((), ()); - //^^^^^^^^^ Expected 1 argument, found 2 -} -"#, - ) - } - - #[test] - fn cfgd_out_call_arguments() { - check_diagnostics( - r#" -struct C(#[cfg(FALSE)] ()); -impl C { - fn new() -> Self { - Self( - #[cfg(FALSE)] - (), - ) - } - - fn method(&self) {} -} - -fn main() { - C::new().method(#[cfg(FALSE)] 0); -} - "#, - ); - } - - #[test] - fn cfgd_out_fn_params() { - check_diagnostics( - r#" -fn foo(#[cfg(NEVER)] x: ()) {} - -struct S; - -impl S { - fn method(#[cfg(NEVER)] self) {} - fn method2(#[cfg(NEVER)] self, arg: u8) {} - fn method3(self, #[cfg(NEVER)] arg: u8) {} -} - -extern "C" { - fn fixed(fixed: u8, #[cfg(NEVER)] ...); - fn varargs(#[cfg(not(NEVER))] ...); -} - -fn main() { - foo(); - S::method(); - S::method2(0); - S::method3(S); - S.method3(); - unsafe { - fixed(0); - varargs(1, 2, 3); - } -} - "#, - ) - } -} diff --git a/crates/hir_ty/src/diagnostics/match_check.rs b/crates/hir_ty/src/diagnostics/match_check.rs index c8e1b23de..a30e42699 100644 --- a/crates/hir_ty/src/diagnostics/match_check.rs +++ b/crates/hir_ty/src/diagnostics/match_check.rs @@ -364,960 +364,3 @@ impl PatternFoldable for PatKind { } } } - -#[cfg(test)] -pub(super) mod tests { - mod report { - use std::any::Any; - - use hir_def::{expr::PatId, DefWithBodyId}; - use hir_expand::{HirFileId, InFile}; - use syntax::SyntaxNodePtr; - - use crate::{ - db::HirDatabase, - diagnostics_sink::{Diagnostic, DiagnosticCode, DiagnosticSink}, - }; - - /// In tests, match check bails out loudly. - /// This helps to catch incorrect tests that pass due to false negatives. - pub(crate) fn report_bail_out( - db: &dyn HirDatabase, - def: DefWithBodyId, - pat: PatId, - sink: &mut DiagnosticSink, - ) { - let (_, source_map) = db.body_with_source_map(def); - if let Ok(source_ptr) = source_map.pat_syntax(pat) { - let pat_syntax_ptr = source_ptr.value.either(Into::into, Into::into); - sink.push(BailedOut { file: source_ptr.file_id, pat_syntax_ptr }); - } - } - - #[derive(Debug)] - struct BailedOut { - file: HirFileId, - pat_syntax_ptr: SyntaxNodePtr, - } - - impl Diagnostic for BailedOut { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("internal:match-check-bailed-out") - } - fn message(&self) -> String { - format!("Internal: match check bailed out") - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.pat_syntax_ptr.clone() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - } - } - - use crate::diagnostics::tests::check_diagnostics; - - pub(crate) use self::report::report_bail_out; - - #[test] - fn empty_tuple() { - check_diagnostics( - r#" -fn main() { - match () { } - //^^ Missing match arm - match (()) { } - //^^^^ Missing match arm - - match () { _ => (), } - match () { () => (), } - match (()) { (()) => (), } -} -"#, - ); - } - - #[test] - fn tuple_of_two_empty_tuple() { - check_diagnostics( - r#" -fn main() { - match ((), ()) { } - //^^^^^^^^ Missing match arm - - match ((), ()) { ((), ()) => (), } -} -"#, - ); - } - - #[test] - fn boolean() { - check_diagnostics( - r#" -fn test_main() { - match false { } - //^^^^^ Missing match arm - match false { true => (), } - //^^^^^ Missing match arm - match (false, true) {} - //^^^^^^^^^^^^^ Missing match arm - match (false, true) { (true, true) => (), } - //^^^^^^^^^^^^^ Missing match arm - match (false, true) { - //^^^^^^^^^^^^^ Missing match arm - (false, true) => (), - (false, false) => (), - (true, false) => (), - } - match (false, true) { (true, _x) => (), } - //^^^^^^^^^^^^^ Missing match arm - - match false { true => (), false => (), } - match (false, true) { - (false, _) => (), - (true, false) => (), - (_, true) => (), - } - match (false, true) { - (true, true) => (), - (true, false) => (), - (false, true) => (), - (false, false) => (), - } - match (false, true) { - (true, _x) => (), - (false, true) => (), - (false, false) => (), - } - match (false, true, false) { - (false, ..) => (), - (true, ..) => (), - } - match (false, true, false) { - (.., false) => (), - (.., true) => (), - } - match (false, true, false) { (..) => (), } -} -"#, - ); - } - - #[test] - fn tuple_of_tuple_and_bools() { - check_diagnostics( - r#" -fn main() { - match (false, ((), false)) {} - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - match (false, ((), false)) { (true, ((), true)) => (), } - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - match (false, ((), false)) { (true, _) => (), } - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - - match (false, ((), false)) { - (true, ((), true)) => (), - (true, ((), false)) => (), - (false, ((), true)) => (), - (false, ((), false)) => (), - } - match (false, ((), false)) { - (true, ((), true)) => (), - (true, ((), false)) => (), - (false, _) => (), - } -} -"#, - ); - } - - #[test] - fn enums() { - check_diagnostics( - r#" -enum Either { A, B, } - -fn main() { - match Either::A { } - //^^^^^^^^^ Missing match arm - match Either::B { Either::A => (), } - //^^^^^^^^^ Missing match arm - - match &Either::B { - //^^^^^^^^^^ Missing match arm - Either::A => (), - } - - match Either::B { - Either::A => (), Either::B => (), - } - match &Either::B { - Either::A => (), Either::B => (), - } -} -"#, - ); - } - - #[test] - fn enum_containing_bool() { - check_diagnostics( - r#" -enum Either { A(bool), B } - -fn main() { - match Either::B { } - //^^^^^^^^^ Missing match arm - match Either::B { - //^^^^^^^^^ Missing match arm - Either::A(true) => (), Either::B => () - } - - match Either::B { - Either::A(true) => (), - Either::A(false) => (), - Either::B => (), - } - match Either::B { - Either::B => (), - _ => (), - } - match Either::B { - Either::A(_) => (), - Either::B => (), - } - -} - "#, - ); - } - - #[test] - fn enum_different_sizes() { - check_diagnostics( - r#" -enum Either { A(bool), B(bool, bool) } - -fn main() { - match Either::A(false) { - //^^^^^^^^^^^^^^^^ Missing match arm - Either::A(_) => (), - Either::B(false, _) => (), - } - - match Either::A(false) { - Either::A(_) => (), - Either::B(true, _) => (), - Either::B(false, _) => (), - } - match Either::A(false) { - Either::A(true) | Either::A(false) => (), - Either::B(true, _) => (), - Either::B(false, _) => (), - } -} -"#, - ); - } - - #[test] - fn tuple_of_enum_no_diagnostic() { - check_diagnostics( - r#" -enum Either { A(bool), B(bool, bool) } -enum Either2 { C, D } - -fn main() { - match (Either::A(false), Either2::C) { - (Either::A(true), _) | (Either::A(false), _) => (), - (Either::B(true, _), Either2::C) => (), - (Either::B(false, _), Either2::C) => (), - (Either::B(_, _), Either2::D) => (), - } -} -"#, - ); - } - - #[test] - fn or_pattern_no_diagnostic() { - check_diagnostics( - r#" -enum Either {A, B} - -fn main() { - match (Either::A, Either::B) { - (Either::A | Either::B, _) => (), - } -}"#, - ) - } - - #[test] - fn mismatched_types() { - // Match statements with arms that don't match the - // expression pattern do not fire this diagnostic. - check_diagnostics( - r#" -enum Either { A, B } -enum Either2 { C, D } - -fn main() { - match Either::A { - Either2::C => (), - // ^^^^^^^^^^ Internal: match check bailed out - Either2::D => (), - } - match (true, false) { - (true, false, true) => (), - // ^^^^^^^^^^^^^^^^^^^ Internal: match check bailed out - (true) => (), - } - match (true, false) { (true,) => {} } - // ^^^^^^^ Internal: match check bailed out - match (0) { () => () } - // ^^ Internal: match check bailed out - match Unresolved::Bar { Unresolved::Baz => () } -} - "#, - ); - } - - #[test] - fn mismatched_types_in_or_patterns() { - check_diagnostics( - r#" -fn main() { - match false { true | () => {} } - // ^^^^^^^^^ Internal: match check bailed out - match (false,) { (true | (),) => {} } - // ^^^^^^^^^^^^ Internal: match check bailed out -} -"#, - ); - } - - #[test] - fn malformed_match_arm_tuple_enum_missing_pattern() { - // We are testing to be sure we don't panic here when the match - // arm `Either::B` is missing its pattern. - check_diagnostics( - r#" -enum Either { A, B(u32) } - -fn main() { - match Either::A { - Either::A => (), - Either::B() => (), - } -} -"#, - ); - } - - #[test] - fn malformed_match_arm_extra_fields() { - check_diagnostics( - r#" -enum A { B(isize, isize), C } -fn main() { - match A::B(1, 2) { - A::B(_, _, _) => (), - // ^^^^^^^^^^^^^ Internal: match check bailed out - } - match A::B(1, 2) { - A::C(_) => (), - // ^^^^^^^ Internal: match check bailed out - } -} -"#, - ); - } - - #[test] - fn expr_diverges() { - check_diagnostics( - r#" -enum Either { A, B } - -fn main() { - match loop {} { - Either::A => (), - // ^^^^^^^^^ Internal: match check bailed out - Either::B => (), - } - match loop {} { - Either::A => (), - // ^^^^^^^^^ Internal: match check bailed out - } - match loop { break Foo::A } { - //^^^^^^^^^^^^^^^^^^^^^ Missing match arm - Either::A => (), - } - match loop { break Foo::A } { - Either::A => (), - Either::B => (), - } -} -"#, - ); - } - - #[test] - fn expr_partially_diverges() { - check_diagnostics( - r#" -enum Either { A(T), B } - -fn foo() -> Either { Either::B } -fn main() -> u32 { - match foo() { - Either::A(val) => val, - Either::B => 0, - } -} -"#, - ); - } - - #[test] - fn enum_record() { - check_diagnostics( - r#" -enum Either { A { foo: bool }, B } - -fn main() { - let a = Either::A { foo: true }; - match a { } - //^ Missing match arm - match a { Either::A { foo: true } => () } - //^ Missing match arm - match a { - Either::A { } => (), - //^^^^^^^^^ Missing structure fields: - // | - foo - Either::B => (), - } - match a { - //^ Missing match arm - Either::A { } => (), - } //^^^^^^^^^ Missing structure fields: - // | - foo - - match a { - Either::A { foo: true } => (), - Either::A { foo: false } => (), - Either::B => (), - } - match a { - Either::A { foo: _ } => (), - Either::B => (), - } -} -"#, - ); - } - - #[test] - fn enum_record_fields_out_of_order() { - check_diagnostics( - r#" -enum Either { - A { foo: bool, bar: () }, - B, -} - -fn main() { - let a = Either::A { foo: true, bar: () }; - match a { - //^ Missing match arm - Either::A { bar: (), foo: false } => (), - Either::A { foo: true, bar: () } => (), - } - - match a { - Either::A { bar: (), foo: false } => (), - Either::A { foo: true, bar: () } => (), - Either::B => (), - } -} -"#, - ); - } - - #[test] - fn enum_record_ellipsis() { - check_diagnostics( - r#" -enum Either { - A { foo: bool, bar: bool }, - B, -} - -fn main() { - let a = Either::B; - match a { - //^ Missing match arm - Either::A { foo: true, .. } => (), - Either::B => (), - } - match a { - //^ Missing match arm - Either::A { .. } => (), - } - - match a { - Either::A { foo: true, .. } => (), - Either::A { foo: false, .. } => (), - Either::B => (), - } - - match a { - Either::A { .. } => (), - Either::B => (), - } -} -"#, - ); - } - - #[test] - fn enum_tuple_partial_ellipsis() { - check_diagnostics( - r#" -enum Either { - A(bool, bool, bool, bool), - B, -} - -fn main() { - match Either::B { - //^^^^^^^^^ Missing match arm - Either::A(true, .., true) => (), - Either::A(true, .., false) => (), - Either::A(false, .., false) => (), - Either::B => (), - } - match Either::B { - //^^^^^^^^^ Missing match arm - Either::A(true, .., true) => (), - Either::A(true, .., false) => (), - Either::A(.., true) => (), - Either::B => (), - } - - match Either::B { - Either::A(true, .., true) => (), - Either::A(true, .., false) => (), - Either::A(false, .., true) => (), - Either::A(false, .., false) => (), - Either::B => (), - } - match Either::B { - Either::A(true, .., true) => (), - Either::A(true, .., false) => (), - Either::A(.., true) => (), - Either::A(.., false) => (), - Either::B => (), - } -} -"#, - ); - } - - #[test] - fn never() { - check_diagnostics( - r#" -enum Never {} - -fn enum_(never: Never) { - match never {} -} -fn enum_ref(never: &Never) { - match never {} - //^^^^^ Missing match arm -} -fn bang(never: !) { - match never {} -} -"#, - ); - } - - #[test] - fn unknown_type() { - check_diagnostics( - r#" -enum Option { Some(T), None } - -fn main() { - // `Never` is deliberately not defined so that it's an uninferred type. - match Option::::None { - None => (), - Some(never) => match never {}, - // ^^^^^^^^^^^ Internal: match check bailed out - } - match Option::::None { - //^^^^^^^^^^^^^^^^^^^^^ Missing match arm - Option::Some(_never) => {}, - } -} -"#, - ); - } - - #[test] - fn tuple_of_bools_with_ellipsis_at_end_missing_arm() { - check_diagnostics( - r#" -fn main() { - match (false, true, false) { - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - (false, ..) => (), - } -}"#, - ); - } - - #[test] - fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() { - check_diagnostics( - r#" -fn main() { - match (false, true, false) { - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - (.., false) => (), - } -}"#, - ); - } - - #[test] - fn tuple_of_bools_with_ellipsis_in_middle_missing_arm() { - check_diagnostics( - r#" -fn main() { - match (false, true, false) { - //^^^^^^^^^^^^^^^^^^^^ Missing match arm - (true, .., false) => (), - } -}"#, - ); - } - - #[test] - fn record_struct() { - check_diagnostics( - r#"struct Foo { a: bool } -fn main(f: Foo) { - match f {} - //^ Missing match arm - match f { Foo { a: true } => () } - //^ Missing match arm - match &f { Foo { a: true } => () } - //^^ Missing match arm - match f { Foo { a: _ } => () } - match f { - Foo { a: true } => (), - Foo { a: false } => (), - } - match &f { - Foo { a: true } => (), - Foo { a: false } => (), - } -} -"#, - ); - } - - #[test] - fn tuple_struct() { - check_diagnostics( - r#"struct Foo(bool); -fn main(f: Foo) { - match f {} - //^ Missing match arm - match f { Foo(true) => () } - //^ Missing match arm - match f { - Foo(true) => (), - Foo(false) => (), - } -} -"#, - ); - } - - #[test] - fn unit_struct() { - check_diagnostics( - r#"struct Foo; -fn main(f: Foo) { - match f {} - //^ Missing match arm - match f { Foo => () } -} -"#, - ); - } - - #[test] - fn record_struct_ellipsis() { - check_diagnostics( - r#"struct Foo { foo: bool, bar: bool } -fn main(f: Foo) { - match f { Foo { foo: true, .. } => () } - //^ Missing match arm - match f { - //^ Missing match arm - Foo { foo: true, .. } => (), - Foo { bar: false, .. } => () - } - match f { Foo { .. } => () } - match f { - Foo { foo: true, .. } => (), - Foo { foo: false, .. } => () - } -} -"#, - ); - } - - #[test] - fn internal_or() { - check_diagnostics( - r#" -fn main() { - enum Either { A(bool), B } - match Either::B { - //^^^^^^^^^ Missing match arm - Either::A(true | false) => (), - } -} -"#, - ); - } - - #[test] - fn no_panic_at_unimplemented_subpattern_type() { - check_diagnostics( - r#" -struct S { a: char} -fn main(v: S) { - match v { S{ a } => {} } - match v { S{ a: _x } => {} } - match v { S{ a: 'a' } => {} } - //^^^^^^^^^^^ Internal: match check bailed out - match v { S{..} => {} } - match v { _ => {} } - match v { } - //^ Missing match arm -} -"#, - ); - } - - #[test] - fn binding() { - check_diagnostics( - r#" -fn main() { - match true { - _x @ true => {} - false => {} - } - match true { _x @ true => {} } - //^^^^ Missing match arm -} -"#, - ); - } - - #[test] - fn binding_ref_has_correct_type() { - // Asserts `PatKind::Binding(ref _x): bool`, not &bool. - // If that's not true match checking will panic with "incompatible constructors" - // FIXME: make facilities to test this directly like `tests::check_infer(..)` - check_diagnostics( - r#" -enum Foo { A } -fn main() { - // FIXME: this should not bail out but current behavior is such as the old algorithm. - // ExprValidator::validate_match(..) checks types of top level patterns incorrecly. - match Foo::A { - ref _x => {} - // ^^^^^^ Internal: match check bailed out - Foo::A => {} - } - match (true,) { - (ref _x,) => {} - (true,) => {} - } -} -"#, - ); - } - - #[test] - fn enum_non_exhaustive() { - check_diagnostics( - r#" -//- /lib.rs crate:lib -#[non_exhaustive] -pub enum E { A, B } -fn _local() { - match E::A { _ => {} } - match E::A { - E::A => {} - E::B => {} - } - match E::A { - E::A | E::B => {} - } -} - -//- /main.rs crate:main deps:lib -use lib::E; -fn main() { - match E::A { _ => {} } - match E::A { - //^^^^ Missing match arm - E::A => {} - E::B => {} - } - match E::A { - //^^^^ Missing match arm - E::A | E::B => {} - } -} -"#, - ); - } - - #[test] - fn match_guard() { - check_diagnostics( - r#" -fn main() { - match true { - true if false => {} - true => {} - false => {} - } - match true { - //^^^^ Missing match arm - true if false => {} - false => {} -} -"#, - ); - } - - #[test] - fn pattern_type_is_of_substitution() { - cov_mark::check!(match_check_wildcard_expanded_to_substitutions); - check_diagnostics( - r#" -struct Foo(T); -struct Bar; -fn main() { - match Foo(Bar) { - _ | Foo(Bar) => {} - } -} -"#, - ); - } - - #[test] - fn record_struct_no_such_field() { - check_diagnostics( - r#" -struct Foo { } -fn main(f: Foo) { - match f { Foo { bar } => () } - // ^^^^^^^^^^^ Internal: match check bailed out -} -"#, - ); - } - - #[test] - fn match_ergonomics_issue_9095() { - check_diagnostics( - r#" -enum Foo { A(T) } -fn main() { - match &Foo::A(true) { - _ => {} - Foo::A(_) => {} - } -} -"#, - ); - } - - 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::*; - - #[test] - fn integers() { - // We don't currently check integer exhaustiveness. - check_diagnostics( - r#" -fn main() { - match 5 { - 10 => (), - // ^^ Internal: match check bailed out - 11..20 => (), - } -} -"#, - ); - } - - #[test] - fn reference_patterns_at_top_level() { - check_diagnostics( - r#" -fn main() { - match &false { - &true => {} - // ^^^^^ Internal: match check bailed out - } -} - "#, - ); - } - - #[test] - fn reference_patterns_in_fields() { - check_diagnostics( - r#" -fn main() { - match (&false,) { - (true,) => {} - // ^^^^^^^ Internal: match check bailed out - } - match (&false,) { - (&true,) => {} - // ^^^^^^^^ Internal: match check bailed out - } -} - "#, - ); - } - } -} diff --git a/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs b/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs index 1f4219b42..222141bd6 100644 --- a/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs +++ b/crates/hir_ty/src/diagnostics/match_check/deconstruct_pat.rs @@ -664,7 +664,6 @@ impl Fields { let is_non_exhaustive = is_field_list_non_exhaustive(variant_id, cx) && !adt_is_local; - cov_mark::hit!(match_check_wildcard_expanded_to_substitutions); let field_ty_data = cx.db.field_types(variant_id); let field_tys = || { field_ty_data -- cgit v1.2.3