From 7e8f17188efcecfdfd1afbbc894a53c65985f836 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 21 Mar 2019 22:13:11 +0300 Subject: [PATCH 01/11] diagnostics --- crates/ra_hir/src/code_model_api.rs | 5 +++++ crates/ra_hir/src/diagnostics.rs | 6 ++++++ crates/ra_hir/src/expr.rs | 15 ++++++++++++-- crates/ra_hir/src/lib.rs | 1 + crates/ra_hir/src/ty/infer.rs | 23 ++++++++++++++++++--- crates/ra_ide_api/src/diagnostics.rs | 30 +++++++++++++++++++++++++++- 6 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 crates/ra_hir/src/diagnostics.rs diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index 45fa4cd110..58481e715e 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -17,6 +17,7 @@ use crate::{ ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeId}, impl_block::ImplBlock, resolve::Resolver, + diagnostics::FunctionDiagnostic, }; /// hir::Crate describes a single crate. It's the main interface with which @@ -519,6 +520,10 @@ impl Function { let r = if !p.params.is_empty() { r.push_generic_params_scope(p) } else { r }; r } + + pub fn diagnostics(&self, db: &impl HirDatabase) -> Vec { + self.infer(db).diagnostics() + } } impl Docs for Function { diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs new file mode 100644 index 0000000000..82aff9cee0 --- /dev/null +++ b/crates/ra_hir/src/diagnostics.rs @@ -0,0 +1,6 @@ +use crate::{expr::ExprId}; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum FunctionDiagnostic { + NoSuchField { expr: ExprId, field: usize }, +} diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index c37fd0454b..31af5d2410 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -5,7 +5,7 @@ use rustc_hash::FxHashMap; use ra_arena::{Arena, RawId, impl_arena_id, map::ArenaMap}; use ra_syntax::{ - SyntaxNodePtr, AstNode, + SyntaxNodePtr, AstPtr, AstNode, ast::{self, LoopBodyOwner, ArgListOwner, NameOwner, LiteralFlavor, TypeAscriptionOwner} }; @@ -54,6 +54,7 @@ pub struct BodySourceMap { expr_map_back: ArenaMap, pat_map: FxHashMap, pat_map_back: ArenaMap, + field_map: FxHashMap<(ExprId, usize), AstPtr>, } impl Body { @@ -138,6 +139,10 @@ impl BodySourceMap { pub fn node_pat(&self, node: &ast::Pat) -> Option { self.pat_map.get(&SyntaxNodePtr::new(node.syntax())).cloned() } + + pub fn field_syntax(&self, expr: ExprId, field: usize) -> Option> { + self.field_map.get(&(expr, field)).cloned() + } } #[derive(Debug, Clone, Eq, PartialEq)] @@ -629,8 +634,10 @@ impl ExprCollector { } ast::ExprKind::StructLit(e) => { let path = e.path().and_then(Path::from_ast); + let mut field_ptrs = Vec::new(); let fields = if let Some(nfl) = e.named_field_list() { nfl.fields() + .inspect(|field| field_ptrs.push(AstPtr::new(*field))) .map(|field| StructLitField { name: field .name_ref() @@ -657,7 +664,11 @@ impl ExprCollector { Vec::new() }; let spread = e.spread().map(|s| self.collect_expr(s)); - self.alloc_expr(Expr::StructLit { path, fields, spread }, syntax_ptr) + let res = self.alloc_expr(Expr::StructLit { path, fields, spread }, syntax_ptr); + for (i, ptr) in field_ptrs.into_iter().enumerate() { + self.source_map.field_map.insert((res, i), ptr); + } + res } ast::ExprKind::FieldExpr(e) => { let expr = self.collect_expr_opt(e.expr()); diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index a89c916f88..390aef0a9c 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -35,6 +35,7 @@ mod expr; mod generics; mod docs; mod resolve; +pub mod diagnostics; mod code_model_api; mod code_model_impl; diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index cff7e74819..269b5162e3 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -36,7 +36,8 @@ use crate::{ path::{GenericArgs, GenericArg}, adt::VariantDef, resolve::{Resolver, Resolution}, - nameres::Namespace + nameres::Namespace, + diagnostics::FunctionDiagnostic, }; use super::{Ty, TypableDef, Substs, primitive, op, FnSig, ApplicationTy, TypeCtor}; @@ -96,6 +97,7 @@ pub struct InferenceResult { field_resolutions: FxHashMap, /// For each associated item record what it resolves to assoc_resolutions: FxHashMap, + diagnostics: Vec, pub(super) type_of_expr: ArenaMap, pub(super) type_of_pat: ArenaMap, } @@ -113,6 +115,9 @@ impl InferenceResult { pub fn assoc_resolutions_for_pat(&self, id: PatId) -> Option { self.assoc_resolutions.get(&id.into()).map(|it| *it) } + pub(crate) fn diagnostics(&self) -> Vec { + self.diagnostics.clone() + } } impl Index for InferenceResult { @@ -143,6 +148,7 @@ struct InferenceContext<'a, D: HirDatabase> { assoc_resolutions: FxHashMap, type_of_expr: ArenaMap, type_of_pat: ArenaMap, + diagnostics: Vec, /// The return type of the function being inferred. return_ty: Ty, } @@ -155,6 +161,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { assoc_resolutions: FxHashMap::default(), type_of_expr: ArenaMap::default(), type_of_pat: ArenaMap::default(), + diagnostics: Vec::default(), var_unification_table: InPlaceUnificationTable::new(), return_ty: Ty::Unknown, // set in collect_fn_signature db, @@ -181,6 +188,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { assoc_resolutions: self.assoc_resolutions, type_of_expr: expr_types, type_of_pat: pat_types, + diagnostics: self.diagnostics, } } @@ -915,9 +923,18 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { Expr::StructLit { path, fields, spread } => { let (ty, def_id) = self.resolve_variant(path.as_ref()); let substs = ty.substs().unwrap_or_else(Substs::empty); - for field in fields { + for (field_idx, field) in fields.into_iter().enumerate() { let field_ty = def_id - .and_then(|it| it.field(self.db, &field.name)) + .and_then(|it| match it.field(self.db, &field.name) { + Some(field) => Some(field), + None => { + self.diagnostics.push(FunctionDiagnostic::NoSuchField { + expr: tgt_expr, + field: field_idx, + }); + None + } + }) .map_or(Ty::Unknown, |field| field.ty(self.db)) .subst(&substs); self.infer_expr(field.expr, &Expectation::has_type(field_ty)); diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 156f28ca33..f662f7e2f1 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -3,7 +3,7 @@ use hir::{Problem, source_binder}; use ra_db::SourceDatabase; use ra_syntax::{ Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, - ast::{self, AstNode}, + ast::{self, AstNode, NameOwner}, }; use ra_text_edit::{TextEdit, TextEditBuilder}; @@ -134,6 +134,13 @@ fn check_module( file_id: FileId, module: hir::Module, ) { + for decl in module.declarations(db) { + match decl { + hir::ModuleDef::Function(f) => check_function(acc, db, f), + _ => (), + } + } + let source_root = db.file_source_root(file_id); for (name_node, problem) in module.problems(db) { let diag = match problem { @@ -153,6 +160,27 @@ fn check_module( } } +fn check_function(acc: &mut Vec, db: &RootDatabase, function: hir::Function) { + let (_file_id, fn_def) = function.source(db); + let source_file = fn_def.syntax().ancestors().find_map(ast::SourceFile::cast).unwrap(); + let source_map = function.body_source_map(db); + for d in function.diagnostics(db) { + match d { + hir::diagnostics::FunctionDiagnostic::NoSuchField { expr, field } => { + if let Some(field) = source_map.field_syntax(expr, field) { + let field = field.to_node(&source_file); + acc.push(Diagnostic { + message: "no such field".into(), + range: field.syntax().range(), + severity: Severity::Error, + fix: None, + }) + } + } + } + } +} + #[cfg(test)] mod tests { use test_utils::assert_eq_text; From fcca35969dd7c63a83ee34c4ce7d54cefdb72bbe Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 16:28:47 +0300 Subject: [PATCH 02/11] allow dyn diagnostics --- crates/ra_hir/src/code_model_api.rs | 8 ++-- crates/ra_hir/src/diagnostics.rs | 62 ++++++++++++++++++++++++++-- crates/ra_hir/src/expr.rs | 4 +- crates/ra_hir/src/ty/infer.rs | 39 ++++++++++++++--- crates/ra_ide_api/src/diagnostics.rs | 26 ++++-------- crates/ra_syntax/src/ptr.rs | 6 +++ 6 files changed, 112 insertions(+), 33 deletions(-) diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index 58481e715e..a37d960a14 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -17,7 +17,7 @@ use crate::{ ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeId}, impl_block::ImplBlock, resolve::Resolver, - diagnostics::FunctionDiagnostic, + diagnostics::Diagnostics, }; /// hir::Crate describes a single crate. It's the main interface with which @@ -521,8 +521,10 @@ impl Function { r } - pub fn diagnostics(&self, db: &impl HirDatabase) -> Vec { - self.infer(db).diagnostics() + pub fn diagnostics(&self, db: &impl HirDatabase) -> Diagnostics { + let mut res = Diagnostics::default(); + self.infer(db).add_diagnostics(db, *self, &mut res); + res } } diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 82aff9cee0..46a3fdd479 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,6 +1,60 @@ -use crate::{expr::ExprId}; +use std::{fmt, any::Any}; -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum FunctionDiagnostic { - NoSuchField { expr: ExprId, field: usize }, +use ra_syntax::{SyntaxNodePtr, AstPtr, ast}; + +use crate::HirFileId; + +pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn file(&self) -> HirFileId; + fn syntax_node(&self) -> SyntaxNodePtr; + fn message(&self) -> String; + fn as_any(&self) -> &(Any + Send + 'static); +} + +impl dyn Diagnostic { + pub fn downcast_ref(&self) -> Option<&D> { + self.as_any().downcast_ref() + } +} + +#[derive(Debug, Default)] +pub struct Diagnostics { + data: Vec>, +} + +impl Diagnostics { + pub fn push(&mut self, d: impl Diagnostic) { + self.data.push(Box::new(d)) + } + + pub fn iter<'a>(&'a self) -> impl Iterator + 'a { + self.data.iter().map(|it| it.as_ref()) + } +} + +#[derive(Debug)] +pub struct NoSuchField { + pub(crate) file: HirFileId, + pub(crate) field: AstPtr, +} + +impl NoSuchField { + pub fn field(&self) -> AstPtr { + self.field + } +} + +impl Diagnostic for NoSuchField { + fn file(&self) -> HirFileId { + self.file + } + fn syntax_node(&self) -> SyntaxNodePtr { + self.field.into() + } + fn message(&self) -> String { + "no such field".to_string() + } + fn as_any(&self) -> &(Any + Send + 'static) { + self + } } diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index 31af5d2410..a85422955b 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -140,8 +140,8 @@ impl BodySourceMap { self.pat_map.get(&SyntaxNodePtr::new(node.syntax())).cloned() } - pub fn field_syntax(&self, expr: ExprId, field: usize) -> Option> { - self.field_map.get(&(expr, field)).cloned() + pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { + self.field_map[&(expr, field)].clone() } } diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 269b5162e3..02708ba0f4 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -37,7 +37,8 @@ use crate::{ adt::VariantDef, resolve::{Resolver, Resolution}, nameres::Namespace, - diagnostics::FunctionDiagnostic, + ty::infer::diagnostics::InferenceDiagnostic, + diagnostics::Diagnostics, }; use super::{Ty, TypableDef, Substs, primitive, op, FnSig, ApplicationTy, TypeCtor}; @@ -97,7 +98,7 @@ pub struct InferenceResult { field_resolutions: FxHashMap, /// For each associated item record what it resolves to assoc_resolutions: FxHashMap, - diagnostics: Vec, + diagnostics: Vec, pub(super) type_of_expr: ArenaMap, pub(super) type_of_pat: ArenaMap, } @@ -115,8 +116,13 @@ impl InferenceResult { pub fn assoc_resolutions_for_pat(&self, id: PatId) -> Option { self.assoc_resolutions.get(&id.into()).map(|it| *it) } - pub(crate) fn diagnostics(&self) -> Vec { - self.diagnostics.clone() + pub(crate) fn add_diagnostics( + &self, + db: &impl HirDatabase, + owner: Function, + diagnostics: &mut Diagnostics, + ) { + self.diagnostics.iter().for_each(|it| it.add_to(db, owner, diagnostics)) } } @@ -148,7 +154,7 @@ struct InferenceContext<'a, D: HirDatabase> { assoc_resolutions: FxHashMap, type_of_expr: ArenaMap, type_of_pat: ArenaMap, - diagnostics: Vec, + diagnostics: Vec, /// The return type of the function being inferred. return_ty: Ty, } @@ -928,7 +934,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { .and_then(|it| match it.field(self.db, &field.name) { Some(field) => Some(field), None => { - self.diagnostics.push(FunctionDiagnostic::NoSuchField { + self.diagnostics.push(InferenceDiagnostic::NoSuchField { expr: tgt_expr, field: field_idx, }); @@ -1261,3 +1267,24 @@ impl Expectation { Expectation { ty: Ty::Unknown } } } + +mod diagnostics { + use crate::{expr::ExprId, diagnostics::{Diagnostics, NoSuchField}, HirDatabase, Function}; + + #[derive(Debug, PartialEq, Eq, Clone)] + pub(super) enum InferenceDiagnostic { + NoSuchField { expr: ExprId, field: usize }, + } + + impl InferenceDiagnostic { + pub(super) fn add_to(&self, db: &impl HirDatabase, owner: Function, acc: &mut Diagnostics) { + match self { + InferenceDiagnostic::NoSuchField { expr, field } => { + let (file, _) = owner.source(db); + let field = owner.body_source_map(db).field_syntax(*expr, *field); + acc.push(NoSuchField { file, field }) + } + } + } + } +} diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index f662f7e2f1..943fd2f539 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -3,7 +3,7 @@ use hir::{Problem, source_binder}; use ra_db::SourceDatabase; use ra_syntax::{ Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, - ast::{self, AstNode, NameOwner}, + ast::{self, AstNode}, }; use ra_text_edit::{TextEdit, TextEditBuilder}; @@ -161,23 +161,13 @@ fn check_module( } fn check_function(acc: &mut Vec, db: &RootDatabase, function: hir::Function) { - let (_file_id, fn_def) = function.source(db); - let source_file = fn_def.syntax().ancestors().find_map(ast::SourceFile::cast).unwrap(); - let source_map = function.body_source_map(db); - for d in function.diagnostics(db) { - match d { - hir::diagnostics::FunctionDiagnostic::NoSuchField { expr, field } => { - if let Some(field) = source_map.field_syntax(expr, field) { - let field = field.to_node(&source_file); - acc.push(Diagnostic { - message: "no such field".into(), - range: field.syntax().range(), - severity: Severity::Error, - fix: None, - }) - } - } - } + for d in function.diagnostics(db).iter() { + acc.push(Diagnostic { + message: d.message(), + range: d.syntax_node().range(), + severity: Severity::Error, + fix: None, + }) } } diff --git a/crates/ra_syntax/src/ptr.rs b/crates/ra_syntax/src/ptr.rs index aae590cb62..d8de1c4c1b 100644 --- a/crates/ra_syntax/src/ptr.rs +++ b/crates/ra_syntax/src/ptr.rs @@ -64,6 +64,12 @@ impl AstPtr { } } +impl From> for SyntaxNodePtr { + fn from(ptr: AstPtr) -> SyntaxNodePtr { + ptr.raw + } +} + #[test] fn test_local_syntax_ptr() { use crate::{ast, AstNode}; From 3fb88e95aa5e122a521beec766d5b1264ca4de3b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 18:35:14 +0300 Subject: [PATCH 03/11] switch modules to new diagnostics --- crates/ra_hir/src/code_model_api.rs | 18 ++---- crates/ra_hir/src/code_model_impl/module.rs | 19 +----- crates/ra_hir/src/diagnostics.rs | 45 +++++++++++--- crates/ra_hir/src/lib.rs | 2 +- crates/ra_hir/src/nameres.rs | 65 +++++++++++++------- crates/ra_hir/src/nameres/collector.rs | 63 +++++++++----------- crates/ra_hir/src/ty/infer.rs | 13 ++-- crates/ra_ide_api/src/diagnostics.rs | 66 ++++++++++----------- 8 files changed, 156 insertions(+), 135 deletions(-) diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index a37d960a14..03b1acef3b 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -1,8 +1,7 @@ use std::sync::Arc; -use relative_path::RelativePathBuf; use ra_db::{CrateId, SourceRootId, Edition}; -use ra_syntax::{ast::self, TreeArc, SyntaxNode}; +use ra_syntax::{ast::self, TreeArc}; use crate::{ Name, ScopesWithSourceMap, Ty, HirFileId, @@ -96,11 +95,6 @@ pub enum ModuleSource { Module(TreeArc), } -#[derive(Clone, Debug, Hash, PartialEq, Eq)] -pub enum Problem { - UnresolvedModule { candidate: RelativePathBuf }, -} - impl Module { /// Name of this module. pub fn name(&self, db: &impl HirDatabase) -> Option { @@ -172,8 +166,8 @@ impl Module { db.crate_def_map(self.krate)[self.module_id].scope.clone() } - pub fn problems(&self, db: &impl HirDatabase) -> Vec<(TreeArc, Problem)> { - self.problems_impl(db) + pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) { + db.crate_def_map(self.krate).add_diagnostics(db, self.module_id, sink); } pub fn resolver(&self, db: &impl HirDatabase) -> Resolver { @@ -521,10 +515,8 @@ impl Function { r } - pub fn diagnostics(&self, db: &impl HirDatabase) -> Diagnostics { - let mut res = Diagnostics::default(); - self.infer(db).add_diagnostics(db, *self, &mut res); - res + pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) { + self.infer(db).add_diagnostics(db, *self, sink); } } diff --git a/crates/ra_hir/src/code_model_impl/module.rs b/crates/ra_hir/src/code_model_impl/module.rs index 52a33e981f..14237060cf 100644 --- a/crates/ra_hir/src/code_model_impl/module.rs +++ b/crates/ra_hir/src/code_model_impl/module.rs @@ -1,8 +1,8 @@ use ra_db::FileId; -use ra_syntax::{ast, SyntaxNode, TreeArc, AstNode}; +use ra_syntax::{ast, TreeArc, AstNode}; use crate::{ - Module, ModuleSource, Problem, Name, + Module, ModuleSource, Name, nameres::{CrateModuleId, ImportId}, HirDatabase, DefDatabase, HirFileId, SourceItemId, @@ -108,19 +108,4 @@ impl Module { let parent_id = def_map[self.module_id].parent?; Some(self.with_module_id(parent_id)) } - - pub(crate) fn problems_impl( - &self, - db: &impl HirDatabase, - ) -> Vec<(TreeArc, Problem)> { - let def_map = db.crate_def_map(self.krate); - let (my_file_id, _) = self.definition_source(db); - // FIXME: not entirely corret filterint by module - def_map - .problems() - .iter() - .filter(|(source_item_id, _problem)| my_file_id == source_item_id.file_id) - .map(|(source_item_id, problem)| (db.file_item(*source_item_id), problem.clone())) - .collect() - } } diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 46a3fdd479..e8bb1ed92e 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -3,7 +3,20 @@ use std::{fmt, any::Any}; use ra_syntax::{SyntaxNodePtr, AstPtr, ast}; use crate::HirFileId; +use relative_path::RelativePathBuf; +/// Diagnostic defines hir API for errors and warnings. +/// +/// It is used as a `dyn` object, which you can downcast to a concrete +/// diagnostic. Diagnostics are structured, meaning that they include rich +/// information which can be used by IDE to create fixes. Diagnostics 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 +/// subsytem (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 file(&self) -> HirFileId; fn syntax_node(&self) -> SyntaxNodePtr; @@ -34,14 +47,8 @@ impl Diagnostics { #[derive(Debug)] pub struct NoSuchField { - pub(crate) file: HirFileId, - pub(crate) field: AstPtr, -} - -impl NoSuchField { - pub fn field(&self) -> AstPtr { - self.field - } + pub file: HirFileId, + pub field: AstPtr, } impl Diagnostic for NoSuchField { @@ -58,3 +65,25 @@ impl Diagnostic for NoSuchField { self } } + +#[derive(Debug)] +pub struct UnresolvedModule { + pub file: HirFileId, + pub decl: AstPtr, + pub candidate: RelativePathBuf, +} + +impl Diagnostic for UnresolvedModule { + fn file(&self) -> HirFileId { + self.file + } + fn syntax_node(&self) -> SyntaxNodePtr { + self.decl.into() + } + fn message(&self) -> String { + "unresolved module".to_string() + } + fn as_any(&self) -> &(Any + Send + 'static) { + self + } +} diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 390aef0a9c..ce54d76084 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -64,7 +64,7 @@ pub use self::{ pub use self::code_model_api::{ Crate, CrateDependency, - Module, ModuleDef, ModuleSource, Problem, + Module, ModuleDef, ModuleSource, Struct, Enum, EnumVariant, Function, FnSignature, StructField, FieldSource, diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index d361cf9e62..416f114b40 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -56,14 +56,17 @@ mod tests; use std::sync::Arc; use rustc_hash::FxHashMap; +use relative_path::RelativePathBuf; use ra_arena::{Arena, RawId, impl_arena_id}; use ra_db::{FileId, Edition}; +use ra_syntax::{AstNode, AstPtr, ast}; use test_utils::tested_by; use crate::{ - ModuleDef, Name, Crate, Module, Problem, + ModuleDef, Name, Crate, Module, DefDatabase, Path, PathKind, HirFileId, ids::{SourceItemId, SourceFileItemId, MacroCallId}, + diagnostics::{Diagnostics, UnresolvedModule}, }; pub(crate) use self::raw::{RawItems, ImportId, ImportSourceMap}; @@ -85,7 +88,7 @@ pub struct CrateDefMap { macros: Arena, public_macros: FxHashMap, macro_resolutions: FxHashMap, - problems: CrateDefMapProblems, + diagnostics: Vec, } impl std::ops::Index for CrateDefMap { @@ -125,21 +128,6 @@ pub(crate) struct ModuleData { pub(crate) definition: Option, } -#[derive(Default, Debug, PartialEq, Eq)] -pub(crate) struct CrateDefMapProblems { - problems: Vec<(SourceItemId, Problem)>, -} - -impl CrateDefMapProblems { - fn add(&mut self, source_item_id: SourceItemId, problem: Problem) { - self.problems.push((source_item_id, problem)) - } - - pub(crate) fn iter<'a>(&'a self) -> impl Iterator + 'a { - self.problems.iter().map(|(s, p)| (s, p)) - } -} - #[derive(Debug, Default, PartialEq, Eq, Clone)] pub struct ModuleScope { items: FxHashMap, @@ -212,7 +200,7 @@ impl CrateDefMap { macros: Arena::default(), public_macros: FxHashMap::default(), macro_resolutions: FxHashMap::default(), - problems: CrateDefMapProblems::default(), + diagnostics: Vec::new(), } }; let def_map = collector::collect_defs(db, def_map); @@ -224,10 +212,6 @@ impl CrateDefMap { self.root } - pub(crate) fn problems(&self) -> &CrateDefMapProblems { - &self.problems - } - pub(crate) fn mk_module(&self, module_id: CrateModuleId) -> Module { Module { krate: self.krate, module_id } } @@ -240,6 +224,15 @@ impl CrateDefMap { &self.extern_prelude } + pub(crate) fn add_diagnostics( + &self, + db: &impl DefDatabase, + module: CrateModuleId, + sink: &mut Diagnostics, + ) { + self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink)) + } + pub(crate) fn resolve_macro( &self, macro_call_id: MacroCallId, @@ -452,3 +445,31 @@ impl CrateDefMap { } } } + +#[derive(Debug, PartialEq, Eq)] +enum DefDiagnostic { + UnresolvedModule { + module: CrateModuleId, + declaration: SourceItemId, + candidate: RelativePathBuf, + }, +} + +impl DefDiagnostic { + fn add_to(&self, db: &impl DefDatabase, target_module: CrateModuleId, sink: &mut Diagnostics) { + match self { + DefDiagnostic::UnresolvedModule { module, declaration, candidate } => { + if *module != target_module { + return; + } + let syntax = db.file_item(*declaration); + let decl = ast::Module::cast(&syntax).unwrap(); + sink.push(UnresolvedModule { + file: declaration.file_id, + decl: AstPtr::new(&decl), + candidate: candidate.clone(), + }) + } + } + } +} diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs index c5b73cfbee..bc6bc5e7fd 100644 --- a/crates/ra_hir/src/nameres/collector.rs +++ b/crates/ra_hir/src/nameres/collector.rs @@ -6,9 +6,9 @@ use ra_db::FileId; use crate::{ Function, Module, Struct, Enum, Const, Static, Trait, TypeAlias, - DefDatabase, HirFileId, Name, Path, Problem, Crate, + DefDatabase, HirFileId, Name, Path, Crate, KnownName, - nameres::{Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, raw}, + nameres::{Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, raw, DefDiagnostic}, ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId}, }; @@ -405,25 +405,27 @@ where raw::ModuleData::Declaration { name, source_item_id } => { let source_item_id = source_item_id.with_file_id(self.file_id); let is_root = self.def_collector.def_map.modules[self.module_id].parent.is_none(); - let (file_ids, problem) = - resolve_submodule(self.def_collector.db, self.file_id, name, is_root); - - if let Some(problem) = problem { - self.def_collector.def_map.problems.add(source_item_id, problem) - } - - if let Some(&file_id) = file_ids.first() { - let module_id = - self.push_child_module(name.clone(), source_item_id, Some(file_id)); - let raw_items = self.def_collector.db.raw_items(file_id); - ModCollector { - def_collector: &mut *self.def_collector, - module_id, - file_id: file_id.into(), - raw_items: &raw_items, + match resolve_submodule(self.def_collector.db, self.file_id, name, is_root) { + Ok(file_id) => { + let module_id = + self.push_child_module(name.clone(), source_item_id, Some(file_id)); + let raw_items = self.def_collector.db.raw_items(file_id); + ModCollector { + def_collector: &mut *self.def_collector, + module_id, + file_id: file_id.into(), + raw_items: &raw_items, + } + .collect(raw_items.items()) } - .collect(raw_items.items()) - } + Err(candidate) => self.def_collector.def_map.diagnostics.push( + DefDiagnostic::UnresolvedModule { + module: self.module_id, + declaration: source_item_id, + candidate, + }, + ), + }; } } } @@ -524,7 +526,7 @@ fn resolve_submodule( file_id: HirFileId, name: &Name, is_root: bool, -) -> (Vec, Option) { +) -> Result { // FIXME: handle submodules of inline modules properly let file_id = file_id.original_file(db); let source_root_id = db.file_source_root(file_id); @@ -545,17 +547,10 @@ fn resolve_submodule( candidates.push(file_dir_mod.clone()); }; let sr = db.source_root(source_root_id); - let points_to = candidates - .into_iter() - .filter_map(|path| sr.files.get(&path)) - .map(|&it| it) - .collect::>(); - let problem = if points_to.is_empty() { - Some(Problem::UnresolvedModule { - candidate: if is_dir_owner { file_mod } else { file_dir_mod }, - }) - } else { - None - }; - (points_to, problem) + let mut points_to = candidates.into_iter().filter_map(|path| sr.files.get(&path)).map(|&it| it); + // FIXME: handle ambiguity + match points_to.next() { + Some(file_id) => Ok(file_id), + None => Err(if is_dir_owner { file_mod } else { file_dir_mod }), + } } diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 02708ba0f4..6dc3edc7a1 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -120,9 +120,9 @@ impl InferenceResult { &self, db: &impl HirDatabase, owner: Function, - diagnostics: &mut Diagnostics, + sink: &mut Diagnostics, ) { - self.diagnostics.iter().for_each(|it| it.add_to(db, owner, diagnostics)) + self.diagnostics.iter().for_each(|it| it.add_to(db, owner, sink)) } } @@ -1277,12 +1277,17 @@ mod diagnostics { } impl InferenceDiagnostic { - pub(super) fn add_to(&self, db: &impl HirDatabase, owner: Function, acc: &mut Diagnostics) { + pub(super) fn add_to( + &self, + db: &impl HirDatabase, + owner: Function, + sink: &mut Diagnostics, + ) { match self { InferenceDiagnostic::NoSuchField { expr, field } => { let (file, _) = owner.source(db); let field = owner.body_source_map(db).field_syntax(*expr, *field); - acc.push(NoSuchField { file, field }) + sink.push(NoSuchField { file, field }) } } } diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 943fd2f539..254342c0a6 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -1,5 +1,5 @@ use itertools::Itertools; -use hir::{Problem, source_binder}; +use hir::{source_binder, diagnostics::Diagnostic as _}; use ra_db::SourceDatabase; use ra_syntax::{ Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, @@ -28,7 +28,7 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec } if let Some(m) = source_binder::module_from_file_id(db, file_id) { - check_module(&mut res, db, file_id, m); + check_module(&mut res, db, m); }; res } @@ -128,46 +128,40 @@ fn check_struct_shorthand_initialization( Some(()) } -fn check_module( - acc: &mut Vec, - db: &RootDatabase, - file_id: FileId, - module: hir::Module, -) { +fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Module) { + let mut diagnostics = hir::diagnostics::Diagnostics::default(); + module.diagnostics(db, &mut diagnostics); for decl in module.declarations(db) { match decl { - hir::ModuleDef::Function(f) => check_function(acc, db, f), + hir::ModuleDef::Function(f) => f.diagnostics(db, &mut diagnostics), _ => (), } } - let source_root = db.file_source_root(file_id); - for (name_node, problem) in module.problems(db) { - let diag = match problem { - Problem::UnresolvedModule { candidate } => { - let create_file = - FileSystemEdit::CreateFile { source_root, path: candidate.clone() }; - let fix = SourceChange::file_system_edit("create module", create_file); - Diagnostic { - range: name_node.range(), - message: "unresolved module".to_string(), - severity: Severity::Error, - fix: Some(fix), - } - } - }; - acc.push(diag) - } -} - -fn check_function(acc: &mut Vec, db: &RootDatabase, function: hir::Function) { - for d in function.diagnostics(db).iter() { - acc.push(Diagnostic { - message: d.message(), - range: d.syntax_node().range(), - severity: Severity::Error, - fix: None, - }) + for d in diagnostics.iter() { + if let Some(d) = d.downcast_ref::() { + let source_root = db.file_source_root(d.file().original_file(db)); + let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; + let fix = SourceChange { + label: "create module".to_string(), + source_file_edits: Vec::new(), + file_system_edits: vec![create_file], + cursor_position: None, + }; + acc.push(Diagnostic { + range: d.syntax_node().range(), + message: d.message(), + severity: Severity::Error, + fix: Some(fix), + }) + } else { + acc.push(Diagnostic { + message: d.message(), + range: d.syntax_node().range(), + severity: Severity::Error, + fix: None, + }) + } } } From 79df62bc742afa33dcf5bedefd60860ca296b9da Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 20:41:59 +0300 Subject: [PATCH 04/11] cleanup --- crates/ra_hir/src/code_model_api.rs | 6 +-- crates/ra_hir/src/diagnostics.rs | 28 +++++------ crates/ra_hir/src/nameres.rs | 66 ++++++++++++++++---------- crates/ra_hir/src/nameres/collector.rs | 9 ++-- crates/ra_hir/src/ty/infer.rs | 8 ++-- crates/ra_ide_api/src/diagnostics.rs | 4 +- 6 files changed, 70 insertions(+), 51 deletions(-) diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index 03b1acef3b..bc0f74c896 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -16,7 +16,7 @@ use crate::{ ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeId}, impl_block::ImplBlock, resolve::Resolver, - diagnostics::Diagnostics, + diagnostics::DiagnosticSink, }; /// hir::Crate describes a single crate. It's the main interface with which @@ -166,7 +166,7 @@ impl Module { db.crate_def_map(self.krate)[self.module_id].scope.clone() } - pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) { + pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut DiagnosticSink) { db.crate_def_map(self.krate).add_diagnostics(db, self.module_id, sink); } @@ -515,7 +515,7 @@ impl Function { r } - pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) { + pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut DiagnosticSink) { self.infer(db).add_diagnostics(db, *self, sink); } } diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index e8bb1ed92e..d6b28159e9 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -8,19 +8,19 @@ use relative_path::RelativePathBuf; /// Diagnostic defines hir API for errors and warnings. /// /// It is used as a `dyn` object, which you can downcast to a concrete -/// diagnostic. Diagnostics are structured, meaning that they include rich -/// information which can be used by IDE to create fixes. Diagnostics are +/// 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 -/// subsytem (typically, an `enum`), which are safe to store in salsa but do not +/// 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 file(&self) -> HirFileId; fn syntax_node(&self) -> SyntaxNodePtr; - fn message(&self) -> String; fn as_any(&self) -> &(Any + Send + 'static); } @@ -31,17 +31,17 @@ impl dyn Diagnostic { } #[derive(Debug, Default)] -pub struct Diagnostics { +pub struct DiagnosticSink { data: Vec>, } -impl Diagnostics { +impl DiagnosticSink { pub fn push(&mut self, d: impl Diagnostic) { self.data.push(Box::new(d)) } - pub fn iter<'a>(&'a self) -> impl Iterator + 'a { - self.data.iter().map(|it| it.as_ref()) + pub fn into_diagnostics(self) -> Vec> { + self.data } } @@ -52,15 +52,15 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { + fn message(&self) -> String { + "no such field".to_string() + } fn file(&self) -> HirFileId { self.file } fn syntax_node(&self) -> SyntaxNodePtr { self.field.into() } - fn message(&self) -> String { - "no such field".to_string() - } fn as_any(&self) -> &(Any + Send + 'static) { self } @@ -74,15 +74,15 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { + fn message(&self) -> String { + "unresolved module".to_string() + } fn file(&self) -> HirFileId { self.file } fn syntax_node(&self) -> SyntaxNodePtr { self.decl.into() } - fn message(&self) -> String { - "unresolved module".to_string() - } fn as_any(&self) -> &(Any + Send + 'static) { self } diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index 416f114b40..56ed872d59 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -56,17 +56,16 @@ mod tests; use std::sync::Arc; use rustc_hash::FxHashMap; -use relative_path::RelativePathBuf; use ra_arena::{Arena, RawId, impl_arena_id}; use ra_db::{FileId, Edition}; -use ra_syntax::{AstNode, AstPtr, ast}; use test_utils::tested_by; use crate::{ ModuleDef, Name, Crate, Module, DefDatabase, Path, PathKind, HirFileId, ids::{SourceItemId, SourceFileItemId, MacroCallId}, - diagnostics::{Diagnostics, UnresolvedModule}, + diagnostics::DiagnosticSink, + nameres::diagnostics::DefDiagnostic, }; pub(crate) use self::raw::{RawItems, ImportId, ImportSourceMap}; @@ -228,7 +227,7 @@ impl CrateDefMap { &self, db: &impl DefDatabase, module: CrateModuleId, - sink: &mut Diagnostics, + sink: &mut DiagnosticSink, ) { self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink)) } @@ -446,30 +445,47 @@ impl CrateDefMap { } } -#[derive(Debug, PartialEq, Eq)] -enum DefDiagnostic { - UnresolvedModule { - module: CrateModuleId, - declaration: SourceItemId, - candidate: RelativePathBuf, - }, -} +mod diagnostics { + use relative_path::RelativePathBuf; + use ra_syntax::{AstPtr, AstNode, ast}; -impl DefDiagnostic { - fn add_to(&self, db: &impl DefDatabase, target_module: CrateModuleId, sink: &mut Diagnostics) { - match self { - DefDiagnostic::UnresolvedModule { module, declaration, candidate } => { - if *module != target_module { - return; + use crate::{ + SourceItemId, DefDatabase, + nameres::CrateModuleId, + diagnostics::{DiagnosticSink, UnresolvedModule}, +}; + + #[derive(Debug, PartialEq, Eq)] + pub(super) enum DefDiagnostic { + UnresolvedModule { + module: CrateModuleId, + declaration: SourceItemId, + candidate: RelativePathBuf, + }, + } + + impl DefDiagnostic { + pub(super) fn add_to( + &self, + db: &impl DefDatabase, + target_module: CrateModuleId, + sink: &mut DiagnosticSink, + ) { + match self { + DefDiagnostic::UnresolvedModule { module, declaration, candidate } => { + if *module != target_module { + return; + } + let syntax = db.file_item(*declaration); + let decl = ast::Module::cast(&syntax).unwrap(); + sink.push(UnresolvedModule { + file: declaration.file_id, + decl: AstPtr::new(&decl), + candidate: candidate.clone(), + }) } - let syntax = db.file_item(*declaration); - let decl = ast::Module::cast(&syntax).unwrap(); - sink.push(UnresolvedModule { - file: declaration.file_id, - decl: AstPtr::new(&decl), - candidate: candidate.clone(), - }) } } } + } diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs index bc6bc5e7fd..8830b4624a 100644 --- a/crates/ra_hir/src/nameres/collector.rs +++ b/crates/ra_hir/src/nameres/collector.rs @@ -8,12 +8,15 @@ use crate::{ Function, Module, Struct, Enum, Const, Static, Trait, TypeAlias, DefDatabase, HirFileId, Name, Path, Crate, KnownName, - nameres::{Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, raw, DefDiagnostic}, + nameres::{ + Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, + CrateDefMap, CrateModuleId, ModuleData, CrateMacroId, + diagnostics::DefDiagnostic, + raw, + }, ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId}, }; -use super::{CrateDefMap, CrateModuleId, ModuleData, CrateMacroId}; - pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> CrateDefMap { // populate external prelude for dep in def_map.krate.dependencies(db) { diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 6dc3edc7a1..5fd602a9e6 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -38,7 +38,7 @@ use crate::{ resolve::{Resolver, Resolution}, nameres::Namespace, ty::infer::diagnostics::InferenceDiagnostic, - diagnostics::Diagnostics, + diagnostics::DiagnosticSink, }; use super::{Ty, TypableDef, Substs, primitive, op, FnSig, ApplicationTy, TypeCtor}; @@ -120,7 +120,7 @@ impl InferenceResult { &self, db: &impl HirDatabase, owner: Function, - sink: &mut Diagnostics, + sink: &mut DiagnosticSink, ) { self.diagnostics.iter().for_each(|it| it.add_to(db, owner, sink)) } @@ -1269,7 +1269,7 @@ impl Expectation { } mod diagnostics { - use crate::{expr::ExprId, diagnostics::{Diagnostics, NoSuchField}, HirDatabase, Function}; + use crate::{expr::ExprId, diagnostics::{DiagnosticSink, NoSuchField}, HirDatabase, Function}; #[derive(Debug, PartialEq, Eq, Clone)] pub(super) enum InferenceDiagnostic { @@ -1281,7 +1281,7 @@ mod diagnostics { &self, db: &impl HirDatabase, owner: Function, - sink: &mut Diagnostics, + sink: &mut DiagnosticSink, ) { match self { InferenceDiagnostic::NoSuchField { expr, field } => { diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 254342c0a6..1395cede21 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -129,7 +129,7 @@ fn check_struct_shorthand_initialization( } fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Module) { - let mut diagnostics = hir::diagnostics::Diagnostics::default(); + let mut diagnostics = hir::diagnostics::DiagnosticSink::default(); module.diagnostics(db, &mut diagnostics); for decl in module.declarations(db) { match decl { @@ -138,7 +138,7 @@ fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Modul } } - for d in diagnostics.iter() { + for d in diagnostics.into_diagnostics().iter() { if let Some(d) = d.downcast_ref::() { let source_root = db.file_source_root(d.file().original_file(db)); let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; From 45fbab2b1ac02dab971d245c45c2404494cb3e03 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 21:17:05 +0300 Subject: [PATCH 05/11] check impls as well --- crates/ra_hir/src/nameres.rs | 2 +- crates/ra_ide_api/src/diagnostics.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index 56ed872d59..cdf5023687 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -192,7 +192,7 @@ impl CrateDefMap { CrateDefMap { krate, edition, - extern_prelude: FxHashMap::default(), + xextern_prelude: FxHashMap::default(), prelude: None, root, modules, diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 1395cede21..e03dcaa8fe 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -138,6 +138,15 @@ fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Modul } } + for impl_block in module.impl_blocks(db) { + for item in impl_block.items(db) { + match item { + hir::ImplItem::Method(f) => f.diagnostics(db, &mut diagnostics), + _ => (), + } + } + } + for d in diagnostics.into_diagnostics().iter() { if let Some(d) = d.downcast_ref::() { let source_root = db.file_source_root(d.file().original_file(db)); From 7ee2887d1ec129afed80845c2361ce35f1a0c013 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 21:20:49 +0300 Subject: [PATCH 06/11] fixes --- crates/ra_hir/src/nameres.rs | 2 +- crates/ra_ide_api/src/diagnostics.rs | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index cdf5023687..56ed872d59 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -192,7 +192,7 @@ impl CrateDefMap { CrateDefMap { krate, edition, - xextern_prelude: FxHashMap::default(), + extern_prelude: FxHashMap::default(), prelude: None, root, modules, diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index e03dcaa8fe..5b1a90c82b 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -1,10 +1,9 @@ use itertools::Itertools; -use hir::{source_binder, diagnostics::Diagnostic as _}; +use hir::{source_binder, diagnostics::{Diagnostic as _, DiagnosticSink}}; use ra_db::SourceDatabase; use ra_syntax::{ Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, ast::{self, AstNode}, - }; use ra_text_edit::{TextEdit, TextEditBuilder}; @@ -129,7 +128,7 @@ fn check_struct_shorthand_initialization( } fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Module) { - let mut diagnostics = hir::diagnostics::DiagnosticSink::default(); + let mut diagnostics = DiagnosticSink::default(); module.diagnostics(db, &mut diagnostics); for decl in module.declarations(db) { match decl { From c7ffd939f670a1cba5bf415759b43e63700761a7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 24 Mar 2019 10:21:36 +0300 Subject: [PATCH 07/11] more enterprisey diagnostics setup --- crates/ra_hir/src/code_model_api.rs | 16 ++++++ crates/ra_hir/src/diagnostics.rs | 39 ++++++++++---- crates/ra_ide_api/src/diagnostics.rs | 79 +++++++++++----------------- 3 files changed, 75 insertions(+), 59 deletions(-) diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index bc0f74c896..5437133b87 100644 --- a/crates/ra_hir/src/code_model_api.rs +++ b/crates/ra_hir/src/code_model_api.rs @@ -168,6 +168,22 @@ impl Module { pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut DiagnosticSink) { db.crate_def_map(self.krate).add_diagnostics(db, self.module_id, sink); + for decl in self.declarations(db) { + match decl { + crate::ModuleDef::Function(f) => f.diagnostics(db, sink), + crate::ModuleDef::Module(f) => f.diagnostics(db, sink), + _ => (), + } + } + + for impl_block in self.impl_blocks(db) { + for item in impl_block.items(db) { + match item { + crate::ImplItem::Method(f) => f.diagnostics(db, sink), + _ => (), + } + } + } } pub fn resolver(&self, db: &impl HirDatabase) -> Resolver { diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index d6b28159e9..a6ca68d863 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,9 +1,9 @@ use std::{fmt, any::Any}; use ra_syntax::{SyntaxNodePtr, AstPtr, ast}; +use relative_path::RelativePathBuf; use crate::HirFileId; -use relative_path::RelativePathBuf; /// Diagnostic defines hir API for errors and warnings. /// @@ -21,7 +21,7 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; fn file(&self) -> HirFileId; fn syntax_node(&self) -> SyntaxNodePtr; - fn as_any(&self) -> &(Any + Send + 'static); + fn as_any(&self) -> &(dyn Any + Send + 'static); } impl dyn Diagnostic { @@ -30,18 +30,37 @@ impl dyn Diagnostic { } } -#[derive(Debug, Default)] -pub struct DiagnosticSink { - data: Vec>, +pub struct DiagnosticSink<'a> { + callbacks: Vec Result<(), ()> + 'a>>, + default_callback: Box, } -impl DiagnosticSink { - pub fn push(&mut self, d: impl Diagnostic) { - self.data.push(Box::new(d)) +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 into_diagnostics(self) -> Vec> { - self.data + 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) } } diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 5b1a90c82b..bf77c9ab12 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -1,3 +1,5 @@ +use std::cell::RefCell; + use itertools::Itertools; use hir::{source_binder, diagnostics::{Diagnostic as _, DiagnosticSink}}; use ra_db::SourceDatabase; @@ -25,11 +27,36 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec check_unnecessary_braces_in_use_statement(&mut res, file_id, node); check_struct_shorthand_initialization(&mut res, file_id, node); } - + let res = RefCell::new(res); + let mut sink = DiagnosticSink::new(|d| { + res.borrow_mut().push(Diagnostic { + message: d.message(), + range: d.syntax_node().range(), + severity: Severity::Error, + fix: None, + }) + }) + .on::(|d| { + let source_root = db.file_source_root(d.file().original_file(db)); + let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; + let fix = SourceChange { + label: "create module".to_string(), + source_file_edits: Vec::new(), + file_system_edits: vec![create_file], + cursor_position: None, + }; + res.borrow_mut().push(Diagnostic { + range: d.syntax_node().range(), + message: d.message(), + severity: Severity::Error, + fix: Some(fix), + }) + }); if let Some(m) = source_binder::module_from_file_id(db, file_id) { - check_module(&mut res, db, m); + m.diagnostics(db, &mut sink); }; - res + drop(sink); + res.into_inner() } fn syntax_errors(acc: &mut Vec, source_file: &SourceFile) { @@ -127,52 +154,6 @@ fn check_struct_shorthand_initialization( Some(()) } -fn check_module(acc: &mut Vec, db: &RootDatabase, module: hir::Module) { - let mut diagnostics = DiagnosticSink::default(); - module.diagnostics(db, &mut diagnostics); - for decl in module.declarations(db) { - match decl { - hir::ModuleDef::Function(f) => f.diagnostics(db, &mut diagnostics), - _ => (), - } - } - - for impl_block in module.impl_blocks(db) { - for item in impl_block.items(db) { - match item { - hir::ImplItem::Method(f) => f.diagnostics(db, &mut diagnostics), - _ => (), - } - } - } - - for d in diagnostics.into_diagnostics().iter() { - if let Some(d) = d.downcast_ref::() { - let source_root = db.file_source_root(d.file().original_file(db)); - let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; - let fix = SourceChange { - label: "create module".to_string(), - source_file_edits: Vec::new(), - file_system_edits: vec![create_file], - cursor_position: None, - }; - acc.push(Diagnostic { - range: d.syntax_node().range(), - message: d.message(), - severity: Severity::Error, - fix: Some(fix), - }) - } else { - acc.push(Diagnostic { - message: d.message(), - range: d.syntax_node().range(), - severity: Severity::Error, - fix: None, - }) - } - } -} - #[cfg(test)] mod tests { use test_utils::assert_eq_text; From 4c4a714328490d7f2626272663827fd51dfab0bd Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 24 Mar 2019 11:39:47 +0300 Subject: [PATCH 08/11] test diagnostics --- crates/ra_hir/src/mock.rs | 20 +++++++++++++++++++- crates/ra_hir/src/nameres/tests.rs | 18 ++++++++++++++++++ crates/ra_hir/src/ty/tests.rs | 24 ++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs index 10d4c1b8cc..08f927bb46 100644 --- a/crates/ra_hir/src/mock.rs +++ b/crates/ra_hir/src/mock.rs @@ -9,7 +9,7 @@ use relative_path::RelativePathBuf; use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset}; use rustc_hash::FxHashMap; -use crate::{db, HirInterner}; +use crate::{db, HirInterner, diagnostics::DiagnosticSink, DefDatabase}; pub const WORKSPACE: SourceRootId = SourceRootId(0); @@ -70,6 +70,24 @@ impl MockDatabase { self.set_crate_graph(Arc::new(crate_graph)) } + pub fn diagnostics(&self) -> String { + let mut buf = String::from("\n"); + let mut files: Vec = self.files.values().map(|&it| it).collect(); + files.sort(); + for file in files { + let module = crate::source_binder::module_from_file_id(self, file).unwrap(); + module.diagnostics( + self, + &mut DiagnosticSink::new(|d| { + let source_file = self.hir_parse(d.file()); + let syntax_node = d.syntax_node().to_node(&source_file); + buf += &format!("{:?}: {}\n", syntax_node.text(), d.message()); + }), + ) + } + buf + } + fn from_fixture(fixture: &str) -> (MockDatabase, Option) { let mut db = MockDatabase::default(); diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs index ac9b88520e..277b0757c4 100644 --- a/crates/ra_hir/src/nameres/tests.rs +++ b/crates/ra_hir/src/nameres/tests.rs @@ -552,3 +552,21 @@ foo: v "### ); } + +#[test] +fn unresolved_module_diagnostics() { + let diagnostics = MockDatabase::with_files( + r" + //- /lib.rs + mod foo; + mod bar; + //- /foo.rs + ", + ) + .diagnostics(); + + assert_snapshot_matches!(diagnostics, @r###" +"mod bar;": unresolved module +"### + ); +} diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs index 5d8ad4aa75..3aedba2431 100644 --- a/crates/ra_hir/src/ty/tests.rs +++ b/crates/ra_hir/src/ty/tests.rs @@ -2319,3 +2319,27 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() { assert!(!format!("{:?}", events).contains("infer"), "{:#?}", events) } } + +#[test] +fn no_such_field_diagnostics() { + let diagnostics = MockDatabase::with_files( + r" + //- /lib.rs + struct S { foo: i32, bar: () } + impl S { + fn new() -> S { + S { + foo: 92, + baz: 62, + } + } + } + ", + ) + .diagnostics(); + + assert_snapshot_matches!(diagnostics, @r###" +"baz: 62": no such field +"### + ); +} From 5ce84f3cbc5d5a3106ea89460da4a3f473618f32 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 25 Mar 2019 09:40:49 +0300 Subject: [PATCH 09/11] tweak diagnostics API --- crates/ra_hir/src/diagnostics.rs | 17 ++++++++++++----- crates/ra_hir/src/mock.rs | 6 ++---- crates/ra_ide_api/src/diagnostics.rs | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index a6ca68d863..d6a51b8332 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,9 +1,9 @@ use std::{fmt, any::Any}; -use ra_syntax::{SyntaxNodePtr, AstPtr, ast}; +use ra_syntax::{SyntaxNodePtr, TreeArc, AstPtr, TextRange, ast, SyntaxNode}; use relative_path::RelativePathBuf; -use crate::HirFileId; +use crate::{HirFileId, HirDatabase}; /// Diagnostic defines hir API for errors and warnings. /// @@ -20,11 +20,18 @@ use crate::HirFileId; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; fn file(&self) -> HirFileId; - fn syntax_node(&self) -> SyntaxNodePtr; + fn syntax_node_ptr(&self) -> SyntaxNodePtr; + fn highlight_range(&self) -> TextRange { + self.syntax_node_ptr().range() + } fn as_any(&self) -> &(dyn Any + Send + 'static); } impl dyn Diagnostic { + pub fn syntax_node(&self, db: &impl HirDatabase) -> TreeArc { + let source_file = db.hir_parse(self.file()); + self.syntax_node_ptr().to_node(&source_file).to_owned() + } pub fn downcast_ref(&self) -> Option<&D> { self.as_any().downcast_ref() } @@ -77,7 +84,7 @@ impl Diagnostic for NoSuchField { fn file(&self) -> HirFileId { self.file } - fn syntax_node(&self) -> SyntaxNodePtr { + fn syntax_node_ptr(&self) -> SyntaxNodePtr { self.field.into() } fn as_any(&self) -> &(Any + Send + 'static) { @@ -99,7 +106,7 @@ impl Diagnostic for UnresolvedModule { fn file(&self) -> HirFileId { self.file } - fn syntax_node(&self) -> SyntaxNodePtr { + fn syntax_node_ptr(&self) -> SyntaxNodePtr { self.decl.into() } fn as_any(&self) -> &(Any + Send + 'static) { diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs index 08f927bb46..aeab6b1800 100644 --- a/crates/ra_hir/src/mock.rs +++ b/crates/ra_hir/src/mock.rs @@ -9,7 +9,7 @@ use relative_path::RelativePathBuf; use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset}; use rustc_hash::FxHashMap; -use crate::{db, HirInterner, diagnostics::DiagnosticSink, DefDatabase}; +use crate::{db, HirInterner, diagnostics::DiagnosticSink}; pub const WORKSPACE: SourceRootId = SourceRootId(0); @@ -79,9 +79,7 @@ impl MockDatabase { module.diagnostics( self, &mut DiagnosticSink::new(|d| { - let source_file = self.hir_parse(d.file()); - let syntax_node = d.syntax_node().to_node(&source_file); - buf += &format!("{:?}: {}\n", syntax_node.text(), d.message()); + buf += &format!("{:?}: {}\n", d.syntax_node(self).text(), d.message()); }), ) } diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index bf77c9ab12..11391ef02e 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -31,7 +31,7 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec let mut sink = DiagnosticSink::new(|d| { res.borrow_mut().push(Diagnostic { message: d.message(), - range: d.syntax_node().range(), + range: d.highlight_range(), severity: Severity::Error, fix: None, }) @@ -46,7 +46,7 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec cursor_position: None, }; res.borrow_mut().push(Diagnostic { - range: d.syntax_node().range(), + range: d.highlight_range(), message: d.message(), severity: Severity::Error, fix: Some(fix), From e9af69d9db8856d79f0da7ae7da66169cc225aac Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 25 Mar 2019 10:56:55 +0300 Subject: [PATCH 10/11] simplify --- crates/ra_ide_api/src/diagnostics.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 11391ef02e..8411033229 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -39,12 +39,7 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec .on::(|d| { let source_root = db.file_source_root(d.file().original_file(db)); let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() }; - let fix = SourceChange { - label: "create module".to_string(), - source_file_edits: Vec::new(), - file_system_edits: vec![create_file], - cursor_position: None, - }; + let fix = SourceChange::file_system_edit("create module", create_file); res.borrow_mut().push(Diagnostic { range: d.highlight_range(), message: d.message(), From 309716cffe93d065bcad0344b0f332425576c1e5 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 25 Mar 2019 14:28:04 +0300 Subject: [PATCH 11/11] move tests to where they belong --- crates/ra_hir/src/nameres/tests.rs | 1 + crates/ra_ide_api/src/diagnostics.rs | 31 +++++++++++++++++++ crates/ra_ide_api/tests/test/main.rs | 16 ---------- .../test__unresolved_module_diagnostic.snap | 28 ----------------- 4 files changed, 32 insertions(+), 44 deletions(-) delete mode 100644 crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs index 277b0757c4..572bd1bf74 100644 --- a/crates/ra_hir/src/nameres/tests.rs +++ b/crates/ra_hir/src/nameres/tests.rs @@ -560,6 +560,7 @@ fn unresolved_module_diagnostics() { //- /lib.rs mod foo; mod bar; + mod baz {} //- /foo.rs ", ) diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 8411033229..5a78e94d82 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -152,6 +152,9 @@ fn check_struct_shorthand_initialization( #[cfg(test)] mod tests { use test_utils::assert_eq_text; + use insta::assert_debug_snapshot_matches; + + use crate::mock_analysis::single_file; use super::*; @@ -180,6 +183,34 @@ mod tests { assert_eq_text!(after, &actual); } + #[test] + fn test_unresolved_module_diagnostic() { + let (analysis, file_id) = single_file("mod foo;"); + let diagnostics = analysis.diagnostics(file_id).unwrap(); + assert_debug_snapshot_matches!(diagnostics, @r####"[ + Diagnostic { + message: "unresolved module", + range: [0; 8), + fix: Some( + SourceChange { + label: "create module", + source_file_edits: [], + file_system_edits: [ + CreateFile { + source_root: SourceRootId( + 0 + ), + path: "foo.rs" + } + ], + cursor_position: None + } + ), + severity: Error + } +]"####); + } + #[test] fn test_check_unnecessary_braces_in_use_statement() { check_not_applicable( diff --git a/crates/ra_ide_api/tests/test/main.rs b/crates/ra_ide_api/tests/test/main.rs index 0f0766f621..d4ff21c09c 100644 --- a/crates/ra_ide_api/tests/test/main.rs +++ b/crates/ra_ide_api/tests/test/main.rs @@ -1,4 +1,3 @@ -use insta::assert_debug_snapshot_matches; use ra_ide_api::{ mock_analysis::{single_file, single_file_with_position, single_file_with_range, MockAnalysis}, AnalysisChange, CrateGraph, Edition::Edition2018, Query, NavigationTarget, @@ -6,21 +5,6 @@ use ra_ide_api::{ }; use ra_syntax::SmolStr; -#[test] -fn test_unresolved_module_diagnostic() { - let (analysis, file_id) = single_file("mod foo;"); - let diagnostics = analysis.diagnostics(file_id).unwrap(); - assert_debug_snapshot_matches!("unresolved_module_diagnostic", &diagnostics); -} - -// FIXME: move this test to hir -#[test] -fn test_unresolved_module_diagnostic_no_diag_for_inline_mode() { - let (analysis, file_id) = single_file("mod foo {}"); - let diagnostics = analysis.diagnostics(file_id).unwrap(); - assert!(diagnostics.is_empty()); -} - #[test] fn test_resolve_crate_root() { let mock = MockAnalysis::with_files( diff --git a/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap b/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap deleted file mode 100644 index 5bb9538922..0000000000 --- a/crates/ra_ide_api/tests/test/snapshots/test__unresolved_module_diagnostic.snap +++ /dev/null @@ -1,28 +0,0 @@ ---- -created: "2019-01-22T14:45:01.486985900+00:00" -creator: insta@0.4.0 -expression: "&diagnostics" -source: "crates\\ra_ide_api\\tests\\test\\main.rs" ---- -[ - Diagnostic { - message: "unresolved module", - range: [0; 8), - fix: Some( - SourceChange { - label: "create module", - source_file_edits: [], - file_system_edits: [ - CreateFile { - source_root: SourceRootId( - 0 - ), - path: "foo.rs" - } - ], - cursor_position: None - } - ), - severity: Error - } -]