From 935c53b92eea7c288b781ecd68436c9733ec8a83 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 21:55:51 +0300 Subject: internal: use cov-mark rather than bailing out diagnostic --- crates/hir/src/diagnostics.rs | 23 ------ crates/hir/src/lib.rs | 28 ++----- crates/hir_ty/src/diagnostics/expr.rs | 17 +--- crates/ide/src/diagnostics.rs | 3 +- crates/ide/src/diagnostics/missing_match_arms.rs | 99 +++++++++++++----------- 5 files changed, 62 insertions(+), 108 deletions(-) diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 5cffef47f..1f6a70006 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -3,8 +3,6 @@ //! //! This probably isn't the best way to do this -- ideally, diagnistics should //! be expressed in terms of hir types themselves. -use std::any::Any; - use cfg::{CfgExpr, CfgOptions}; use either::Either; use hir_def::path::ModPath; @@ -157,25 +155,4 @@ pub struct MissingMatchArms { pub arms: AstPtr, } -#[derive(Debug)] -pub struct InternalBailedOut { - pub file: HirFileId, - pub pat_syntax_ptr: SyntaxNodePtr, -} - -impl Diagnostic for InternalBailedOut { - 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 - } -} - pub use hir_ty::diagnostics::IncorrectCase; diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2e794ff4a..7f689cd41 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -86,8 +86,8 @@ use crate::{ pub use crate::{ attrs::{HasAttrs, Namespace}, diagnostics::{ - AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InternalBailedOut, - MacroError, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, + AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, MacroError, + MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro, @@ -461,7 +461,6 @@ impl Module { self, db: &dyn HirDatabase, sink: &mut DiagnosticSink, - internal_diagnostics: bool, ) -> Vec { let _p = profile::span("Module::diagnostics").detail(|| { format!("{:?}", self.name(db).map_or("".into(), |name| name.to_string())) @@ -619,11 +618,11 @@ impl Module { } for decl in self.declarations(db) { match decl { - ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink, internal_diagnostics)), + ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink)), ModuleDef::Module(m) => { // Only add diagnostics from inline modules if def_map[m.id.local_id].origin.is_inline() { - acc.extend(m.diagnostics(db, sink, internal_diagnostics)) + acc.extend(m.diagnostics(db, sink)) } } _ => acc.extend(decl.diagnostics(db)), @@ -633,7 +632,7 @@ impl Module { for impl_def in self.impl_defs(db) { for item in impl_def.items(db) { if let AssocItem::Function(f) = item { - acc.extend(f.diagnostics(db, sink, internal_diagnostics)); + acc.extend(f.diagnostics(db, sink)); } } } @@ -1040,7 +1039,6 @@ impl Function { self, db: &dyn HirDatabase, sink: &mut DiagnosticSink, - internal_diagnostics: bool, ) -> Vec { let mut acc: Vec = Vec::new(); let krate = self.module(db).id.krate(); @@ -1100,9 +1098,7 @@ impl Function { } } - for diagnostic in - BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics) - { + for diagnostic in BodyValidationDiagnostic::collect(db, self.id.into()) { match diagnostic { BodyValidationDiagnostic::RecordMissingFields { record, @@ -1223,18 +1219,6 @@ impl Function { Err(SyntheticSyntax) => (), } } - BodyValidationDiagnostic::InternalBailedOut { pat } => { - match source_map.pat_syntax(pat) { - Ok(source_ptr) => { - let pat_syntax_ptr = source_ptr.value.either(Into::into, Into::into); - sink.push(InternalBailedOut { - file: source_ptr.file_id, - pat_syntax_ptr, - }); - } - Err(SyntheticSyntax) => (), - } - } } } diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 2a211fd8e..b809b96a0 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -50,21 +50,13 @@ pub enum BodyValidationDiagnostic { MissingMatchArms { match_expr: ExprId, }, - InternalBailedOut { - pat: PatId, - }, } impl BodyValidationDiagnostic { - pub fn collect( - db: &dyn HirDatabase, - owner: DefWithBodyId, - internal_diagnostics: bool, - ) -> Vec { + pub fn collect(db: &dyn HirDatabase, owner: DefWithBodyId) -> 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 } @@ -74,12 +66,11 @@ struct ExprValidator { owner: DefWithBodyId, infer: Arc, pub(super) diagnostics: Vec, - internal_diagnostics: bool, } impl ExprValidator { fn new(owner: DefWithBodyId, infer: Arc) -> ExprValidator { - ExprValidator { owner, infer, diagnostics: Vec::new(), internal_diagnostics: false } + ExprValidator { owner, infer, diagnostics: Vec::new() } } fn validate_body(&mut self, db: &dyn HirDatabase) { @@ -308,9 +299,7 @@ impl ExprValidator { // 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. - if self.internal_diagnostics { - self.diagnostics.push(BodyValidationDiagnostic::InternalBailedOut { pat: arm.pat }) - } + cov_mark::hit!(validate_match_bailed_out); return; } diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 01b68232e..fe6236e44 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -181,10 +181,9 @@ pub(crate) fn diagnostics( }); let mut diags = Vec::new(); - let internal_diagnostics = cfg!(test); let module = sema.to_module_def(file_id); if let Some(m) = module { - diags = m.diagnostics(db, &mut sink, internal_diagnostics) + diags = m.diagnostics(db, &mut sink) } drop(sink); diff --git a/crates/ide/src/diagnostics/missing_match_arms.rs b/crates/ide/src/diagnostics/missing_match_arms.rs index 6b977fa59..b636489b3 100644 --- a/crates/ide/src/diagnostics/missing_match_arms.rs +++ b/crates/ide/src/diagnostics/missing_match_arms.rs @@ -20,9 +20,14 @@ pub(super) fn missing_match_arms( pub(super) mod tests { use crate::diagnostics::tests::check_diagnostics; + fn check_diagnostics_no_bails(ra_fixture: &str) { + cov_mark::check_count!(validate_match_bailed_out, 0); + crate::diagnostics::tests::check_diagnostics(ra_fixture) + } + #[test] fn empty_tuple() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match () { } @@ -40,7 +45,7 @@ fn main() { #[test] fn tuple_of_two_empty_tuple() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match ((), ()) { } @@ -54,7 +59,7 @@ fn main() { #[test] fn boolean() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn test_main() { match false { } @@ -107,7 +112,7 @@ fn test_main() { #[test] fn tuple_of_tuple_and_bools() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match (false, ((), false)) {} @@ -135,7 +140,7 @@ fn main() { #[test] fn enums() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A, B, } @@ -163,7 +168,7 @@ fn main() { #[test] fn enum_containing_bool() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A(bool), B } @@ -196,7 +201,7 @@ fn main() { #[test] fn enum_different_sizes() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A(bool), B(bool, bool) } @@ -224,7 +229,7 @@ fn main() { #[test] fn tuple_of_enum_no_diagnostic() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A(bool), B(bool, bool) } enum Either2 { C, D } @@ -243,7 +248,7 @@ fn main() { #[test] fn or_pattern_no_diagnostic() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either {A, B} @@ -257,6 +262,7 @@ fn main() { #[test] fn mismatched_types() { + cov_mark::check_count!(validate_match_bailed_out, 4); // Match statements with arms that don't match the // expression pattern do not fire this diagnostic. check_diagnostics( @@ -267,18 +273,14 @@ 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 => () } } "#, @@ -287,13 +289,12 @@ fn main() { #[test] fn mismatched_types_in_or_patterns() { + cov_mark::check_count!(validate_match_bailed_out, 2); check_diagnostics( r#" fn main() { match false { true | () => {} } - // ^^^^^^^^^ Internal: match check bailed out match (false,) { (true | (),) => {} } - // ^^^^^^^^^^^^ Internal: match check bailed out } "#, ); @@ -303,7 +304,7 @@ fn main() { 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( + check_diagnostics_no_bails( r#" enum Either { A, B(u32) } @@ -319,17 +320,16 @@ fn main() { #[test] fn malformed_match_arm_extra_fields() { + cov_mark::check_count!(validate_match_bailed_out, 2); 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 } } "#, @@ -338,6 +338,7 @@ fn main() { #[test] fn expr_diverges() { + cov_mark::check_count!(validate_match_bailed_out, 2); check_diagnostics( r#" enum Either { A, B } @@ -345,12 +346,10 @@ 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 @@ -367,7 +366,7 @@ fn main() { #[test] fn expr_partially_diverges() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A(T), B } @@ -384,7 +383,7 @@ fn main() -> u32 { #[test] fn enum_record() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A { foo: bool }, B } @@ -422,7 +421,7 @@ fn main() { #[test] fn enum_record_fields_out_of_order() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A { foo: bool, bar: () }, @@ -449,7 +448,7 @@ fn main() { #[test] fn enum_record_ellipsis() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A { foo: bool, bar: bool }, @@ -485,7 +484,7 @@ fn main() { #[test] fn enum_tuple_partial_ellipsis() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Either { A(bool, bool, bool, bool), @@ -529,7 +528,7 @@ fn main() { #[test] fn never() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Never {} @@ -549,6 +548,8 @@ fn bang(never: !) { #[test] fn unknown_type() { + cov_mark::check_count!(validate_match_bailed_out, 1); + check_diagnostics( r#" enum Option { Some(T), None } @@ -558,7 +559,6 @@ fn main() { match Option::::None { None => (), Some(never) => match never {}, - // ^^^^^^^^^^^ Internal: match check bailed out } match Option::::None { //^^^^^^^^^^^^^^^^^^^^^ missing match arm @@ -571,7 +571,7 @@ fn main() { #[test] fn tuple_of_bools_with_ellipsis_at_end_missing_arm() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match (false, true, false) { @@ -584,7 +584,7 @@ fn main() { #[test] fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match (false, true, false) { @@ -597,7 +597,7 @@ fn main() { #[test] fn tuple_of_bools_with_ellipsis_in_middle_missing_arm() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match (false, true, false) { @@ -610,7 +610,7 @@ fn main() { #[test] fn record_struct() { - check_diagnostics( + check_diagnostics_no_bails( r#"struct Foo { a: bool } fn main(f: Foo) { match f {} @@ -635,7 +635,7 @@ fn main(f: Foo) { #[test] fn tuple_struct() { - check_diagnostics( + check_diagnostics_no_bails( r#"struct Foo(bool); fn main(f: Foo) { match f {} @@ -653,7 +653,7 @@ fn main(f: Foo) { #[test] fn unit_struct() { - check_diagnostics( + check_diagnostics_no_bails( r#"struct Foo; fn main(f: Foo) { match f {} @@ -666,7 +666,7 @@ fn main(f: Foo) { #[test] fn record_struct_ellipsis() { - check_diagnostics( + check_diagnostics_no_bails( r#"struct Foo { foo: bool, bar: bool } fn main(f: Foo) { match f { Foo { foo: true, .. } => () } @@ -688,7 +688,7 @@ fn main(f: Foo) { #[test] fn internal_or() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { enum Either { A(bool), B } @@ -703,6 +703,8 @@ fn main() { #[test] fn no_panic_at_unimplemented_subpattern_type() { + cov_mark::check_count!(validate_match_bailed_out, 1); + check_diagnostics( r#" struct S { a: char} @@ -710,7 +712,6 @@ 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 { } @@ -722,7 +723,7 @@ fn main(v: S) { #[test] fn binding() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match true { @@ -738,6 +739,8 @@ fn main() { #[test] fn binding_ref_has_correct_type() { + cov_mark::check_count!(validate_match_bailed_out, 1); + // 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(..)` @@ -749,7 +752,6 @@ fn main() { // ExprValidator::validate_match(..) checks types of top level patterns incorrecly. match Foo::A { ref _x => {} - // ^^^^^^ Internal: match check bailed out Foo::A => {} } match (true,) { @@ -763,7 +765,7 @@ fn main() { #[test] fn enum_non_exhaustive() { - check_diagnostics( + check_diagnostics_no_bails( r#" //- /lib.rs crate:lib #[non_exhaustive] @@ -799,7 +801,7 @@ fn main() { #[test] fn match_guard() { - check_diagnostics( + check_diagnostics_no_bails( r#" fn main() { match true { @@ -820,7 +822,7 @@ fn main() { #[test] fn pattern_type_is_of_substitution() { cov_mark::check!(match_check_wildcard_expanded_to_substitutions); - check_diagnostics( + check_diagnostics_no_bails( r#" struct Foo(T); struct Bar; @@ -835,12 +837,13 @@ fn main() { #[test] fn record_struct_no_such_field() { + cov_mark::check_count!(validate_match_bailed_out, 1); + check_diagnostics( r#" struct Foo { } fn main(f: Foo) { match f { Foo { bar } => () } - // ^^^^^^^^^^^ Internal: match check bailed out } "#, ); @@ -848,7 +851,7 @@ fn main(f: Foo) { #[test] fn match_ergonomics_issue_9095() { - check_diagnostics( + check_diagnostics_no_bails( r#" enum Foo { A(T) } fn main() { @@ -875,13 +878,14 @@ fn main() { #[test] fn integers() { + cov_mark::check_count!(validate_match_bailed_out, 1); + // We don't currently check integer exhaustiveness. check_diagnostics( r#" fn main() { match 5 { 10 => (), - // ^^ Internal: match check bailed out 11..20 => (), } } @@ -891,12 +895,13 @@ fn main() { #[test] fn reference_patterns_at_top_level() { + cov_mark::check_count!(validate_match_bailed_out, 1); + check_diagnostics( r#" fn main() { match &false { &true => {} - // ^^^^^ Internal: match check bailed out } } "#, @@ -905,16 +910,16 @@ fn main() { #[test] fn reference_patterns_in_fields() { + cov_mark::check_count!(validate_match_bailed_out, 2); + check_diagnostics( r#" fn main() { match (&false,) { (true,) => {} - // ^^^^^^^ Internal: match check bailed out } match (&false,) { (&true,) => {} - // ^^^^^^^^ Internal: match check bailed out } } "#, -- cgit v1.2.3