From 13735d91a78ba51fb202cb7dde1dfe25420afe9a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 2 Nov 2019 23:42:38 +0300 Subject: [PATCH] Move diagnostics to hir_expand --- crates/ra_hir/src/code_model.rs | 6 +- crates/ra_hir/src/diagnostics.rs | 81 ++--------------------- crates/ra_hir/src/expr/validation.rs | 3 +- crates/ra_hir/src/mock.rs | 3 +- crates/ra_hir/src/nameres.rs | 5 +- crates/ra_hir/src/ty/infer.rs | 12 ++-- crates/ra_hir_expand/src/diagnostics.rs | 85 +++++++++++++++++++++++++ crates/ra_hir_expand/src/lib.rs | 1 + 8 files changed, 107 insertions(+), 89 deletions(-) create mode 100644 crates/ra_hir_expand/src/diagnostics.rs diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index c97ea18a24e..5b78bdfef38 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -11,14 +11,16 @@ type_ref::{Mutability, TypeRef}, CrateModuleId, LocalEnumVariantId, LocalStructFieldId, ModuleId, }; -use hir_expand::name::{self, AsName}; +use hir_expand::{ + diagnostics::DiagnosticSink, + name::{self, AsName}, +}; use ra_db::{CrateId, Edition}; use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner}; use crate::{ adt::VariantDef, db::{AstDatabase, DefDatabase, HirDatabase}, - diagnostics::DiagnosticSink, expr::{validation::ExprValidator, Body, BodySourceMap}, generics::HasGenericParams, ids::{ diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 9acdaf8ed3f..a33af8f4675 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,82 +1,13 @@ //! FIXME: write short doc here -use std::{any::Any, fmt}; +use std::any::Any; -use ra_syntax::{ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr, TextRange}; +use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; use relative_path::RelativePathBuf; -use crate::{db::HirDatabase, HirFileId, Name, Source}; +use crate::{db::AstDatabase, HirFileId, Name, Source}; -/// Diagnostic defines hir API for errors and warnings. -/// -/// It is used as a `dyn` object, which you can downcast to a concrete -/// diagnostic. DiagnosticSink are structured, meaning that they include rich -/// information which can be used by IDE to create fixes. DiagnosticSink are -/// expressed in terms of macro-expanded syntax tree nodes (so, it's a bad idea -/// to diagnostic in a salsa value). -/// -/// Internally, various subsystems of hir produce diagnostics specific to a -/// subsystem (typically, an `enum`), which are safe to store in salsa but do not -/// include source locations. Such internal diagnostic are transformed into an -/// instance of `Diagnostic` on demand. -pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn message(&self) -> String; - fn source(&self) -> Source; - fn highlight_range(&self) -> TextRange { - self.source().ast.range() - } - fn as_any(&self) -> &(dyn Any + Send + 'static); -} - -pub trait AstDiagnostic { - type AST; - fn ast(&self, db: &impl HirDatabase) -> Self::AST; -} - -impl dyn Diagnostic { - pub fn syntax_node(&self, db: &impl HirDatabase) -> SyntaxNode { - let node = db.parse_or_expand(self.source().file_id).unwrap(); - self.source().ast.to_node(&node) - } - - pub fn downcast_ref(&self) -> Option<&D> { - self.as_any().downcast_ref() - } -} - -pub struct DiagnosticSink<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - default_callback: Box, -} - -impl<'a> DiagnosticSink<'a> { - pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> { - DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) } - } - - pub fn on(mut self, mut cb: F) -> DiagnosticSink<'a> { - let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } - None => Err(()), - }; - self.callbacks.push(Box::new(cb)); - self - } - - pub(crate) fn push(&mut self, d: impl Diagnostic) { - let d: &dyn Diagnostic = &d; - for cb in self.callbacks.iter_mut() { - match cb(d) { - Ok(()) => return, - Err(()) => (), - } - } - (self.default_callback)(d) - } -} +pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; #[derive(Debug)] pub struct NoSuchField { @@ -139,7 +70,7 @@ fn as_any(&self) -> &(dyn Any + Send + 'static) { impl AstDiagnostic for MissingFields { type AST = ast::RecordFieldList; - fn ast(&self, db: &impl HirDatabase) -> Self::AST { + fn ast(&self, db: &impl AstDatabase) -> Self::AST { let root = db.parse_or_expand(self.source().file_id).unwrap(); let node = self.source().ast.to_node(&root); ast::RecordFieldList::cast(node).unwrap() @@ -167,7 +98,7 @@ fn as_any(&self) -> &(dyn Any + Send + 'static) { impl AstDiagnostic for MissingOkInTailExpr { type AST = ast::Expr; - fn ast(&self, db: &impl HirDatabase) -> Self::AST { + fn ast(&self, db: &impl AstDatabase) -> Self::AST { let root = db.parse_or_expand(self.file).unwrap(); let node = self.source().ast.to_node(&root); ast::Expr::cast(node).unwrap() diff --git a/crates/ra_hir/src/expr/validation.rs b/crates/ra_hir/src/expr/validation.rs index c685edda193..3054f1dcedf 100644 --- a/crates/ra_hir/src/expr/validation.rs +++ b/crates/ra_hir/src/expr/validation.rs @@ -3,12 +3,13 @@ use std::sync::Arc; use hir_def::path::known; +use hir_expand::diagnostics::DiagnosticSink; use ra_syntax::ast; use rustc_hash::FxHashSet; use crate::{ db::HirDatabase, - diagnostics::{DiagnosticSink, MissingFields, MissingOkInTailExpr}, + diagnostics::{MissingFields, MissingOkInTailExpr}, expr::AstPtr, ty::{ApplicationTy, InferenceResult, Ty, TypeCtor}, Adt, Function, Name, Path, diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs index 35dfaf3bad0..4c89c8d3843 100644 --- a/crates/ra_hir/src/mock.rs +++ b/crates/ra_hir/src/mock.rs @@ -2,6 +2,7 @@ use std::{panic, sync::Arc}; +use hir_expand::diagnostics::DiagnosticSink; use parking_lot::Mutex; use ra_cfg::CfgOptions; use ra_db::{ @@ -12,7 +13,7 @@ use rustc_hash::FxHashMap; use test_utils::{extract_offset, parse_fixture, CURSOR_MARKER}; -use crate::{db, debug::HirDebugHelper, diagnostics::DiagnosticSink}; +use crate::{db, debug::HirDebugHelper}; pub const WORKSPACE: SourceRootId = SourceRootId(0); diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index 7ba03182718..32a6ab47495 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -55,6 +55,7 @@ use std::sync::Arc; use hir_def::{builtin_type::BuiltinType, CrateModuleId}; +use hir_expand::diagnostics::DiagnosticSink; use once_cell::sync::Lazy; use ra_arena::Arena; use ra_db::{Edition, FileId}; @@ -65,7 +66,6 @@ use crate::{ db::{AstDatabase, DefDatabase}, - diagnostics::DiagnosticSink, ids::MacroDefId, nameres::diagnostics::DefDiagnostic, Adt, AstId, Crate, HirFileId, MacroDef, Module, ModuleDef, Name, Path, PathKind, Trait, @@ -513,12 +513,13 @@ fn resolve_in_prelude(&self, db: &impl DefDatabase, name: &Name) -> PerNs { } mod diagnostics { + use hir_expand::diagnostics::DiagnosticSink; use ra_syntax::{ast, AstPtr}; use relative_path::RelativePathBuf; use crate::{ db::{AstDatabase, DefDatabase}, - diagnostics::{DiagnosticSink, UnresolvedModule}, + diagnostics::UnresolvedModule, nameres::CrateModuleId, AstId, }; diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 6694467a36c..2370e8d4f52 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -25,7 +25,7 @@ path::known, type_ref::{Mutability, TypeRef}, }; -use hir_expand::name; +use hir_expand::{diagnostics::DiagnosticSink, name}; use ra_arena::map::ArenaMap; use ra_prof::profile; use test_utils::tested_by; @@ -40,7 +40,6 @@ adt::VariantDef, code_model::TypeAlias, db::HirDatabase, - diagnostics::DiagnosticSink, expr::{BindingAnnotation, Body, ExprId, PatId}, resolve::{Resolver, TypeNs}, ty::infer::diagnostics::InferenceDiagnostic, @@ -719,12 +718,9 @@ fn none() -> Self { } mod diagnostics { - use crate::{ - db::HirDatabase, - diagnostics::{DiagnosticSink, NoSuchField}, - expr::ExprId, - Function, HasSource, - }; + use hir_expand::diagnostics::DiagnosticSink; + + use crate::{db::HirDatabase, diagnostics::NoSuchField, expr::ExprId, Function, HasSource}; #[derive(Debug, PartialEq, Eq, Clone)] pub(super) enum InferenceDiagnostic { diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs new file mode 100644 index 00000000000..201884b9574 --- /dev/null +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -0,0 +1,85 @@ +//! 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 sued 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 ra_syntax::{SyntaxNode, SyntaxNodePtr, TextRange}; + +use crate::{db::AstDatabase, Source}; + +pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn message(&self) -> String; + fn source(&self) -> Source; + fn highlight_range(&self) -> TextRange { + self.source().ast.range() + } + fn as_any(&self) -> &(dyn Any + Send + 'static); +} + +pub trait AstDiagnostic { + type AST; + fn ast(&self, db: &impl AstDatabase) -> Self::AST; +} + +impl dyn Diagnostic { + pub fn syntax_node(&self, db: &impl AstDatabase) -> SyntaxNode { + let node = db.parse_or_expand(self.source().file_id).unwrap(); + self.source().ast.to_node(&node) + } + + pub fn downcast_ref(&self) -> Option<&D> { + self.as_any().downcast_ref() + } +} + +pub struct DiagnosticSink<'a> { + callbacks: Vec Result<(), ()> + 'a>>, + default_callback: Box, +} + +impl<'a> DiagnosticSink<'a> { + /// FIXME: split `new` and `on` into a separate builder type + pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> { + DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) } + } + + pub fn on(mut self, mut cb: F) -> DiagnosticSink<'a> { + let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { + Some(d) => { + cb(d); + Ok(()) + } + None => Err(()), + }; + self.callbacks.push(Box::new(cb)); + self + } + + pub fn push(&mut self, d: impl Diagnostic) { + let d: &dyn Diagnostic = &d; + self._push(d); + } + + fn _push(&mut self, d: &dyn Diagnostic) { + for cb in self.callbacks.iter_mut() { + match cb(d) { + Ok(()) => return, + Err(()) => (), + } + } + (self.default_callback)(d) + } +} diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index 85c2b22ace5..dd07a16b4ca 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -9,6 +9,7 @@ pub mod either; pub mod name; pub mod hygiene; +pub mod diagnostics; use std::hash::{Hash, Hasher};