Auto merge of #13934 - Veykril:unlinked-file-inline-modules, r=Veykril

feat: Make unlinked_file diagnostic quickfixes work for inline modules

Finally got myself to fix this, bothered me quite a bit that this never worked
![Code_Qe3WlMvt5Q](https://user-images.githubusercontent.com/3757771/211927799-023e48ee-7cdd-4dd7-8e25-a23eddc7d897.gif)

(Just gotta fix up the indentation still)
This commit is contained in:
bors 2023-01-12 15:25:12 +00:00
commit fb39efe26c
4 changed files with 204 additions and 52 deletions

View file

@ -356,6 +356,14 @@ impl HirFileId {
}
}
#[inline]
pub fn file_id(self) -> Option<FileId> {
match self.0 & Self::MACRO_FILE_TAG_MASK {
0 => Some(FileId(self.0)),
_ => None,
}
}
fn repr(self) -> HirFileIdRepr {
match self.0 & Self::MACRO_FILE_TAG_MASK {
0 => HirFileIdRepr::FileId(FileId(self.0)),

View file

@ -1,13 +1,15 @@
//! Diagnostic emitted for files that aren't part of any crate.
use hir::db::DefDatabase;
use std::iter;
use hir::{db::DefDatabase, InFile, ModuleSource};
use ide_db::{
base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt},
source_change::SourceChange,
RootDatabase,
};
use syntax::{
ast::{self, HasModuleItem, HasName},
ast::{self, edit::IndentLevel, HasModuleItem, HasName},
AstNode, TextRange, TextSize,
};
use text_edit::TextEdit;
@ -42,47 +44,99 @@ fn fixes(ctx: &DiagnosticsContext<'_>, file_id: FileId) -> Option<Vec<Assist>> {
let source_root = ctx.sema.db.source_root(ctx.sema.db.file_source_root(file_id));
let our_path = source_root.path_for_file(&file_id)?;
let (mut module_name, _) = our_path.name_and_extension()?;
// Candidates to look for:
// - `mod.rs`, `main.rs` and `lib.rs` in the same folder
// - `$dir.rs` in the parent folder, where `$dir` is the directory containing `self.file_id`
let parent = our_path.parent()?;
let paths = {
let parent = if module_name == "mod" {
// for mod.rs we need to actually look up one higher
// and take the parent as our to be module name
let (module_name, _) = our_path.name_and_extension()?;
let (parent, module_name) = match module_name {
// for mod.rs we need to actually look up one higher
// and take the parent as our to be module name
"mod" => {
let (name, _) = parent.name_and_extension()?;
module_name = name;
parent.parent()?
} else {
parent
};
let mut paths =
vec![parent.join("mod.rs")?, parent.join("lib.rs")?, parent.join("main.rs")?];
// `submod/bla.rs` -> `submod.rs`
let parent_mod = (|| {
let (name, _) = parent.name_and_extension()?;
parent.parent()?.join(&format!("{name}.rs"))
})();
paths.extend(parent_mod);
paths
(parent.parent()?, name.to_owned())
}
_ => (parent, module_name.to_owned()),
};
for &parent_id in paths.iter().filter_map(|path| source_root.file_for_path(path)) {
for &krate in ctx.sema.db.relevant_crates(parent_id).iter() {
let crate_def_map = ctx.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;
}
// check crate roots, i.e. main.rs, lib.rs, ...
'crates: for &krate in &*ctx.sema.db.relevant_crates(file_id) {
let crate_def_map = ctx.sema.db.crate_def_map(krate);
if module.origin.file_id() == Some(parent_id) {
return make_fixes(ctx.sema.db, parent_id, module_name, file_id);
let root_module = &crate_def_map[crate_def_map.root()];
let Some(root_file_id) = root_module.origin.file_id() else { continue };
let Some(crate_root_path) = source_root.path_for_file(&root_file_id) else { continue };
let Some(rel) = parent.strip_prefix(&crate_root_path.parent()?) else { continue };
// try resolving the relative difference of the paths as inline modules
let mut current = root_module;
for ele in rel.as_ref().components() {
let seg = match ele {
std::path::Component::Normal(seg) => seg.to_str()?,
std::path::Component::RootDir => continue,
// shouldn't occur
_ => continue 'crates,
};
match current.children.iter().find(|(name, _)| name.to_smol_str() == seg) {
Some((_, &child)) => current = &crate_def_map[child],
None => continue 'crates,
}
if !current.origin.is_inline() {
continue 'crates;
}
}
let InFile { file_id: parent_file_id, value: source } =
current.definition_source(ctx.sema.db);
let parent_file_id = parent_file_id.file_id()?;
return make_fixes(ctx.sema.db, parent_file_id, source, &module_name, file_id);
}
// if we aren't adding to a crate root, walk backwards such that we support `#[path = ...]` overrides if possible
// build all parent paths of the form `../module_name/mod.rs` and `../module_name.rs`
let paths = iter::successors(Some(parent.clone()), |prev| prev.parent()).filter_map(|path| {
let parent = path.parent()?;
let (name, _) = path.name_and_extension()?;
Some(([parent.join(&format!("{name}.rs"))?, path.join("mod.rs")?], name.to_owned()))
});
let mut stack = vec![];
let &parent_id =
paths.inspect(|(_, name)| stack.push(name.clone())).find_map(|(paths, _)| {
paths.into_iter().find_map(|path| source_root.file_for_path(&path))
})?;
stack.pop();
'crates: for &krate in ctx.sema.db.relevant_crates(parent_id).iter() {
let crate_def_map = ctx.sema.db.crate_def_map(krate);
let Some((_, module)) =
crate_def_map.modules()
.find(|(_, module)| module.origin.file_id() == Some(parent_id) && !module.origin.is_inline())
else { continue };
if stack.is_empty() {
return make_fixes(
ctx.sema.db,
parent_id,
module.definition_source(ctx.sema.db).value,
&module_name,
file_id,
);
} else {
// direct parent file is missing,
// try finding a parent that has an inline tree from here on
let mut current = module;
for s in stack.iter().rev() {
match module.children.iter().find(|(name, _)| name.to_smol_str() == s) {
Some((_, child)) => {
current = &crate_def_map[*child];
}
None => continue 'crates,
}
if !current.origin.is_inline() {
continue 'crates;
}
}
let InFile { file_id: parent_file_id, value: source } =
current.definition_source(ctx.sema.db);
let parent_file_id = parent_file_id.file_id()?;
return make_fixes(ctx.sema.db, parent_file_id, source, &module_name, file_id);
}
}
@ -92,6 +146,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, file_id: FileId) -> Option<Vec<Assist>> {
fn make_fixes(
db: &RootDatabase,
parent_file_id: FileId,
source: ModuleSource,
new_mod_name: &str,
added_file_id: FileId,
) -> Option<Vec<Assist>> {
@ -102,14 +157,18 @@ fn make_fixes(
let mod_decl = format!("mod {new_mod_name};");
let pub_mod_decl = format!("pub mod {new_mod_name};");
let ast: ast::SourceFile = db.parse(parent_file_id).tree();
let mut mod_decl_builder = TextEdit::builder();
let mut pub_mod_decl_builder = TextEdit::builder();
let mut items = match &source {
ModuleSource::SourceFile(it) => it.items(),
ModuleSource::Module(it) => it.item_list()?.items(),
ModuleSource::BlockExpr(_) => return None,
};
// 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() {
for item in items.clone() {
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 {
@ -121,28 +180,40 @@ fn make_fixes(
}
// 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(is_outline_mod).last() {
match items.clone().skip_while(|item| !is_outline_mod(item)).take_while(is_outline_mod).last() {
Some(last) => {
cov_mark::hit!(unlinked_file_append_to_existing_mods);
let offset = last.syntax().text_range().end();
mod_decl_builder.insert(offset, format!("\n{mod_decl}"));
pub_mod_decl_builder.insert(offset, format!("\n{pub_mod_decl}"));
let indent = IndentLevel::from_node(last.syntax());
mod_decl_builder.insert(offset, format!("\n{indent}{mod_decl}"));
pub_mod_decl_builder.insert(offset, format!("\n{indent}{pub_mod_decl}"));
}
None => {
// Prepend before the first item in the file.
match ast.items().next() {
Some(item) => {
match items.next() {
Some(first) => {
cov_mark::hit!(unlinked_file_prepend_before_first_item);
let offset = item.syntax().text_range().start();
mod_decl_builder.insert(offset, format!("{mod_decl}\n\n"));
pub_mod_decl_builder.insert(offset, format!("{pub_mod_decl}\n\n"));
let offset = first.syntax().text_range().start();
let indent = IndentLevel::from_node(first.syntax());
mod_decl_builder.insert(offset, format!("{mod_decl}\n\n{indent}"));
pub_mod_decl_builder.insert(offset, format!("{pub_mod_decl}\n\n{indent}"));
}
None => {
// No items in the file, so just append at the end.
cov_mark::hit!(unlinked_file_empty_file);
let offset = ast.syntax().text_range().end();
mod_decl_builder.insert(offset, format!("{mod_decl}\n"));
pub_mod_decl_builder.insert(offset, format!("{pub_mod_decl}\n"));
let mut indent = IndentLevel::from(0);
let offset = match &source {
ModuleSource::SourceFile(it) => it.syntax().text_range().end(),
ModuleSource::Module(it) => {
indent = IndentLevel::from_node(it.syntax()) + 1;
it.item_list()?.r_curly_token()?.text_range().start()
}
ModuleSource::BlockExpr(it) => {
it.stmt_list()?.r_curly_token()?.text_range().start()
}
};
mod_decl_builder.insert(offset, format!("{indent}{mod_decl}\n"));
pub_mod_decl_builder.insert(offset, format!("{indent}{pub_mod_decl}\n"));
}
}
}
@ -167,7 +238,6 @@ fn make_fixes(
#[cfg(test)]
mod tests {
use crate::tests::{check_diagnostics, check_fix, check_fixes, check_no_fix};
#[test]
@ -330,6 +400,64 @@ $0
mod foo;
//- /foo.rs
"#,
);
}
#[test]
fn unlinked_file_insert_into_inline_simple() {
check_fix(
r#"
//- /main.rs
mod bar;
//- /bar.rs
mod foo {
}
//- /bar/foo/baz.rs
$0
"#,
r#"
mod foo {
mod baz;
}
"#,
);
}
#[test]
fn unlinked_file_insert_into_inline_simple_modrs() {
check_fix(
r#"
//- /main.rs
mod bar;
//- /bar.rs
mod baz {
}
//- /bar/baz/foo/mod.rs
$0
"#,
r#"
mod baz {
mod foo;
}
"#,
);
}
#[test]
fn unlinked_file_insert_into_inline_simple_modrs_main() {
check_fix(
r#"
//- /main.rs
mod bar {
}
//- /bar/foo/mod.rs
$0
"#,
r#"
mod bar {
mod foo;
}
"#,
);
}

View file

@ -17,6 +17,7 @@ use crate::{
// This is the same as `Go to Definition` with the following exceptions:
// - outline modules will navigate to the `mod name;` item declaration
// - trait assoc items will navigate to the assoc item of the trait declaration opposed to the trait impl
// - fields in patterns will navigate to the field declaration of the struct, union or variant
pub(crate) fn goto_declaration(
db: &RootDatabase,
position: FilePosition,

View file

@ -1,7 +1,7 @@
//! Abstract-ish representation of paths for VFS.
use std::fmt;
use paths::{AbsPath, AbsPathBuf};
use paths::{AbsPath, AbsPathBuf, RelPath};
/// Path in [`Vfs`].
///
@ -84,6 +84,14 @@ impl VfsPath {
}
}
pub fn strip_prefix(&self, other: &VfsPath) -> Option<&RelPath> {
match (&self.0, &other.0) {
(VfsPathRepr::PathBuf(lhs), VfsPathRepr::PathBuf(rhs)) => lhs.strip_prefix(rhs),
(VfsPathRepr::VirtualPath(lhs), VfsPathRepr::VirtualPath(rhs)) => lhs.strip_prefix(rhs),
(VfsPathRepr::PathBuf(_) | VfsPathRepr::VirtualPath(_), _) => None,
}
}
/// Returns the `VfsPath` without its final component, if there is one.
///
/// Returns [`None`] if the path is a root or prefix.
@ -320,6 +328,13 @@ impl VirtualPath {
self.0.starts_with(&other.0)
}
fn strip_prefix(&self, base: &VirtualPath) -> Option<&RelPath> {
<_ as AsRef<std::path::Path>>::as_ref(&self.0)
.strip_prefix(&base.0)
.ok()
.map(RelPath::new_unchecked)
}
/// Remove the last component of `self`.
///
/// This will find the last `'/'` in `self`, and remove everything after it,