From 296d51afa21fcb6fcab91f37b9afb43126a2e8bd Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 30 Jan 2022 13:46:53 -0500 Subject: [PATCH] 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 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. --- src/parse/session.rs | 22 +++++++++++++++++-- src/test/mod_resolver.rs | 16 ++++++++++++++ tests/mod-resolver/issue-5198/a.rs | 1 + tests/mod-resolver/issue-5198/lib.rs | 3 +++ tests/mod-resolver/issue-5198/lib/b.rs | 1 + tests/mod-resolver/issue-5198/lib/c/d.rs | 3 +++ .../issue-5198/lib/c/d/explanation.txt | 16 ++++++++++++++ tests/mod-resolver/issue-5198/lib/c/d/f.rs | 1 + .../mod-resolver/issue-5198/lib/c/d/g/mod.rs | 1 + tests/mod-resolver/issue-5198/lib/c/e.rs | 1 + tests/mod-resolver/issue-5198/lib/c/mod.rs | 3 +++ .../issue-5198/lib/explanation.txt | 10 +++++++++ 12 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 tests/mod-resolver/issue-5198/a.rs create mode 100644 tests/mod-resolver/issue-5198/lib.rs create mode 100644 tests/mod-resolver/issue-5198/lib/b.rs create mode 100644 tests/mod-resolver/issue-5198/lib/c/d.rs create mode 100644 tests/mod-resolver/issue-5198/lib/c/d/explanation.txt create mode 100644 tests/mod-resolver/issue-5198/lib/c/d/f.rs create mode 100644 tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs create mode 100644 tests/mod-resolver/issue-5198/lib/c/e.rs create mode 100644 tests/mod-resolver/issue-5198/lib/c/mod.rs create mode 100644 tests/mod-resolver/issue-5198/lib/explanation.txt diff --git a/src/parse/session.rs b/src/parse/session.rs index 624fed0d2de..fb9182152d1 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -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; @@ -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, dir_path: &Path, - ) -> Result> { - rustc_expand::module::default_submod_path(&self.parse_sess, id, relative, dir_path) + ) -> Result> { + 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 { diff --git a/src/test/mod_resolver.rs b/src/test/mod_resolver.rs index fcff6d14e6f..aacb2acc684 100644 --- a/src/test/mod_resolver.rs +++ b/src/test/mod_resolver.rs @@ -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", + ], + ) +} diff --git a/tests/mod-resolver/issue-5198/a.rs b/tests/mod-resolver/issue-5198/a.rs new file mode 100644 index 00000000000..cd686f56116 --- /dev/null +++ b/tests/mod-resolver/issue-5198/a.rs @@ -0,0 +1 @@ +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib.rs b/tests/mod-resolver/issue-5198/lib.rs new file mode 100644 index 00000000000..7f1c060deb6 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib.rs @@ -0,0 +1,3 @@ +mod a; +mod b; +mod c; \ No newline at end of file diff --git a/tests/mod-resolver/issue-5198/lib/b.rs b/tests/mod-resolver/issue-5198/lib/b.rs new file mode 100644 index 00000000000..cd686f56116 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/b.rs @@ -0,0 +1 @@ +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib/c/d.rs b/tests/mod-resolver/issue-5198/lib/c/d.rs new file mode 100644 index 00000000000..9b444ef39a3 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/d.rs @@ -0,0 +1,3 @@ +mod e; +mod f; +mod g; \ No newline at end of file diff --git a/tests/mod-resolver/issue-5198/lib/c/d/explanation.txt b/tests/mod-resolver/issue-5198/lib/c/d/explanation.txt new file mode 100644 index 00000000000..92c9e302143 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/d/explanation.txt @@ -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. \ No newline at end of file diff --git a/tests/mod-resolver/issue-5198/lib/c/d/f.rs b/tests/mod-resolver/issue-5198/lib/c/d/f.rs new file mode 100644 index 00000000000..cd686f56116 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/d/f.rs @@ -0,0 +1 @@ +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs b/tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs new file mode 100644 index 00000000000..cd686f56116 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/d/g/mod.rs @@ -0,0 +1 @@ +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib/c/e.rs b/tests/mod-resolver/issue-5198/lib/c/e.rs new file mode 100644 index 00000000000..cd686f56116 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/e.rs @@ -0,0 +1 @@ +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib/c/mod.rs b/tests/mod-resolver/issue-5198/lib/c/mod.rs new file mode 100644 index 00000000000..81904619650 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/c/mod.rs @@ -0,0 +1,3 @@ +mod d; + +fn main( ) { println!("Hello World!") } diff --git a/tests/mod-resolver/issue-5198/lib/explanation.txt b/tests/mod-resolver/issue-5198/lib/explanation.txt new file mode 100644 index 00000000000..9c851932c29 --- /dev/null +++ b/tests/mod-resolver/issue-5198/lib/explanation.txt @@ -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.