Skip to content

go_site_file leaves the ./ prefix on "./../x", contradicting its documented contract #138

Open

Description

@soarkit
  • Repository: pingdotgg/ts-rust
  • Target commit: 392e92eb06dc8bc136e0f309a0fb1fee04073254
  • File: crates/ts_goport/src/gopanic.rs:496

Summary

go_site_file (crates/ts_goport/src/gopanic.rs:498-516) walks the path segments to shorten a panic site: it pops a parent segment only when the previous segment is not .. The documented contract says ./../x is ../x, but the guard treats the leading . as a normal directory, so nothing is popped and the unnormalized ./../x is returned. Panic/backtrace formatting uses this helper, so every site built from such a path keeps the spurious ./ prefix; the crate-root case (.../goport_core/../../src/core.rs) still normalizes because the preceding segment is a real name.

Affected code

crates/ts_goport/src/gopanic.rs:

  488      payload
  489  }
  490  
  491  /// The file of a port site (`Location::file`) as panic text prints it:
  492  /// without its `<dir>/..` pairs. The roots of the `parts/` crates are
  493  /// `../../src/<crate>_root.rs`, so their sites are
  494  /// `crates/ts_goport/parts/goport_core/../../src/...`. This gives
  495  /// `crates/ts_goport/src/...`, the text from before the crate split. A `.`
  496  /// segment does not pair with a `..` (`./../x` is `../x`, not `x`).
  497  #[must_use]
  498  pub fn go_site_file(file: &str) -> std::borrow::Cow<'_, str> {
  499      if !file.contains("..") {
  500          return std::borrow::Cow::Borrowed(file);
  501      }
  502      fn name(piece: &str) -> &str {
  503          piece.trim_end_matches(['/', '\\'])
  504      }
  505      // Each piece is a path segment with the separator after it.
  506      let mut pieces: Vec<&str> = Vec::new();
  507      for piece in file.split_inclusive(['/', '\\']) {
  508          let parent = pieces.last().map(|last| name(last));

Expected behavior

go_site_file normalizes a . segment before .. as documented, so ./../x becomes ../x.

Actual behavior

The doc comment at gopanic.rs:496 states ./../x is ../x, but the condition at line 509 only avoids popping the . parent and the concat at line 515 returns ./../x.

How to reproduce

Call go_site_file("./../x") from a small Rust test (compiling gopanic.rs via #[path]); the returned string is "./../x" instead of "../x".

Root cause

go_site_file (crates/ts_goport/src/gopanic.rs:498-516) walks the path segments to shorten a panic site: it pops a parent segment only when the previous segment is not .. The documented contract says ./../x is ../x, but the guard treats the leading . as a normal directory, so nothing is popped and the unnormalized ./../x is returned. Panic/backtrace formatting uses this helper, so every site built from such a path keeps the spurious ./ prefix; the crate-root case (.../goport_core/../../src/core.rs) still normalizes because the preceding segment is a real name.

Suggested fix

Normalize a leading ./ before the parent-pop loop:

if rest.starts_with("./") {
    rest = &rest[2..];
}

Keep the existing ../ normalization for crate-root paths, and add a table test covering ./../x, ./a/../b.ts and ../../src/core.rs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions