Merge #7966

7966: Diagnose files that aren't in the module tree r=jonas-schievink a=jonas-schievink

Fixes #6377

I'm not sure if this is the best way to do this. It will cause false positives for all `include!`d files (though I'm not sure how much IDE functionality we have for these).

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

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"

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::<UnlinkedFile, _>(|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::<hir::diagnostics::UnresolvedProcMacro, _>(|d| {
             // Use more accurate position if available.
             let display_range = d
@@ -197,9 +211,13 @@ 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 => {
+            sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(&parse.tree().syntax()) });
+        }
+    }
+
     drop(sink);
     res.into_inner()
 }
@@ -307,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) {
@@ -975,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
+"#,
+        );
+    }
 }

crates/ide/src/diagnostics/unlinked_file.rs

@@ -0,0 +1,156 @@
+//! Diagnostic emitted for files that aren't part of any crate.
+
+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(crate) struct UnlinkedFile {
+    pub(crate) file_id: FileId,
+    pub(crate) 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<SyntaxNodePtr> {
+        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<RootDatabase>) -> Option<Fix> {
+        // 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<Fix> {
+    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,
+    ))
+}