about summary refs log tree commit diff
diff options
context:
space:
mode:
authorYacin Tmimi <yacintmimi@gmail.com>2022-01-10 20:45:27 -0500
committerCaleb Cartwright <calebcartwright@users.noreply.github.com>2022-03-08 21:51:23 -0600
commit18c0369688d5854ea67dbe29cfac3cc8380cb20e (patch)
tree34d15a4d4d4b252e4c627d1a84241bf292ade34d
parent58de4142c525cc50824dab1cf199dc967c88356f (diff)
downloadrust-18c0369688d5854ea67dbe29cfac3cc8380cb20e.tar.gz
rust-18c0369688d5854ea67dbe29cfac3cc8380cb20e.zip
Improve mod resolution error for mods with multiple candidate files
Fixes 5167

When ``a.rs`` and ``a/mod.rs`` are both present we would emit an error
message telling the user that the module couldn't be found. However,
this behavior is misleading because we're dealing with an ambiguous
module path, not a "file not found" error.

Is the file ``a.rs`` or is it ``a/mod.rs``? Rustfmt can't decide, and
the user needs to resolve this ambiguity themselves.

Now, the error message displayed to the user is in line with what they
would see if they went to compile their code with these conflicting
module names.
-rw-r--r--src/modules.rs38
-rw-r--r--src/parse/session.rs3
-rw-r--r--tests/mod-resolver/issue-5167/src/a.rs0
-rw-r--r--tests/mod-resolver/issue-5167/src/a/mod.rs0
-rw-r--r--tests/mod-resolver/issue-5167/src/lib.rs1
-rw-r--r--tests/mod-resolver/module-not-found/bad_path_attribute/lib.rs3
-rw-r--r--tests/mod-resolver/module-not-found/relative_module/a.rs2
-rw-r--r--tests/mod-resolver/module-not-found/relative_module/lib.rs1
-rw-r--r--tests/mod-resolver/module-not-found/sibling_module/lib.rs2
-rw-r--r--tests/rustfmt/main.rs44
10 files changed, 88 insertions, 6 deletions
diff --git a/src/modules.rs b/src/modules.rs
index 70b937b0283..49c99403974 100644
--- a/src/modules.rs
+++ b/src/modules.rs
@@ -81,6 +81,7 @@ pub struct ModuleResolutionError {
     pub(crate) kind: ModuleResolutionErrorKind,
 }
 
+/// Defines variants similar to those of [rustc_expand::module::ModError]
 #[derive(Debug, Error)]
 pub(crate) enum ModuleResolutionErrorKind {
     /// Find a file that cannot be parsed.
@@ -89,6 +90,12 @@ pub(crate) enum ModuleResolutionErrorKind {
     /// File cannot be found.
     #[error("{file} does not exist")]
     NotFound { file: PathBuf },
+    /// File a.rs and a/mod.rs both exist
+    #[error("file for module found at both {default_path:?} and {secondary_path:?}")]
+    MultipleCandidates {
+        default_path: PathBuf,
+        secondary_path: PathBuf,
+    },
 }
 
 #[derive(Clone)]
@@ -444,12 +451,31 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
                 }
                 Ok(Some(SubModKind::MultiExternal(mods_outside_ast)))
             }
-            Err(_) => Err(ModuleResolutionError {
-                module: mod_name.to_string(),
-                kind: ModuleResolutionErrorKind::NotFound {
-                    file: self.directory.path.clone(),
-                },
-            }),
+            Err(e) => match e {
+                ModError::FileNotFound(_, default_path, _secondary_path) => {
+                    Err(ModuleResolutionError {
+                        module: mod_name.to_string(),
+                        kind: ModuleResolutionErrorKind::NotFound { file: default_path },
+                    })
+                }
+                ModError::MultipleCandidates(_, default_path, secondary_path) => {
+                    Err(ModuleResolutionError {
+                        module: mod_name.to_string(),
+                        kind: ModuleResolutionErrorKind::MultipleCandidates {
+                            default_path,
+                            secondary_path,
+                        },
+                    })
+                }
+                ModError::ParserError(_)
+                | ModError::CircularInclusion(_)
+                | ModError::ModInBlock(_) => Err(ModuleResolutionError {
+                    module: mod_name.to_string(),
+                    kind: ModuleResolutionErrorKind::ParseError {
+                        file: self.directory.path.clone(),
+                    },
+                }),
+            },
         }
     }
 
diff --git a/src/parse/session.rs b/src/parse/session.rs
index fb9182152d1..87ab8fbf20a 100644
--- a/src/parse/session.rs
+++ b/src/parse/session.rs
@@ -163,8 +163,11 @@ impl ParseSess {
             |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 we still can't find the module after searching for it in {dir_path},
+                // surface the original error.
                 if matches!(e, ModError::FileNotFound(..)) && relative.is_some() {
                     rustc_expand::module::default_submod_path(&self.parse_sess, id, None, dir_path)
+                        .map_err(|_| e)
                 } else {
                     Err(e)
                 }
diff --git a/tests/mod-resolver/issue-5167/src/a.rs b/tests/mod-resolver/issue-5167/src/a.rs
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/tests/mod-resolver/issue-5167/src/a.rs
diff --git a/tests/mod-resolver/issue-5167/src/a/mod.rs b/tests/mod-resolver/issue-5167/src/a/mod.rs
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/tests/mod-resolver/issue-5167/src/a/mod.rs
diff --git a/tests/mod-resolver/issue-5167/src/lib.rs b/tests/mod-resolver/issue-5167/src/lib.rs
new file mode 100644
index 00000000000..f21af614da0
--- /dev/null
+++ b/tests/mod-resolver/issue-5167/src/lib.rs
@@ -0,0 +1 @@
+mod a;
diff --git a/tests/mod-resolver/module-not-found/bad_path_attribute/lib.rs b/tests/mod-resolver/module-not-found/bad_path_attribute/lib.rs
new file mode 100644
index 00000000000..2a63c961be8
--- /dev/null
+++ b/tests/mod-resolver/module-not-found/bad_path_attribute/lib.rs
@@ -0,0 +1,3 @@
+// module resolution fails because the path does not exist.
+#[path = "path/to/does_not_exist.rs"]
+mod a;
diff --git a/tests/mod-resolver/module-not-found/relative_module/a.rs b/tests/mod-resolver/module-not-found/relative_module/a.rs
new file mode 100644
index 00000000000..4a1eac8965d
--- /dev/null
+++ b/tests/mod-resolver/module-not-found/relative_module/a.rs
@@ -0,0 +1,2 @@
+// module resolution fails because `./a/b.rs` does not exist
+mod b;
diff --git a/tests/mod-resolver/module-not-found/relative_module/lib.rs b/tests/mod-resolver/module-not-found/relative_module/lib.rs
new file mode 100644
index 00000000000..f21af614da0
--- /dev/null
+++ b/tests/mod-resolver/module-not-found/relative_module/lib.rs
@@ -0,0 +1 @@
+mod a;
diff --git a/tests/mod-resolver/module-not-found/sibling_module/lib.rs b/tests/mod-resolver/module-not-found/sibling_module/lib.rs
new file mode 100644
index 00000000000..d9d9e1e3c90
--- /dev/null
+++ b/tests/mod-resolver/module-not-found/sibling_module/lib.rs
@@ -0,0 +1,2 @@
+// module resolution fails because `./a.rs` does not exist
+mod a;
diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs
index 2262ae3aaac..450051d2fec 100644
--- a/tests/rustfmt/main.rs
+++ b/tests/rustfmt/main.rs
@@ -113,3 +113,47 @@ fn rustfmt_usage_text() {
     let (stdout, _) = rustfmt(&args);
     assert!(stdout.contains("Format Rust code\n\nusage: rustfmt [options] <file>..."));
 }
+
+#[test]
+fn mod_resolution_error_multiple_candidate_files() {
+    // See also https://github.com/rust-lang/rustfmt/issues/5167
+    let default_path = Path::new("tests/mod-resolver/issue-5167/src/a.rs");
+    let secondary_path = Path::new("tests/mod-resolver/issue-5167/src/a/mod.rs");
+    let error_message = format!(
+        "file for module found at both {:?} and {:?}",
+        default_path.canonicalize().unwrap(),
+        secondary_path.canonicalize().unwrap(),
+    );
+
+    let args = ["tests/mod-resolver/issue-5167/src/lib.rs"];
+    let (_stdout, stderr) = rustfmt(&args);
+    assert!(stderr.contains(&error_message))
+}
+
+#[test]
+fn mod_resolution_error_sibling_module_not_found() {
+    let args = ["tests/mod-resolver/module-not-found/sibling_module/lib.rs"];
+    let (_stdout, stderr) = rustfmt(&args);
+    // Module resolution fails because we're unable to find `a.rs` in the same directory as lib.rs
+    assert!(stderr.contains("a.rs does not exist"))
+}
+
+#[test]
+fn mod_resolution_error_relative_module_not_found() {
+    let args = ["tests/mod-resolver/module-not-found/relative_module/lib.rs"];
+    let (_stdout, stderr) = rustfmt(&args);
+    // The file `./a.rs` and directory `./a` both exist.
+    // Module resolution fails becuase we're unable to find `./a/b.rs`
+    #[cfg(not(windows))]
+    assert!(stderr.contains("a/b.rs does not exist"));
+    #[cfg(windows)]
+    assert!(stderr.contains("a\\b.rs does not exist"));
+}
+
+#[test]
+fn mod_resolution_error_path_attribute_does_not_exist() {
+    let args = ["tests/mod-resolver/module-not-found/bad_path_attribute/lib.rs"];
+    let (_stdout, stderr) = rustfmt(&args);
+    // The path attribute points to a file that does not exist
+    assert!(stderr.contains("does_not_exist.rs does not exist"));
+}