Normalize .. and . in diagnostic file paths

Author: arferreira

compiler/rustc_span/src/lib.rs

@@ -74,7 +74,7 @@ use std::fmt::Display;
 use std::hash::Hash;
 use std::io::{self, Read};
 use std::ops::{Add, Range, Sub};
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 use std::str::FromStr;
 use std::sync::Arc;
 use std::{fmt, iter};
@@ -495,11 +495,35 @@ impl RealFileName {
                 .name
                 .file_name()
                 .map_or_else(|| "".into(), |f| f.to_string_lossy()),
-            FileNameDisplayPreference::Scope(scope) => self.path(scope).to_string_lossy(),
+            FileNameDisplayPreference::Scope(scope)
+            | FileNameDisplayPreference::Diagnostics(scope) => self.path(scope).to_string_lossy(),
         }
     }
 }
 
+/// Lexically normalizes a path by resolving `.` and `..` components without
+/// touching the filesystem. Preserves whether the path is relative or absolute.
+///
+/// Used for diagnostic display only. `Path::canonicalize` is intentionally avoided
+/// because it turns relative paths into absolute ones (see #51349, #83345).
+pub(crate) fn normalize_path(path: &Path) -> PathBuf {
+    let mut out = PathBuf::new();
+    for component in path.components() {
+        match component {
+            Component::CurDir => {}
+            Component::ParentDir => match out.components().next_back() {
+                Some(Component::Normal(_)) => {
+                    out.pop();
+                }
+                Some(Component::RootDir) => {}
+                _ => out.push(component),
+            },
+            _ => out.push(component),
+        }
+    }
+    if out.as_os_str().is_empty() { PathBuf::from(".") } else { out }
+}
+
 /// Differentiates between real files and common virtual files.
 #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, Decodable, Encodable)]
 pub enum FileName {
@@ -533,14 +557,36 @@ enum FileNameDisplayPreference {
     Local,
     Short,
     Scope(RemapPathScopeComponents),
+    Diagnostics(RemapPathScopeComponents),
+}
+
+impl<'a> FileNameDisplay<'a> {
+    fn maybe_normalize<'s>(&self, s: Cow<'s, str>) -> Cow<'s, str> {
+        if !matches!(self.display_pref, FileNameDisplayPreference::Diagnostics(_)) {
+            return s;
+        }
+        let path = Path::new(s.as_ref());
+        if !path.components().any(|c| matches!(c, Component::ParentDir | Component::CurDir)) {
+            return s;
+        }
+        Cow::Owned(normalize_path(path).into_os_string().to_string_lossy().into_owned())
+    }
+
+    pub fn to_string_lossy(&self) -> Cow<'a, str> {
+        match self.inner {
+            FileName::Real(inner) => self.maybe_normalize(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))
+                let s = self.maybe_normalize(name.to_string_lossy(self.display_pref));
+                write!(fmt, "{s}")
             }
             CfgSpec(_) => write!(fmt, "<cfgspec>"),
             MacroExpansion(_) => write!(fmt, "<macro expansion>"),
@@ -554,15 +600,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 +646,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,64 @@ fn read_binary_file_handles_lying_stat() {
     let bin = RealFileLoader.read_binary_file(kernel_max).unwrap();
     assert_eq!(&real[..], &bin[..]);
 }
+
+#[test]
+fn normalize_path_resolves_parent_dir() {
+    assert_eq!(normalize_path(&path("a/b/../c.rs")), path("a/c.rs"));
+}
+
+#[test]
+fn normalize_path_removes_cur_dir() {
+    assert_eq!(normalize_path(&path("a/b/./c.rs")), path("a/b/c.rs"));
+}
+
+#[test]
+fn normalize_path_mixed_components() {
+    assert_eq!(normalize_path(&path("a/b/./../c.rs")), path("a/c.rs"));
+}
+
+#[test]
+fn normalize_path_preserves_leading_parent_dir() {
+    assert_eq!(normalize_path(&path("../../lib/foo.rs")), path("../../lib/foo.rs"));
+}
+
+#[test]
+fn normalize_path_absolute() {
+    assert_eq!(normalize_path(&path("/abs/a/../b.rs")), path("/abs/b.rs"));
+}
+
+#[test]
+fn normalize_path_no_op() {
+    assert_eq!(normalize_path(&path("a.rs")), path("a.rs"));
+}
+
+#[test]
+fn normalize_path_dot_becomes_dot() {
+    assert_eq!(normalize_path(Path::new(".")), Path::new("."));
+}
+
+#[test]
+fn normalize_path_reduces_to_dot() {
+    assert_eq!(normalize_path(&path("a/..")), Path::new("."));
+}
+
+#[test]
+fn normalize_path_parent_beyond_normal() {
+    assert_eq!(normalize_path(&path("a/../..")), path(".."));
+}
+
+#[test]
+fn normalize_path_complex_backtrack() {
+    assert_eq!(normalize_path(&path("a/../b/../../c")), path("../c"));
+}
+
+#[test]
+fn normalize_path_parent_at_root_clamped() {
+    // /.. is semantically / (can't go above root).
+    assert_eq!(normalize_path(&path("/..")), path("/"));
+}
+
+#[test]
+fn normalize_path_parent_at_root_with_tail() {
+    assert_eq!(normalize_path(&path("/../a/b")), path("/a/b"));
+}

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/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`.

tests/ui/imports/ambiguous-2.stderr

@@ -6,14 +6,14 @@ LL |     ambiguous_1::id();
    |
    = note: ambiguous because of multiple glob imports of a name in the same module
 note: `id` could refer to the function defined here
-  --> $DIR/auxiliary/../ambiguous-1.rs:13:13
+  --> $DIR/ambiguous-1.rs:13:13
    |
 LL |     pub use self::evp::*;
    |             ^^^^^^^^^
    = help: consider updating this dependency to resolve this error
    = help: if updating the dependency does not resolve the problem report the problem to the author of the relevant crate
 note: `id` could also refer to the function defined here
-  --> $DIR/auxiliary/../ambiguous-1.rs:15:13
+  --> $DIR/ambiguous-1.rs:15:13
    |
 LL |     pub use self::handwritten::*;
    |             ^^^^^^^^^^^^^^^^^
@@ -32,14 +32,14 @@ LL |     ambiguous_1::id();
    |
    = note: ambiguous because of multiple glob imports of a name in the same module
 note: `id` could refer to the function defined here
-  --> $DIR/auxiliary/../ambiguous-1.rs:13:13
+  --> $DIR/ambiguous-1.rs:13:13
    |
 LL |     pub use self::evp::*;
    |             ^^^^^^^^^
    = help: consider updating this dependency to resolve this error
    = help: if updating the dependency does not resolve the problem report the problem to the author of the relevant crate
 note: `id` could also refer to the function defined here
-  --> $DIR/auxiliary/../ambiguous-1.rs:15:13
+  --> $DIR/ambiguous-1.rs:15:13
    |
 LL |     pub use self::handwritten::*;
    |             ^^^^^^^^^^^^^^^^^