Skip to content

Commit 737e6f7

Browse files
ytmimicalebcartwright
authored andcommitted
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``.
1 parent 4a053f2 commit 737e6f7

File tree

10 files changed

+82
-1
lines changed

10 files changed

+82
-1
lines changed

Diff for: src/modules.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
458458
self.directory.path.push(path.as_str());
459459
self.directory.ownership = DirectoryOwnership::Owned { relative: None };
460460
} else {
461+
let id = id.as_str();
461462
// We have to push on the current module name in the case of relative
462463
// paths in order to ensure that any additional module paths from inline
463464
// `mod x { ... }` come after the relative extension.
@@ -468,9 +469,15 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
468469
if let Some(ident) = relative.take() {
469470
// remove the relative offset
470471
self.directory.path.push(ident.as_str());
472+
473+
// In the case where there is an x.rs and an ./x directory we want
474+
// to prevent adding x twice. For example, ./x/x
475+
if self.directory.path.exists() && !self.directory.path.join(id).exists() {
476+
return;
477+
}
471478
}
472479
}
473-
self.directory.path.push(id.as_str());
480+
self.directory.path.push(id);
474481
}
475482
}
476483

Diff for: src/test/mod_resolver.rs

+14
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,17 @@ fn skip_out_of_line_nested_inline_within_out_of_line() {
5050
&["tests/mod-resolver/skip-files-issue-5065/one.rs"],
5151
);
5252
}
53+
54+
#[test]
55+
fn fmt_out_of_line_test_modules() {
56+
// See also https://github.com/rust-lang/rustfmt/issues/5119
57+
verify_mod_resolution(
58+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs",
59+
&[
60+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs",
61+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs",
62+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs",
63+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs",
64+
],
65+
)
66+
}

Diff for: tests/cargo-fmt/main.rs

+24
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use std::env;
44
use std::process::Command;
5+
use std::path::Path;
56

67
/// Run the cargo-fmt executable and return its output.
78
fn cargo_fmt(args: &[&str]) -> (String, String) {
@@ -71,3 +72,26 @@ fn rustfmt_help() {
7172
assert_that!(&["--", "-h"], contains("Format Rust code"));
7273
assert_that!(&["--", "--help=config"], contains("Configuration Options:"));
7374
}
75+
76+
#[test]
77+
fn cargo_fmt_out_of_line_test_modules() {
78+
// See also https://github.com/rust-lang/rustfmt/issues/5119
79+
let expected_modified_files = [
80+
"tests/mod-resolver/test-submodule-issue-5119/src/lib.rs",
81+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1.rs",
82+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub1.rs",
83+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub2.rs",
84+
"tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs",
85+
];
86+
let args = [
87+
"-v",
88+
"--check",
89+
"--manifest-path",
90+
"tests/mod-resolver/test-submodule-issue-5119/Cargo.toml",
91+
];
92+
let (stdout, _) = cargo_fmt(&args);
93+
for file in expected_modified_files {
94+
let path = Path::new(file).canonicalize().unwrap();
95+
assert!(stdout.contains(&format!("Diff in {}", path.display())))
96+
}
97+
}
+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "rustfmt-test-submodule-issue"
3+
version = "0.1.0"
4+
edition = "2018"
5+
6+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
7+
8+
[dependencies]
+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pub fn foo() -> i32 {
2+
3
3+
}
4+
5+
pub fn bar() -> i32 {
6+
4
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
mod test1 {
2+
#[cfg(unix)]
3+
mod sub1;
4+
#[cfg(not(unix))]
5+
mod sub2;
6+
7+
mod sub3;
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
use rustfmt_test_submodule_issue::foo;
2+
3+
#[test]
4+
fn test_foo() {
5+
assert_eq!(3, foo());
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
use rustfmt_test_submodule_issue::bar;
2+
3+
#[test]
4+
fn test_bar() {
5+
assert_eq!(4, bar());
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
mod sub4;

Diff for: tests/mod-resolver/test-submodule-issue-5119/tests/test1/sub3/sub4.rs

Whitespace-only changes.

0 commit comments

Comments
 (0)