rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

join_absolute_paths is not always right on Windows when dealing with just drive letter paths ("C:")

Open smalis-msft opened this issue 1 year ago • 1 comments

Summary

When dealing with a Path that consists solely of a drive letter and a colon (i.e. "C:"), joining it with a separator actually results in a meaningfully different path (i.e. "C:\"). Yes there actually are things that treat these two paths differently.

Lint Name

join_absolute_paths

Reproducer

I tried this code:

fn main() {
    let p = Path::new("C:");
    println!("{:?}, {:?}", p, terminate_drive_letter_paths(&p));
}

fn terminate_drive_letter_paths(path: &Path) -> PathBuf {
    if path.as_os_str().len() == 2 && path.to_str().map_or(false, |s| s.ends_with(':')) {
        path.join("\\")
    } else {
        path.to_owned()
    }
}

This emits the suggestion:

warning: argument to `Path::join` starts with a path separator
  --> src\main.rs:13:19
   |
13 |         path.join("\\")
   |                   ^^^^
   |
   = note: joining a path starting with separator will replace the path instead
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths
   = note: `#[warn(clippy::join_absolute_paths)]` on by default
help: if this is unintentional, try removing the starting separator
   |
13 |         path.join("\")
   |                   ~~~
help: if this is intentional, try using `Path::new` instead
   |
13 |         PathBuf::from("\\")
   |


However running the code shows that the suggestion and explanation are not entirely accurate. That path is not really "replaced", but rather gets a \ appended as we'd expect:

"C:", "C:\\"

Version

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-pc-windows-msvc
release: 1.76.0
LLVM version: 17.0.6

Additional Labels

No response

smalis-msft avatar Feb 08 '24 16:02 smalis-msft

This will probably just be a documentation change and a note to the lint output. The existence of drive relative paths is a quirk of windows that people may be unaware of.

It might be worth disabling the lint in cfg(windows) code.

Jarcho avatar Feb 14 '24 08:02 Jarcho