From b292e1b9da39813e2739cb450c263e7502c97c8d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 21:44:31 +0300 Subject: internal: refactor missing match arms diagnostics --- crates/hir/src/diagnostics.rs | 19 +------------------ crates/hir/src/lib.rs | 13 ++++++++----- 2 files changed, 9 insertions(+), 23 deletions(-) (limited to 'crates/hir') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index c2d608eb5..5cffef47f 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -38,6 +38,7 @@ diagnostics![ MacroError, MismatchedArgCount, MissingFields, + MissingMatchArms, MissingOkOrSomeInTailExpr, MissingUnsafe, NoSuchField, @@ -149,9 +150,6 @@ pub struct MissingOkOrSomeInTailExpr { pub required: String, } -// 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, @@ -159,21 +157,6 @@ pub struct MissingMatchArms { 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 - } -} - #[derive(Debug)] pub struct InternalBailedOut { pub file: HirFileId, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index fc147ade3..2e794ff4a 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1209,11 +1209,14 @@ impl Function { if let (Some(match_expr), Some(arms)) = (match_expr.expr(), match_expr.match_arm_list()) { - sink.push(MissingMatchArms { - file: source_ptr.file_id, - match_expr: AstPtr::new(&match_expr), - arms: AstPtr::new(&arms), - }) + acc.push( + MissingMatchArms { + file: source_ptr.file_id, + match_expr: AstPtr::new(&match_expr), + arms: AstPtr::new(&arms), + } + .into(), + ) } } } -- cgit v1.2.3 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 ++++++---------------------- 2 files changed, 6 insertions(+), 45 deletions(-) (limited to 'crates/hir') 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) => (), - } - } } } -- cgit v1.2.3 From ff52167c9a8dd6f99a56a35eae8d634d0ddf1286 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 22:05:47 +0300 Subject: internal: kill diagnostic sink --- crates/hir/src/diagnostics.rs | 4 -- crates/hir/src/diagnostics_sink.rs | 109 ------------------------------------- crates/hir/src/lib.rs | 28 ++-------- 3 files changed, 6 insertions(+), 135 deletions(-) delete mode 100644 crates/hir/src/diagnostics_sink.rs (limited to 'crates/hir') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 1f6a70006..b4c505898 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -9,10 +9,6 @@ use hir_def::path::ModPath; use hir_expand::{name::Name, HirFileId, InFile}; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; -pub use crate::diagnostics_sink::{ - Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, -}; - macro_rules! diagnostics { ($($diag:ident,)*) => { pub enum AnyDiagnostic {$( diff --git a/crates/hir/src/diagnostics_sink.rs b/crates/hir/src/diagnostics_sink.rs deleted file mode 100644 index 084fa8b06..000000000 --- a/crates/hir/src/diagnostics_sink.rs +++ /dev/null @@ -1,109 +0,0 @@ -//! Semantic errors and warnings. -//! -//! The `Diagnostic` trait defines a trait object which can represent any -//! diagnostic. -//! -//! `DiagnosticSink` struct is used as an emitter for diagnostic. When creating -//! a `DiagnosticSink`, you supply a callback which can react to a `dyn -//! Diagnostic` or to any concrete diagnostic (downcasting is used internally). -//! -//! Because diagnostics store file offsets, it's a bad idea to store them -//! directly in salsa. For this reason, every hir subsytem defines it's own -//! strongly-typed closed set of diagnostics which use hir ids internally, are -//! stored in salsa and do *not* implement the `Diagnostic` trait. Instead, a -//! subsystem provides a separate, non-query-based API which can walk all stored -//! values and transform them into instances of `Diagnostic`. - -use std::{any::Any, fmt}; - -use hir_expand::InFile; -use syntax::SyntaxNodePtr; - -#[derive(Copy, Clone, Debug, PartialEq)] -pub struct DiagnosticCode(pub &'static str); - -impl DiagnosticCode { - pub fn as_str(&self) -> &str { - self.0 - } -} - -pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn code(&self) -> DiagnosticCode; - fn message(&self) -> String; - /// Source element that triggered the diagnostics. - /// - /// Note that this should reflect "semantics", rather than specific span we - /// want to highlight. When rendering the diagnostics into an error message, - /// the IDE will fetch the `SyntaxNode` and will narrow the span - /// appropriately. - fn display_source(&self) -> InFile; - fn as_any(&self) -> &(dyn Any + Send + 'static); - fn is_experimental(&self) -> bool { - false - } -} - -pub struct DiagnosticSink<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, - default_callback: Box, -} - -impl<'a> DiagnosticSink<'a> { - pub fn push(&mut self, d: impl Diagnostic) { - let d: &dyn Diagnostic = &d; - self._push(d); - } - - fn _push(&mut self, d: &dyn Diagnostic) { - for filter in &mut self.filters { - if !filter(d) { - return; - } - } - for cb in &mut self.callbacks { - match cb(d) { - Ok(()) => return, - Err(()) => (), - } - } - (self.default_callback)(d) - } -} - -pub struct DiagnosticSinkBuilder<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, -} - -impl<'a> DiagnosticSinkBuilder<'a> { - pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new() } - } - - pub fn filter bool + 'a>(mut self, cb: F) -> Self { - self.filters.push(Box::new(cb)); - self - } - - pub fn on(mut self, mut cb: F) -> Self { - let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } - None => Err(()), - }; - self.callbacks.push(Box::new(cb)); - self - } - - pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { - DiagnosticSink { - callbacks: self.callbacks, - filters: self.filters, - default_callback: Box::new(default_callback), - } - } -} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 7f689cd41..ce38396d0 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -27,7 +27,6 @@ mod attrs; mod has_source; pub mod diagnostics; -pub mod diagnostics_sink; pub mod db; mod display; @@ -78,10 +77,7 @@ use syntax::{ }; use tt::{Ident, Leaf, Literal, TokenTree}; -use crate::{ - db::{DefDatabase, HirDatabase}, - diagnostics_sink::DiagnosticSink, -}; +use crate::db::{DefDatabase, HirDatabase}; pub use crate::{ attrs::{HasAttrs, Namespace}, @@ -457,15 +453,10 @@ impl Module { self.id.def_map(db.upcast())[self.id.local_id].scope.visibility_of((*def).into()) } - pub fn diagnostics( - self, - db: &dyn HirDatabase, - sink: &mut DiagnosticSink, - ) -> Vec { + pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { let _p = profile::span("Module::diagnostics").detail(|| { format!("{:?}", self.name(db).map_or("".into(), |name| name.to_string())) }); - let mut acc: Vec = Vec::new(); let def_map = self.id.def_map(db.upcast()); for diag in def_map.diagnostics() { if diag.in_module != self.id.local_id { @@ -618,11 +609,11 @@ impl Module { } for decl in self.declarations(db) { match decl { - ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink)), + ModuleDef::Function(f) => f.diagnostics(db, acc), 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)) + m.diagnostics(db, acc) } } _ => acc.extend(decl.diagnostics(db)), @@ -632,11 +623,10 @@ 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)); + f.diagnostics(db, acc); } } } - acc } pub fn declarations(self, db: &dyn HirDatabase) -> Vec { @@ -1035,12 +1025,7 @@ impl Function { db.function_data(self.id).is_async() } - pub fn diagnostics( - self, - db: &dyn HirDatabase, - sink: &mut DiagnosticSink, - ) -> Vec { - let mut acc: Vec = Vec::new(); + pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { let krate = self.module(db).id.krate(); let source_map = db.body_with_source_map(self.id.into()).1; @@ -1225,7 +1210,6 @@ impl Function { for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) { acc.push(diag.into()) } - acc } /// Whether this function declaration has a definition. -- cgit v1.2.3