From 7b1a0d5fb724d3f802fe23f9b4455ffd6d7e114f Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 10 Mar 2021 20:30:20 +0100 Subject: Diagnose files that aren't in the module tree --- crates/ide/src/diagnostics.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index fe32f39b6..760c84780 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -197,9 +197,19 @@ pub(crate) fn diagnostics( ); }); - if let Some(m) = sema.to_module_def(file_id) { - m.diagnostics(db, &mut sink); - }; + match sema.to_module_def(file_id) { + Some(m) => m.diagnostics(db, &mut sink), + None => { + res.borrow_mut().push( + Diagnostic::hint( + parse.tree().syntax().text_range(), + "file not included in module tree".to_string(), + ) + .with_unused(true), + ); + } + } + drop(sink); res.into_inner() } -- cgit v1.2.3 From 8b4cbbb87c6dc946711570096d857603c30a5d8d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 15 Mar 2021 01:39:23 +0100 Subject: Redo it properly and add a quickfix --- crates/ide/Cargo.toml | 1 + crates/ide/src/diagnostics.rs | 161 ++++++++++++++++++++++++++-- crates/ide/src/diagnostics/unlinked_file.rs | 154 ++++++++++++++++++++++++++ 3 files changed, 309 insertions(+), 7 deletions(-) create mode 100644 crates/ide/src/diagnostics/unlinked_file.rs diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 107bd8432..6ae7c9e6c 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -38,3 +38,4 @@ hir = { path = "../hir", version = "0.0.0" } [dev-dependencies] test_utils = { path = "../test_utils" } expect-test = "1.1" +cov-mark = "1.1.0" diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 760c84780..22697a537 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -6,6 +6,7 @@ mod fixes; mod field_shorthand; +mod unlinked_file; use std::cell::RefCell; @@ -22,6 +23,7 @@ use syntax::{ SyntaxNode, SyntaxNodePtr, TextRange, }; use text_edit::TextEdit; +use unlinked_file::UnlinkedFile; use crate::{FileId, Label, SourceChange}; @@ -156,6 +158,18 @@ pub(crate) fn diagnostics( .with_code(Some(d.code())), ); }) + .on::(|d| { + // Override severity and mark as unused. + res.borrow_mut().push( + Diagnostic::hint( + sema.diagnostics_display_range(d.display_source()).range, + d.message(), + ) + .with_unused(true) + .with_fix(d.fix(&sema)) + .with_code(Some(d.code())), + ); + }) .on::(|d| { // Use more accurate position if available. let display_range = d @@ -200,13 +214,7 @@ pub(crate) fn diagnostics( match sema.to_module_def(file_id) { Some(m) => m.diagnostics(db, &mut sink), None => { - res.borrow_mut().push( - Diagnostic::hint( - parse.tree().syntax().text_range(), - "file not included in module tree".to_string(), - ) - .with_unused(true), - ); + sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(&parse.tree().syntax()) }); } } @@ -317,6 +325,17 @@ mod tests { ); } + /// Checks that there's a diagnostic *without* fix at `$0`. + fn check_no_fix(ra_fixture: &str) { + let (analysis, file_position) = fixture::position(ra_fixture); + let diagnostic = analysis + .diagnostics(&DiagnosticsConfig::default(), file_position.file_id) + .unwrap() + .pop() + .unwrap(); + assert!(diagnostic.fix.is_none(), "got a fix when none was expected: {:?}", diagnostic); + } + /// Takes a multi-file input fixture with annotated cursor position and checks that no diagnostics /// apply to the file containing the cursor. pub(crate) fn check_no_diagnostics(ra_fixture: &str) { @@ -985,4 +1004,132 @@ impl TestStruct { check_fix(input, expected); } + + #[test] + fn unlinked_file_prepend_first_item() { + cov_mark::check!(unlinked_file_prepend_before_first_item); + check_fix( + r#" +//- /main.rs +fn f() {} +//- /foo.rs +$0 +"#, + r#" +mod foo; + +fn f() {} +"#, + ); + } + + #[test] + fn unlinked_file_append_mod() { + cov_mark::check!(unlinked_file_append_to_existing_mods); + check_fix( + r#" +//- /main.rs +//! Comment on top + +mod preexisting; + +mod preexisting2; + +struct S; + +mod preexisting_bottom;) +//- /foo.rs +$0 +"#, + r#" +//! Comment on top + +mod preexisting; + +mod preexisting2; +mod foo; + +struct S; + +mod preexisting_bottom;) +"#, + ); + } + + #[test] + fn unlinked_file_insert_in_empty_file() { + cov_mark::check!(unlinked_file_empty_file); + check_fix( + r#" +//- /main.rs +//- /foo.rs +$0 +"#, + r#" +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_old_style_modrs() { + check_fix( + r#" +//- /main.rs +mod submod; +//- /submod/mod.rs +// in mod.rs +//- /submod/foo.rs +$0 +"#, + r#" +// in mod.rs +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_new_style_mod() { + check_fix( + r#" +//- /main.rs +mod submod; +//- /submod.rs +//- /submod/foo.rs +$0 +"#, + r#" +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_with_cfg_off() { + cov_mark::check!(unlinked_file_skip_fix_when_mod_already_exists); + check_no_fix( + r#" +//- /main.rs +#[cfg(never)] +mod foo; + +//- /foo.rs +$0 +"#, + ); + } + + #[test] + fn unlinked_file_with_cfg_on() { + check_no_diagnostics( + r#" +//- /main.rs +#[cfg(not(never))] +mod foo; + +//- /foo.rs +"#, + ); + } } diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs new file mode 100644 index 000000000..f6dc671b9 --- /dev/null +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -0,0 +1,154 @@ +use hir::{ + db::DefDatabase, + diagnostics::{Diagnostic, DiagnosticCode}, + InFile, +}; +use ide_db::{ + base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt}, + source_change::SourceChange, + RootDatabase, +}; +use syntax::{ + ast::{self, ModuleItemOwner, NameOwner}, + AstNode, SyntaxNodePtr, +}; +use text_edit::TextEdit; + +use crate::Fix; + +use super::fixes::DiagnosticWithFix; + +#[derive(Debug)] +pub struct UnlinkedFile { + pub file_id: FileId, + pub node: SyntaxNodePtr, +} + +impl Diagnostic for UnlinkedFile { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("unlinked-file") + } + + fn message(&self) -> String { + "file not included in module tree".to_string() + } + + fn display_source(&self) -> InFile { + InFile::new(self.file_id.into(), self.node.clone()) + } + + fn as_any(&self) -> &(dyn std::any::Any + Send + 'static) { + self + } +} + +impl DiagnosticWithFix for UnlinkedFile { + fn fix(&self, sema: &hir::Semantics) -> Option { + // If there's an existing module that could add a `mod` item to include the unlinked file, + // suggest that as a fix. + + let source_root = sema.db.source_root(sema.db.file_source_root(self.file_id)); + let our_path = source_root.path_for_file(&self.file_id)?; + let module_name = our_path.name_and_extension()?.0; + + // Candidates to look for: + // - `mod.rs` in the same folder + // - we also check `main.rs` and `lib.rs` + // - `$dir.rs` in the parent folder, where `$dir` is the directory containing `self.file_id` + let parent = our_path.parent()?; + let mut paths = + vec![parent.join("mod.rs")?, parent.join("main.rs")?, parent.join("lib.rs")?]; + + // `submod/bla.rs` -> `submod.rs` + if let Some(newmod) = (|| { + let name = parent.name_and_extension()?.0; + parent.parent()?.join(&format!("{}.rs", name)) + })() { + paths.push(newmod); + } + + for path in &paths { + if let Some(parent_id) = source_root.file_for_path(path) { + for krate in sema.db.relevant_crates(*parent_id).iter() { + let crate_def_map = sema.db.crate_def_map(*krate); + for (_, module) in crate_def_map.modules() { + if module.origin.is_inline() { + // We don't handle inline `mod parent {}`s, they use different paths. + continue; + } + + if module.origin.file_id() == Some(*parent_id) { + return make_fix(sema.db, *parent_id, module_name, self.file_id); + } + } + } + } + } + + None + } +} + +fn make_fix( + db: &RootDatabase, + parent_file_id: FileId, + new_mod_name: &str, + added_file_id: FileId, +) -> Option { + fn is_outline_mod(item: &ast::Item) -> bool { + matches!(item, ast::Item::Module(m) if m.item_list().is_none()) + } + + let mod_decl = format!("mod {};", new_mod_name); + let ast: ast::SourceFile = db.parse(parent_file_id).tree(); + + let mut builder = TextEdit::builder(); + + // If there's an existing `mod m;` statement matching the new one, don't emit a fix (it's + // probably `#[cfg]`d out). + for item in ast.items() { + if let ast::Item::Module(m) = item { + if let Some(name) = m.name() { + if m.item_list().is_none() && name.to_string() == new_mod_name { + cov_mark::hit!(unlinked_file_skip_fix_when_mod_already_exists); + return None; + } + } + } + } + + // If there are existing `mod m;` items, append after them (after the first group of them, rather). + match ast + .items() + .skip_while(|item| !is_outline_mod(item)) + .take_while(|item| is_outline_mod(item)) + .last() + { + Some(last) => { + cov_mark::hit!(unlinked_file_append_to_existing_mods); + builder.insert(last.syntax().text_range().end(), format!("\n{}", mod_decl)); + } + None => { + // Prepend before the first item in the file. + match ast.items().next() { + Some(item) => { + cov_mark::hit!(unlinked_file_prepend_before_first_item); + builder.insert(item.syntax().text_range().start(), format!("{}\n\n", mod_decl)); + } + None => { + // No items in the file, so just append at the end. + cov_mark::hit!(unlinked_file_empty_file); + builder.insert(ast.syntax().text_range().end(), format!("{}\n", mod_decl)); + } + } + } + } + + let edit = builder.finish(); + let trigger_range = db.parse(added_file_id).tree().syntax().text_range(); + Some(Fix::new( + &format!("Insert `{}`", mod_decl), + SourceChange::from_text_edit(parent_file_id, edit), + trigger_range, + )) +} -- cgit v1.2.3 From 40638b16c8c014d4d226d25f05ad19b16827cb98 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 15 Mar 2021 01:46:59 +0100 Subject: Use pub(crate) --- crates/ide/src/diagnostics/unlinked_file.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index f6dc671b9..37d32f1a6 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -19,9 +19,9 @@ use crate::Fix; use super::fixes::DiagnosticWithFix; #[derive(Debug)] -pub struct UnlinkedFile { - pub file_id: FileId, - pub node: SyntaxNodePtr, +pub(crate) struct UnlinkedFile { + pub(crate) file_id: FileId, + pub(crate) node: SyntaxNodePtr, } impl Diagnostic for UnlinkedFile { -- cgit v1.2.3 From 32e1ca54eac6b5d401c2f33ec1281794ef9a0a52 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 15 Mar 2021 02:23:00 +0100 Subject: Add module comment --- crates/ide/src/diagnostics/unlinked_file.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index 37d32f1a6..c5741bf6b 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -1,3 +1,5 @@ +//! Diagnostic emitted for files that aren't part of any crate. + use hir::{ db::DefDatabase, diagnostics::{Diagnostic, DiagnosticCode}, -- cgit v1.2.3