Skip to content

Commit

Permalink
fallback to dir_path when relative external mod resolution fails
Browse files Browse the repository at this point in the history
We only want to fall back if two conditions are met:

1) Initial module resolution is performed relative to some nested
   directory.
2) Module resolution fails becuase of a ModError::FileNotFound error.

When these conditions are met we can try to fallback to searching for
the module's file relative to the dir_path instead of the nested
relative directory.

Fixes 5198

As demonstraited by 5198, it's possible that a directory name conflicts
with a rust file name. For example, src/lib/ and src/lib.rs.

If src/lib.rs references an external module like ``mod foo;``, then
module resolution will try to resolve ``foo`` to src/lib/foo.rs or
src/lib/foo/mod.rs. Module resolution would fail with a file not
found error if the ``foo`` module were defined at src/foo.rs.

When encountering these kinds of module resolution issues we now fall
back to the current directory and attempt to resolve the module again.

Given the current example, this means that if we can't find the module
``foo`` at src/lib/foo.rs or src/lib/foo/mod.rs, we'll attempt
to resolve the module to src/foo.rs.
  • Loading branch information
ytmimi committed Jan 30, 2022
1 parent 8b0b213 commit 296d51a
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/parse/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_span::{

use crate::config::file_lines::LineRange;
use crate::ignore_path::IgnorePathSet;
use crate::parse::parser::{ModError, ModulePathSuccess};
use crate::source_map::LineRangeUtils;
use crate::utils::starts_with_newline;
use crate::visitor::SnippetProvider;
Expand Down Expand Up @@ -145,13 +146,30 @@ impl ParseSess {
})
}

/// Determine the submodule path for the given module identifier.
///
/// * `id` - The name of the module
/// * `relative` - If Some(symbol), the symbol name is a directory relative to the dir_path.
/// If relative is Some, resolve the submodle at {dir_path}/{symbol}/{id}.rs
/// or {dir_path}/{symbol}/{id}/mod.rs. if None, resolve the module at {dir_path}/{id}.rs.
/// * `dir_path` - Module resolution will occur relative to this direcotry.
pub(crate) fn default_submod_path(
&self,
id: symbol::Ident,
relative: Option<symbol::Ident>,
dir_path: &Path,
) -> Result<rustc_expand::module::ModulePathSuccess, rustc_expand::module::ModError<'_>> {
rustc_expand::module::default_submod_path(&self.parse_sess, id, relative, dir_path)
) -> Result<ModulePathSuccess, ModError<'_>> {
rustc_expand::module::default_submod_path(&self.parse_sess, id, relative, dir_path).or_else(
|e| {
// If resloving a module relative to {dir_path}/{symbol} fails because a file
// could not be found, then try to resolve the module relative to {dir_path}.
if matches!(e, ModError::FileNotFound(..)) && relative.is_some() {
rustc_expand::module::default_submod_path(&self.parse_sess, id, None, dir_path)
} else {
Err(e)
}
},
)
}

pub(crate) fn is_file_parsed(&self, path: &Path) -> bool {
Expand Down
16 changes: 16 additions & 0 deletions src/test/mod_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,19 @@ fn fmt_out_of_line_test_modules() {
],
)
}

#[test]
fn fallback_and_try_to_resolve_external_submod_relative_to_current_dir_path() {
// See also https://github.com/rust-lang/rustfmt/issues/5198
verify_mod_resolution(
"tests/mod-resolver/issue-5198/lib.rs",
&[
"tests/mod-resolver/issue-5198/a.rs",
"tests/mod-resolver/issue-5198/lib/b.rs",
"tests/mod-resolver/issue-5198/lib/c/mod.rs",
"tests/mod-resolver/issue-5198/lib/c/e.rs",
"tests/mod-resolver/issue-5198/lib/c/d/f.rs",
"tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs",
],
)
}
1 change: 1 addition & 0 deletions tests/mod-resolver/issue-5198/a.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main( ) { println!("Hello World!") }
3 changes: 3 additions & 0 deletions tests/mod-resolver/issue-5198/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
mod a;
mod b;
mod c;
1 change: 1 addition & 0 deletions tests/mod-resolver/issue-5198/lib/b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main( ) { println!("Hello World!") }
3 changes: 3 additions & 0 deletions tests/mod-resolver/issue-5198/lib/c/d.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
mod e;
mod f;
mod g;
16 changes: 16 additions & 0 deletions tests/mod-resolver/issue-5198/lib/c/d/explanation.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
This file is contained in the './lib/c/d/' directory.

The directory name './lib/c/d/' conflicts with the './lib/c/d.rs' file name.

'./lib/c/d.rs' defines 3 external modules:

* mod e;
* mod f;
* mod g;

Module resolution will fail if we look for './lib/c/d/e.rs' or './lib/c/d/e/mod.rs',
so we should fall back to looking for './lib/c/e.rs', which correctly finds the modlue, that
rustfmt should format.

'./lib/c/d/f.rs' and './lib/c/d/g/mod.rs' exist at the default submodule paths so we should be able
to resolve these modules with no problems.
1 change: 1 addition & 0 deletions tests/mod-resolver/issue-5198/lib/c/d/f.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main( ) { println!("Hello World!") }
1 change: 1 addition & 0 deletions tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main( ) { println!("Hello World!") }
1 change: 1 addition & 0 deletions tests/mod-resolver/issue-5198/lib/c/e.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main( ) { println!("Hello World!") }
3 changes: 3 additions & 0 deletions tests/mod-resolver/issue-5198/lib/c/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
mod d;

fn main( ) { println!("Hello World!") }
10 changes: 10 additions & 0 deletions tests/mod-resolver/issue-5198/lib/explanation.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This file is contained in the './lib' directory.

The directory name './lib' conflicts with the './lib.rs' file name.

Module resolution will fail if we look for './lib/a.rs' or './lib/a/mod.rs',
so we should fall back to looking for './a.rs', which correctly finds the modlue that
rustfmt should format.

'./lib/b.rs' and './lib/c/mod.rs' exist at the default submodule paths so we should be able
to resolve these modules with no problems.

0 comments on commit 296d51a

Please sign in to comment.