Skip to content

Commit 12048e4

Browse files
ytmimicalebcartwright
authored andcommitted
fallback to dir_path when relative external mod resolution fails
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 because 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 demonstrated 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.
1 parent 89ca3f3 commit 12048e4

File tree

12 files changed

+82
-2
lines changed

12 files changed

+82
-2
lines changed

Diff for: src/parse/session.rs

+20-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_span::{
1212

1313
use crate::config::file_lines::LineRange;
1414
use crate::ignore_path::IgnorePathSet;
15+
use crate::parse::parser::{ModError, ModulePathSuccess};
1516
use crate::source_map::LineRangeUtils;
1617
use crate::utils::starts_with_newline;
1718
use crate::visitor::SnippetProvider;
@@ -145,13 +146,30 @@ impl ParseSess {
145146
})
146147
}
147148

149+
/// Determine the submodule path for the given module identifier.
150+
///
151+
/// * `id` - The name of the module
152+
/// * `relative` - If Some(symbol), the symbol name is a directory relative to the dir_path.
153+
/// If relative is Some, resolve the submodle at {dir_path}/{symbol}/{id}.rs
154+
/// or {dir_path}/{symbol}/{id}/mod.rs. if None, resolve the module at {dir_path}/{id}.rs.
155+
/// * `dir_path` - Module resolution will occur relative to this direcotry.
148156
pub(crate) fn default_submod_path(
149157
&self,
150158
id: symbol::Ident,
151159
relative: Option<symbol::Ident>,
152160
dir_path: &Path,
153-
) -> Result<rustc_expand::module::ModulePathSuccess, rustc_expand::module::ModError<'_>> {
154-
rustc_expand::module::default_submod_path(&self.parse_sess, id, relative, dir_path)
161+
) -> Result<ModulePathSuccess, ModError<'_>> {
162+
rustc_expand::module::default_submod_path(&self.parse_sess, id, relative, dir_path).or_else(
163+
|e| {
164+
// If resloving a module relative to {dir_path}/{symbol} fails because a file
165+
// could not be found, then try to resolve the module relative to {dir_path}.
166+
if matches!(e, ModError::FileNotFound(..)) && relative.is_some() {
167+
rustc_expand::module::default_submod_path(&self.parse_sess, id, None, dir_path)
168+
} else {
169+
Err(e)
170+
}
171+
},
172+
)
155173
}
156174

157175
pub(crate) fn is_file_parsed(&self, path: &Path) -> bool {

Diff for: src/test/mod_resolver.rs

+16
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,19 @@ fn fmt_out_of_line_test_modules() {
6464
],
6565
)
6666
}
67+
68+
#[test]
69+
fn fallback_and_try_to_resolve_external_submod_relative_to_current_dir_path() {
70+
// See also https://github.com/rust-lang/rustfmt/issues/5198
71+
verify_mod_resolution(
72+
"tests/mod-resolver/issue-5198/lib.rs",
73+
&[
74+
"tests/mod-resolver/issue-5198/a.rs",
75+
"tests/mod-resolver/issue-5198/lib/b.rs",
76+
"tests/mod-resolver/issue-5198/lib/c/mod.rs",
77+
"tests/mod-resolver/issue-5198/lib/c/e.rs",
78+
"tests/mod-resolver/issue-5198/lib/c/d/f.rs",
79+
"tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs",
80+
],
81+
)
82+
}

Diff for: tests/mod-resolver/issue-5198/a.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod a;
2+
mod b;
3+
mod c;

Diff for: tests/mod-resolver/issue-5198/lib/b.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib/c/d.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod e;
2+
mod f;
3+
mod g;
+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
This file is contained in the './lib/c/d/' directory.
2+
3+
The directory name './lib/c/d/' conflicts with the './lib/c/d.rs' file name.
4+
5+
'./lib/c/d.rs' defines 3 external modules:
6+
7+
* mod e;
8+
* mod f;
9+
* mod g;
10+
11+
Module resolution will fail if we look for './lib/c/d/e.rs' or './lib/c/d/e/mod.rs',
12+
so we should fall back to looking for './lib/c/e.rs', which correctly finds the modlue, that
13+
rustfmt should format.
14+
15+
'./lib/c/d/f.rs' and './lib/c/d/g/mod.rs' exist at the default submodule paths so we should be able
16+
to resolve these modules with no problems.

Diff for: tests/mod-resolver/issue-5198/lib/c/d/f.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib/c/e.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib/c/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod d;
2+
3+
fn main( ) { println!("Hello World!") }

Diff for: tests/mod-resolver/issue-5198/lib/explanation.txt

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
This file is contained in the './lib' directory.
2+
3+
The directory name './lib' conflicts with the './lib.rs' file name.
4+
5+
'lib.rs' defines 3 external modules:
6+
7+
* mod a;
8+
* mod b;
9+
* mod c;
10+
11+
Module resolution will fail if we look for './lib/a.rs' or './lib/a/mod.rs',
12+
so we should fall back to looking for './a.rs', which correctly finds the modlue that
13+
rustfmt should format.
14+
15+
'./lib/b.rs' and './lib/c/mod.rs' exist at the default submodule paths so we should be able
16+
to resolve these modules with no problems.

0 commit comments

Comments
 (0)