Normalize .. and . in diagnostic file paths

Author: arferreira

compiler/rustc_span/src/lib.rs

@@ -21,6 +21,7 @@
 #![feature(core_io_borrowed_buf)]
 #![feature(map_try_insert)]
 #![feature(negative_impls)]
+#![feature(normalize_lexically)]
 #![feature(read_buf)]
 #![feature(rustc_attrs)]
 // tidy-alphabetical-end
@@ -496,6 +497,15 @@ impl RealFileName {
                 .file_name()
                 .map_or_else(|| "".into(), |f| f.to_string_lossy()),
             FileNameDisplayPreference::Scope(scope) => self.path(scope).to_string_lossy(),
+            FileNameDisplayPreference::Diagnostics(scope) => {
+                let path = self.path(scope);
+                match path.normalize_lexically() {
+                    Ok(normalized) => {
+                        Cow::Owned(normalized.into_os_string().to_string_lossy().into_owned())
+                    }
+                    Err(_) => path.to_string_lossy(),
+                }
+            }
         }
     }
 }
@@ -533,15 +543,23 @@ enum FileNameDisplayPreference {
     Local,
     Short,
     Scope(RemapPathScopeComponents),
+    Diagnostics(RemapPathScopeComponents),
+}
+
+impl<'a> FileNameDisplay<'a> {
+    pub fn to_string_lossy(&self) -> Cow<'a, str> {
+        match self.inner {
+            FileName::Real(inner) => inner.to_string_lossy(self.display_pref),
+            _ => Cow::from(self.to_string()),
+        }
+    }
 }
 
 impl fmt::Display for FileNameDisplay<'_> {
     fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         use FileName::*;
         match *self.inner {
-            Real(ref name) => {
-                write!(fmt, "{}", name.to_string_lossy(self.display_pref))
-            }
+            Real(ref name) => write!(fmt, "{}", name.to_string_lossy(self.display_pref)),
             CfgSpec(_) => write!(fmt, "<cfgspec>"),
             MacroExpansion(_) => write!(fmt, "<macro expansion>"),
             Anon(_) => write!(fmt, "<anon>"),
@@ -554,15 +572,6 @@ impl fmt::Display for FileNameDisplay<'_> {
     }
 }
 
-impl<'a> FileNameDisplay<'a> {
-    pub fn to_string_lossy(&self) -> Cow<'a, str> {
-        match self.inner {
-            FileName::Real(inner) => inner.to_string_lossy(self.display_pref),
-            _ => Cow::from(self.to_string()),
-        }
-    }
-}
-
 impl FileName {
     pub fn is_real(&self) -> bool {
         use FileName::*;
@@ -609,6 +618,12 @@ impl FileName {
         FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Scope(scope) }
     }
 
+    /// Like `display`, but with `.` and `..` resolved lexically. See #51349.
+    #[inline]
+    pub fn display_normalized(&self, scope: RemapPathScopeComponents) -> FileNameDisplay<'_> {
+        FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Diagnostics(scope) }
+    }
+
     pub fn macro_expansion_source_code(src: &str) -> FileName {
         let mut hasher = StableHasher::new();
         src.hash(&mut hasher);

compiler/rustc_span/src/source_map.rs

@@ -487,7 +487,7 @@ impl SourceMap {
     }
 
     pub fn filename_for_diagnostics<'a>(&self, filename: &'a FileName) -> FileNameDisplay<'a> {
-        filename.display(RemapPathScopeComponents::DIAGNOSTICS)
+        filename.display_normalized(RemapPathScopeComponents::DIAGNOSTICS)
     }
 
     pub fn is_multiline(&self, sp: Span) -> bool {

compiler/rustc_span/src/source_map/tests.rs

@@ -797,3 +797,14 @@ fn read_binary_file_handles_lying_stat() {
     let bin = RealFileLoader.read_binary_file(kernel_max).unwrap();
     assert_eq!(&real[..], &bin[..]);
 }
+
+#[test]
+fn filename_for_diagnostics_resolves_parent_dir() {
+    let sm = SourceMap::new(FilePathMapping::empty());
+
+    let with_parent = filename(&sm, "tests/sub/../helper.rs");
+    assert_eq!(sm.filename_for_diagnostics(&with_parent).to_string(), path_str("tests/helper.rs"));
+
+    let clean = filename(&sm, "tests/clean.rs");
+    assert_eq!(sm.filename_for_diagnostics(&clean).to_string(), path_str("tests/clean.rs"));
+}

src/tools/compiletest/src/runtest.rs

@@ -2485,12 +2485,22 @@ impl<'test> TestCx<'test> {
         let parent_dir = self.testpaths.file.parent().unwrap();
         normalize_path(parent_dir, "$DIR");
 
+        // After #51349, rustc normalizes `tests/x/y/../aux/foo.rs` to
+        // `tests/x/aux/foo.rs`. Replace the grandparent with `$DIR/..` so
+        // stderrs keep the pre-normalization form.
+        if let Some(grandparent_dir) = parent_dir.parent() {
+            normalize_path(grandparent_dir, "$DIR/..");
+        }
+
         if self.props.remap_src_base {
             let mut remapped_parent_dir = Utf8PathBuf::from(FAKE_SRC_BASE);
             if self.testpaths.relative_dir != Utf8Path::new("") {
                 remapped_parent_dir.push(&self.testpaths.relative_dir);
             }
             normalize_path(&remapped_parent_dir, "$DIR");
+            if let Some(remapped_grandparent) = remapped_parent_dir.parent() {
+                normalize_path(remapped_grandparent, "$DIR/..");
+            }
         }
 
         let base_dir = Utf8Path::new("/rustc/FAKE_PREFIX");

tests/ui/README.md

@@ -440,6 +440,10 @@ Everything to do with `--diagnostic-width`.
 
 Exercises `#[diagnostic::*]` namespaced attributes. See [RFC 3368 Diagnostic attribute namespace](https://github.com/rust-lang/rfcs/blob/master/text/3368-diagnostic-attribute-namespace.md).
 
+## `tests/ui/diagnostics/`
+
+Tests for diagnostic output quality, such as path normalization in error messages.
+
 ## `tests/ui/did_you_mean/`
 
 Tests for miscellaneous suggestions.

tests/ui/const-generics/generic_arg_infer/issue-91614.stderr

@@ -5,7 +5,7 @@ LL |     let y = Mask::<_, _>::splat(false);
    |         ^   ------------ type must be known at this point
    |
 note: required by a const generic parameter in `Mask`
-  --> $SRC_DIR/core/src/../../portable-simd/crates/core_simd/src/masks.rs:LL:COL
+  --> $SRC_DIR/portable-simd/crates/core_simd/src/masks.rs:LL:COL
 help: consider giving `y` an explicit type, where the value of const parameter `N` is specified
    |
 LL |     let y: Mask<_, N> = Mask::<_, _>::splat(false);
@@ -18,7 +18,7 @@ LL |     let y = Mask::<_, _>::splat(false);
    |         ^   -------------------------- type must be known at this point
    |
 note: required by a const generic parameter in `Mask::<T, N>::splat`
-  --> $SRC_DIR/core/src/../../portable-simd/crates/core_simd/src/masks.rs:LL:COL
+  --> $SRC_DIR/portable-simd/crates/core_simd/src/masks.rs:LL:COL
 help: consider giving `y` an explicit type, where the value of const parameter `N` is specified
    |
 LL |     let y: Mask<_, N> = Mask::<_, _>::splat(false);

tests/ui/diagnostics/auxiliary/helper.rs

@@ -0,0 +1,3 @@
+pub fn foo() -> u32 {
+    "not a u32"
+}

tests/ui/diagnostics/auxiliary/sub/mod.rs

@@ -0,0 +1,2 @@
+#[path = "../helper.rs"]
+mod helper;

tests/ui/diagnostics/normalize-path.rs

@@ -0,0 +1,9 @@
+// Verify that diagnostic file paths are lexically normalized.
+// Without the fix for #51349, the error location would show
+// `auxiliary/sub/../helper.rs` instead of `auxiliary/helper.rs`.
+#[path = "auxiliary/sub/mod.rs"]
+mod sub;
+
+fn main() {}
+
+//~? ERROR mismatched types

tests/ui/diagnostics/normalize-path.stderr

@@ -0,0 +1,11 @@
+error[E0308]: mismatched types
+  --> $DIR/auxiliary/helper.rs:2:5
+   |
+LL | pub fn foo() -> u32 {
+   |                 --- expected `u32` because of return type
+LL |     "not a u32"
+   |     ^^^^^^^^^^^ expected `u32`, found `&str`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0308`.