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 ++-------- crates/ide/src/diagnostics.rs | 48 +++++----------- 4 files changed, 19 insertions(+), 170 deletions(-) delete mode 100644 crates/hir/src/diagnostics_sink.rs (limited to 'crates') 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. diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index fe6236e44..c024e3e1e 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -26,12 +26,7 @@ mod unresolved_proc_macro; mod field_shorthand; -use std::cell::RefCell; - -use hir::{ - diagnostics::{AnyDiagnostic, DiagnosticCode, DiagnosticSinkBuilder}, - Semantics, -}; +use hir::{diagnostics::AnyDiagnostic, Semantics}; use ide_assists::AssistResolveStrategy; use ide_db::{base_db::SourceDatabase, RootDatabase}; use itertools::Itertools; @@ -45,6 +40,15 @@ use unlinked_file::UnlinkedFile; use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange}; +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct DiagnosticCode(pub &'static str); + +impl DiagnosticCode { + pub fn as_str(&self) -> &str { + self.0 + } +} + #[derive(Debug)] pub struct Diagnostic { // pub name: Option, @@ -113,10 +117,6 @@ impl Diagnostic { fn with_unused(self, unused: bool) -> Self { Self { unused, ..self } } - - fn with_code(self, code: Option) -> Self { - Self { code, ..self } - } } #[derive(Debug, Copy, Clone)] @@ -161,35 +161,13 @@ pub(crate) fn diagnostics( check_unnecessary_braces_in_use_statement(&mut res, file_id, &node); field_shorthand::check(&mut res, file_id, &node); } - let res = RefCell::new(res); - let sink_builder = DiagnosticSinkBuilder::new() - // Only collect experimental diagnostics when they're enabled. - .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) - .filter(|diag| !config.disabled.contains(diag.code().as_str())); - - // Finalize the `DiagnosticSink` building process. - let mut sink = sink_builder - // Diagnostics not handled above get no fix and default treatment. - .build(|d| { - res.borrow_mut().push( - Diagnostic::error( - sema.diagnostics_display_range(d.display_source()).range, - d.message(), - ) - .with_code(Some(d.code())), - ); - }); let mut diags = Vec::new(); let module = sema.to_module_def(file_id); if let Some(m) = module { - diags = m.diagnostics(db, &mut sink) + m.diagnostics(db, &mut diags) } - drop(sink); - - let mut res = res.into_inner(); - let ctx = DiagnosticsContext { config, sema, resolve }; if module.is_none() { let d = UnlinkedFile { file: file_id }; @@ -350,8 +328,8 @@ mod tests { ) .unwrap() .pop() - .unwrap(); - let fix = &diagnostic.fixes.unwrap()[nth]; + .expect("no diagnostics"); + let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; let actual = { let source_change = fix.source_change.as_ref().unwrap(); let file_id = *source_change.source_file_edits.keys().next().unwrap(); -- cgit v1.2.3