From a9dd6063872a0d309862d57b04b9ac5864934578 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 11 Mar 2022 16:49:41 +0100 Subject: [PATCH] fix: Show what file paths were expected for unresolved modules --- Cargo.lock | 1 + crates/hir/src/diagnostics.rs | 2 +- crates/hir/src/lib.rs | 4 +- crates/hir_def/Cargo.toml | 1 + crates/hir_def/src/item_tree.rs | 2 +- crates/hir_def/src/item_tree/lower.rs | 2 +- crates/hir_def/src/item_tree/pretty.rs | 2 +- crates/hir_def/src/nameres/collector.rs | 6 +- crates/hir_def/src/nameres/diagnostics.rs | 6 +- crates/hir_def/src/nameres/mod_resolution.rs | 20 ++--- .../src/handlers/unresolved_module.rs | 76 +++++++++++++++---- crates/rust-analyzer/tests/slow-tests/main.rs | 70 ++++++++++++----- 12 files changed, 133 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5dfc14e8b3..1a17831dc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -504,6 +504,7 @@ name = "hir_def" version = "0.0.0" dependencies = [ "anymap", + "arrayvec", "base_db", "cfg", "cov-mark", diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 16b9a9ea45..b5e7d5db63 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -54,7 +54,7 @@ diagnostics![ #[derive(Debug)] pub struct UnresolvedModule { pub decl: InFile>, - pub candidate: String, + pub candidates: Box<[String]>, } #[derive(Debug)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index a90120a467..cf93a3b9f4 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -593,12 +593,12 @@ impl Module { fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: &DefDiagnostic) { match &diag.kind { - DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate } => { + DefDiagnosticKind::UnresolvedModule { ast: declaration, candidates } => { let decl = declaration.to_node(db.upcast()); acc.push( UnresolvedModule { decl: InFile::new(declaration.file_id, AstPtr::new(&decl)), - candidate: candidate.clone(), + candidates: candidates.clone(), } .into(), ) diff --git a/crates/hir_def/Cargo.toml b/crates/hir_def/Cargo.toml index 794f92b233..8a886f4a96 100644 --- a/crates/hir_def/Cargo.toml +++ b/crates/hir_def/Cargo.toml @@ -24,6 +24,7 @@ fst = { version = "0.4", default-features = false } itertools = "0.10.0" indexmap = "1.7.0" smallvec = "1.4.0" +arrayvec = "0.7.2" la-arena = { version = "0.3.0", path = "../../lib/arena" } stdx = { path = "../stdx", version = "0.0.0" } diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 0af5d654af..2b89ac5613 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -709,7 +709,7 @@ pub enum ModKind { Inline { items: Box<[ModItem]> }, /// `mod m;` - Outline {}, + Outline, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 45b5430a22..516a16bcf0 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -401,7 +401,7 @@ impl<'a> Ctx<'a> { let name = module.name()?.as_name(); let visibility = self.lower_visibility(module); let kind = if module.semicolon_token().is_some() { - ModKind::Outline {} + ModKind::Outline } else { ModKind::Inline { items: module diff --git a/crates/hir_def/src/item_tree/pretty.rs b/crates/hir_def/src/item_tree/pretty.rs index 6fea95a2ac..ca164148a1 100644 --- a/crates/hir_def/src/item_tree/pretty.rs +++ b/crates/hir_def/src/item_tree/pretty.rs @@ -423,7 +423,7 @@ impl<'a> Printer<'a> { }); wln!(self, "}}"); } - ModKind::Outline {} => { + ModKind::Outline => { wln!(self, ";"); } } diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 985789b70e..95a50d942e 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -1713,7 +1713,7 @@ impl ModCollector<'_, '_> { } } // out of line module, resolve, parse and recurse - ModKind::Outline {} => { + ModKind::Outline => { let ast_id = AstId::new(self.tree_id.file_id(), module.ast_id); let db = self.def_collector.db; match self.mod_dir.resolve_declaration(db, self.file_id(), &module.name, path_attr) @@ -1751,9 +1751,9 @@ impl ModCollector<'_, '_> { } } } - Err(candidate) => { + Err(candidates) => { self.def_collector.def_map.diagnostics.push( - DefDiagnostic::unresolved_module(self.module_id, ast_id, candidate), + DefDiagnostic::unresolved_module(self.module_id, ast_id, candidates), ); } }; diff --git a/crates/hir_def/src/nameres/diagnostics.rs b/crates/hir_def/src/nameres/diagnostics.rs index 60b8601bdd..ab7a368268 100644 --- a/crates/hir_def/src/nameres/diagnostics.rs +++ b/crates/hir_def/src/nameres/diagnostics.rs @@ -15,7 +15,7 @@ use crate::{ #[derive(Debug, PartialEq, Eq)] pub enum DefDiagnosticKind { - UnresolvedModule { ast: AstId, candidate: String }, + UnresolvedModule { ast: AstId, candidates: Box<[String]> }, UnresolvedExternCrate { ast: AstId }, @@ -46,11 +46,11 @@ impl DefDiagnostic { pub(super) fn unresolved_module( container: LocalModuleId, declaration: AstId, - candidate: String, + candidates: Box<[String]>, ) -> Self { Self { in_module: container, - kind: DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate }, + kind: DefDiagnosticKind::UnresolvedModule { ast: declaration, candidates }, } } diff --git a/crates/hir_def/src/nameres/mod_resolution.rs b/crates/hir_def/src/nameres/mod_resolution.rs index b6f1b17870..52a620fe22 100644 --- a/crates/hir_def/src/nameres/mod_resolution.rs +++ b/crates/hir_def/src/nameres/mod_resolution.rs @@ -1,4 +1,5 @@ //! This module resolves `mod foo;` declaration to file. +use arrayvec::ArrayVec; use base_db::{AnchoredPath, FileId}; use hir_expand::name::Name; use limit::Limit; @@ -63,22 +64,21 @@ impl ModDir { file_id: HirFileId, name: &Name, attr_path: Option<&SmolStr>, - ) -> Result<(FileId, bool, ModDir), String> { + ) -> Result<(FileId, bool, ModDir), Box<[String]>> { let orig_file_id = file_id.original_file(db.upcast()); - let mut candidate_files = Vec::new(); + let mut candidate_files = ArrayVec::<_, 2>::new(); match attr_path { Some(attr_path) => { candidate_files.push(self.dir_path.join_attr(attr_path, self.root_non_dir_owner)) } + None if file_id.is_include_macro(db.upcast()) => { + candidate_files.push(format!("{}.rs", name)); + candidate_files.push(format!("{}/mod.rs", name)); + } None => { - if file_id.is_include_macro(db.upcast()) { - candidate_files.push(format!("{}.rs", name)); - candidate_files.push(format!("{}/mod.rs", name)); - } else { - candidate_files.push(format!("{}{}.rs", self.dir_path.0, name)); - candidate_files.push(format!("{}{}/mod.rs", self.dir_path.0, name)); - } + candidate_files.push(format!("{}{}.rs", self.dir_path.0, name)); + candidate_files.push(format!("{}{}/mod.rs", self.dir_path.0, name)); } }; @@ -97,7 +97,7 @@ impl ModDir { } } } - Err(candidate_files.remove(0)) + Err(candidate_files.into_iter().collect()) } } diff --git a/crates/ide_diagnostics/src/handlers/unresolved_module.rs b/crates/ide_diagnostics/src/handlers/unresolved_module.rs index 61fc43604f..640ba38a28 100644 --- a/crates/ide_diagnostics/src/handlers/unresolved_module.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_module.rs @@ -1,5 +1,6 @@ use hir::db::AstDatabase; use ide_db::{assists::Assist, base_db::AnchoredPathBuf, source_change::FileSystemEdit}; +use itertools::Itertools; use syntax::AstNode; use crate::{fix, Diagnostic, DiagnosticsContext}; @@ -13,7 +14,17 @@ pub(crate) fn unresolved_module( ) -> Diagnostic { Diagnostic::new( "unresolved-module", - "unresolved module", + match &*d.candidates { + [] => "unresolved module".to_string(), + [candidate] => format!("unresolved module, can't find module file: {}", candidate), + [candidates @ .., last] => { + format!( + "unresolved module, can't find module file: {}, or {}", + candidates.iter().format(", "), + last + ) + } + }, ctx.sema.diagnostics_display_range(d.decl.clone().map(|it| it.into())).range, ) .with_fixes(fixes(ctx, d)) @@ -22,19 +33,26 @@ pub(crate) fn unresolved_module( fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Option> { let root = ctx.sema.db.parse_or_expand(d.decl.file_id)?; let unresolved_module = d.decl.value.to_node(&root); - Some(vec![fix( - "create_module", - "Create module", - FileSystemEdit::CreateFile { - dst: AnchoredPathBuf { - anchor: d.decl.file_id.original_file(ctx.sema.db), - path: d.candidate.clone(), - }, - initial_contents: "".to_string(), - } - .into(), - unresolved_module.syntax().text_range(), - )]) + Some( + d.candidates + .iter() + .map(|candidate| { + fix( + "create_module", + "Create module", + FileSystemEdit::CreateFile { + dst: AnchoredPathBuf { + anchor: d.decl.file_id.original_file(ctx.sema.db), + path: candidate.clone(), + }, + initial_contents: "".to_string(), + } + .into(), + unresolved_module.syntax().text_range(), + ) + }) + .collect(), + ) } #[cfg(test)] @@ -50,7 +68,7 @@ mod tests { //- /lib.rs mod foo; mod bar; -//^^^^^^^^ 💡 error: unresolved module +//^^^^^^^^ 💡 error: unresolved module, can't find module file: bar.rs, or bar/mod.rs mod baz {} //- /foo.rs "#, @@ -67,7 +85,7 @@ mod baz {} code: DiagnosticCode( "unresolved-module", ), - message: "unresolved module", + message: "unresolved module, can't find module file: foo.rs, or foo/mod.rs", range: 0..8, severity: Error, unused: false, @@ -100,6 +118,32 @@ mod baz {} }, ), }, + Assist { + id: AssistId( + "create_module", + QuickFix, + ), + label: "Create module", + group: None, + target: 0..8, + source_change: Some( + SourceChange { + source_file_edits: {}, + file_system_edits: [ + CreateFile { + dst: AnchoredPathBuf { + anchor: FileId( + 0, + ), + path: "foo/mod.rs", + }, + initial_contents: "", + }, + ], + is_snippet: false, + }, + ), + }, ], ), }, diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index 9e555dc49e..9e83995484 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -463,18 +463,32 @@ fn main() {} partial_result_params: PartialResultParams::default(), work_done_progress_params: WorkDoneProgressParams::default(), }, - json!([{ - "edit": { - "documentChanges": [ - { - "kind": "create", - "uri": "file:///[..]/src/bar.rs" + json!([ + { + "title": "Create module", + "kind": "quickfix", + "edit": { + "documentChanges": [ + { + "kind": "create", + "uri": "file://[..]/src/bar.rs" + } + ] } - ] }, - "kind": "quickfix", - "title": "Create module" - }]), + { + "title": "Create module", + "kind": "quickfix", + "edit": { + "documentChanges": [ + { + "kind": "create", + "uri": "file://[..]src/bar/mod.rs" + } + ] + } + } + ]), ); server.request::( @@ -492,7 +506,7 @@ fn main() {} #[test] fn test_missing_module_code_action_in_json_project() { if skip_slow_tests() { - return; + // return; } let tmp_dir = TestDir::new(); @@ -533,18 +547,32 @@ fn main() {{}} partial_result_params: PartialResultParams::default(), work_done_progress_params: WorkDoneProgressParams::default(), }, - json!([{ - "edit": { - "documentChanges": [ - { - "kind": "create", - "uri": "file://[..]/src/bar.rs" + json!([ + { + "title": "Create module", + "kind": "quickfix", + "edit": { + "documentChanges": [ + { + "kind": "create", + "uri": "file://[..]/src/bar.rs" + } + ] } - ] }, - "kind": "quickfix", - "title": "Create module" - }]), + { + "title": "Create module", + "kind": "quickfix", + "edit": { + "documentChanges": [ + { + "kind": "create", + "uri": "file://[..]src/bar/mod.rs" + } + ] + } + } + ]), ); server.request::(