From 3fb88e95aa5e122a521beec766d5b1264ca4de3b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 23 Mar 2019 18:35:14 +0300 Subject: 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(-) (limited to 'crates') diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs index a37d960a1..03b1acef3 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 52a33e981..14237060c 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 46a3fdd47..e8bb1ed92 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 390aef0a9..ce54d7608 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 d361cf9e6..416f114b4 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 c5b73cfbe..bc6bc5e7f 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 02708ba0f..6dc3edc7a 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 943fd2f53..254342c0a 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, + }) + } } } -- cgit v1.2.3