From 07aa4ce604d0d3d52e22e7c29a95d904a5a5da03 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 19 Dec 2021 21:27:25 -0500 Subject: [PATCH] Improve out of line module resolution Fixes 5119 When the directory structure was laid out as follows: ``` dir |---mod_a | |---sub_mod_1.rs | |---sub_mod_2.rs |---mod_a.rs ``` And ``mod_a.rs`` contains the following content: ```rust mod mod_a { mod sub_mod_1; mod sub_mod_2; } ``` rustfmt previously tried to find ``sub_mod_1.rs`` and ``sub_mod_2.rs`` in ``./mod_a/mod_a/``. This directory does not exist and this caused rustfmt to fail with the error message: Error writing files: failed to resolve mod Now, both ``sub_mod_1.rs`` and ``sub_mod_2.rs`` are resolved correctly and found at ``mod_a/sub_mod_1.rs`` and ``mod_a/sub_mod_2.rs``. --- src/modules.rs | 9 ++++++- src/test/mod_resolver.rs | 14 +++++++++++ tests/cargo-fmt/main.rs | 24 +++++++++++++++++++ .../test-submodule-issue-5119/Cargo.toml | 8 +++++++ .../test-submodule-issue-5119/src/lib.rs | 7 ++++++ .../test-submodule-issue-5119/tests/test1.rs | 8 +++++++ .../tests/test1/sub1.rs | 6 +++++ .../tests/test1/sub2.rs | 6 +++++ .../tests/test1/sub3/mod.rs | 1 + .../tests/test1/sub3/sub4.rs | 0 10 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/mod-resolver/test-submodule-issue-5119/Cargo.toml create mode 100644 tests/mod-resolver/test-submodule-issue-5119/src/lib.rs create mode 100644 tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs create mode 100644 tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs create mode 100644 tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs create mode 100644 tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/mod.rs create mode 100644 tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs diff --git a/src/modules.rs b/src/modules.rs index 9c964b274e0..70b937b0283 100644 --- a/src/modules.rs +++ b/src/modules.rs @@ -458,6 +458,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { self.directory.path.push(path.as_str()); self.directory.ownership = DirectoryOwnership::Owned { relative: None }; } else { + let id = id.as_str(); // We have to push on the current module name in the case of relative // paths in order to ensure that any additional module paths from inline // `mod x { ... }` come after the relative extension. @@ -468,9 +469,15 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { if let Some(ident) = relative.take() { // remove the relative offset self.directory.path.push(ident.as_str()); + + // In the case where there is an x.rs and an ./x directory we want + // to prevent adding x twice. For example, ./x/x + if self.directory.path.exists() && !self.directory.path.join(id).exists() { + return; + } } } - self.directory.path.push(id.as_str()); + self.directory.path.push(id); } } diff --git a/src/test/mod_resolver.rs b/src/test/mod_resolver.rs index ec9ed0f0b8d..fcff6d14e6f 100644 --- a/src/test/mod_resolver.rs +++ b/src/test/mod_resolver.rs @@ -50,3 +50,17 @@ fn skip_out_of_line_nested_inline_within_out_of_line() { &["tests/mod-resolver/skip-files-issue-5065/one.rs"], ); } + +#[test] +fn fmt_out_of_line_test_modules() { + // See also https://github.com/rust-lang/rustfmt/issues/5119 + verify_mod_resolution( + "tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs", + &[ + "tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs", + ], + ) +} diff --git a/tests/cargo-fmt/main.rs b/tests/cargo-fmt/main.rs index 5493b09e4aa..bf81f253f69 100644 --- a/tests/cargo-fmt/main.rs +++ b/tests/cargo-fmt/main.rs @@ -2,6 +2,7 @@ use std::env; use std::process::Command; +use std::path::Path; /// Run the cargo-fmt executable and return its output. fn cargo_fmt(args: &[&str]) -> (String, String) { @@ -71,3 +72,26 @@ fn rustfmt_help() { assert_that!(&["--", "-h"], contains("Format Rust code")); assert_that!(&["--", "--help=config"], contains("Configuration Options:")); } + +#[test] +fn cargo_fmt_out_of_line_test_modules() { + // See also https://github.com/rust-lang/rustfmt/issues/5119 + let expected_modified_files = [ + "tests/mod-resolver/test-submodule-issue-5119/src/lib.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs", + "tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs", + ]; + let args = [ + "-v", + "--check", + "--manifest-path", + "tests/mod-resolver/test-submodule-issue-5119/Cargo.toml", + ]; + let (stdout, _) = cargo_fmt(&args); + for file in expected_modified_files { + let path = Path::new(file).canonicalize().unwrap(); + assert!(stdout.contains(&format!("Diff in {}", path.display()))) + } +} diff --git a/tests/mod-resolver/test-submodule-issue-5119/Cargo.toml b/tests/mod-resolver/test-submodule-issue-5119/Cargo.toml new file mode 100644 index 00000000000..0993f127959 --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "rustfmt-test-submodule-issue" +version = "0.1.0" +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/tests/mod-resolver/test-submodule-issue-5119/src/lib.rs b/tests/mod-resolver/test-submodule-issue-5119/src/lib.rs new file mode 100644 index 00000000000..3f7ddba8a28 --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/src/lib.rs @@ -0,0 +1,7 @@ +pub fn foo() -> i32 { +3 +} + +pub fn bar() -> i32 { +4 +} diff --git a/tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs b/tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs new file mode 100644 index 00000000000..da4e86169ad --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs @@ -0,0 +1,8 @@ +mod test1 { +#[cfg(unix)] +mod sub1; +#[cfg(not(unix))] +mod sub2; + +mod sub3; +} diff --git a/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs new file mode 100644 index 00000000000..b760ba23cd2 --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs @@ -0,0 +1,6 @@ +use rustfmt_test_submodule_issue::foo; + +#[test] +fn test_foo() { +assert_eq!(3, foo()); +} diff --git a/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs new file mode 100644 index 00000000000..4fd8286eac4 --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs @@ -0,0 +1,6 @@ +use rustfmt_test_submodule_issue::bar; + +#[test] +fn test_bar() { +assert_eq!(4, bar()); +} diff --git a/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/mod.rs b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/mod.rs new file mode 100644 index 00000000000..e029785bc24 --- /dev/null +++ b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/mod.rs @@ -0,0 +1 @@ +mod sub4; diff --git a/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs b/tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs new file mode 100644 index 00000000000..e69de29bb2d