diff --git a/clippy_lints/src/methods/join_absolute_paths.rs b/clippy_lints/src/methods/join_absolute_paths.rs index d7acd386e747..2f2eec0d342e 100644 --- a/clippy_lints/src/methods/join_absolute_paths.rs +++ b/clippy_lints/src/methods/join_absolute_paths.rs @@ -1,18 +1,23 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::expr_or_init; +use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_type_diagnostic_item; use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_span::symbol::sym::Path; +use rustc_span::symbol::sym; +use rustc_span::Span; use super::JOIN_ABSOLUTE_PATHS; -pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>) { - let ty = cx.typeck_results().expr_ty(expr).peel_refs(); - if is_type_diagnostic_item(cx, ty, Path) - && let ExprKind::Lit(spanned) = &join_arg.kind +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, recv: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, expr_span: Span) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + if (is_type_diagnostic_item(cx, ty, sym::Path) || is_type_diagnostic_item(cx, ty, sym::PathBuf)) + && let ExprKind::Lit(spanned) = expr_or_init(cx, join_arg).kind && let LitKind::Str(symbol, _) = spanned.node - && (symbol.as_str().starts_with('/') || symbol.as_str().starts_with('\\')) + && let sym_str = symbol.as_str() + && (sym_str.starts_with('/') || sym_str.starts_with('\\')) { span_lint_and_then( cx, @@ -20,10 +25,27 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_a join_arg.span, "argument to `Path::join` starts with a path separator", |diag| { - diag - .note("joining a path starting with separator will replace the path instead") - .help("if this is unintentional, try removing the starting separator") - .help("if this is intentional, try creating a new Path instead"); + let arg_str = snippet_opt(cx, spanned.span).unwrap_or_else(|| "..".to_string()); + + let no_separator = if sym_str.starts_with('/') { + arg_str.replacen('/', "", 1) + } else { + arg_str.replacen('\\', "", 1) + }; + + diag.note("joining a path starting with separator will replace the path instead") + .span_suggestion( + spanned.span, + "if this is unintentional, try removing the starting separator", + no_separator, + Applicability::Unspecified + ) + .span_suggestion( + expr_span, + "if this is intentional, try creating a new Path instead", + format!("PathBuf::from({arg_str})"), + Applicability::Unspecified + ); }, ); } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7cff2ce3d6ba..961f5dd0fe98 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3633,11 +3633,15 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// Checks for calls to `Path::join` that start with a path separator, like `\\` or `/`.. + /// ### What it does + /// Checks for calls to `Path::join` that start with a path separator (`\\` or `/`). /// /// ### Why is this bad? - /// `.join()` arguments starting with a separator (`/` or `\\`) can replace the entire path. - /// If this is intentional, prefer using `Path::new()` instead. + /// If the argument to `Path::join` starts with a separator, it will overwrite + /// the original path. If this is intentional, prefer using `Path::new` instead. + /// + /// Note the behavior is platform dependent. A leading `\\` will be accepted + /// on unix systems as part of the file name /// /// See [`Path::join()`](https://doc.rust-lang.org/std/path/struct.Path.html#method.join) /// @@ -3665,7 +3669,7 @@ declare_clippy_lint! { #[clippy::version = "1.74.0"] pub JOIN_ABSOLUTE_PATHS, correctness, - "arg to .join called on a `Path` contains leading separator" + "calls to `Path::join` which will overwrite the original path" } pub struct Methods { @@ -4178,7 +4182,7 @@ impl Methods { if let Some(("collect", _, _, span, _)) = method_call(recv) { unnecessary_join::check(cx, expr, recv, join_arg, span); } else { - join_absolute_paths::check(cx, recv, join_arg); + join_absolute_paths::check(cx, recv, join_arg, expr.span); } }, ("last", []) => { diff --git a/tests/ui/join_absolute_paths.rs b/tests/ui/join_absolute_paths.rs index cef22a1bfa5b..efa77a0492e6 100644 --- a/tests/ui/join_absolute_paths.rs +++ b/tests/ui/join_absolute_paths.rs @@ -1,26 +1,30 @@ -#![allow(unused)] +//@no-rustfix + +#![allow(clippy::needless_raw_string_hashes)] #![warn(clippy::join_absolute_paths)] use std::path::{Path, PathBuf}; fn main() { - // should be linted let path = Path::new("/bin"); path.join("/sh"); + //~^ ERROR: argument to `Path::join` starts with a path separator - // should be linted let path = Path::new("C:\\Users"); path.join("\\user"); + //~^ ERROR: argument to `Path::join` starts with a path separator + + let path = PathBuf::from("/bin"); + path.join("/sh"); + //~^ ERROR: argument to `Path::join` starts with a path separator + + let path = PathBuf::from("/bin"); + path.join(r#"/sh"#); + //~^ ERROR: argument to `Path::join` starts with a path separator - // should not be linted let path: &[&str] = &["/bin"]; path.join("/sh"); - // should not be linted let path = Path::new("/bin"); path.join("sh"); - - // should be linted - let path = PathBuf::from("/bin"); - path.join("/sh"); } diff --git a/tests/ui/join_absolute_paths.stderr b/tests/ui/join_absolute_paths.stderr index 4dc803fc6213..f6de415b5796 100644 --- a/tests/ui/join_absolute_paths.stderr +++ b/tests/ui/join_absolute_paths.stderr @@ -1,23 +1,67 @@ error: argument to `Path::join` starts with a path separator - --> $DIR/join_absolute_paths.rs:9:15 + --> $DIR/join_absolute_paths.rs:10:15 | LL | path.join("/sh"); | ^^^^^ | = note: joining a path starting with separator will replace the path instead - = help: if this is unintentional, try removing the starting separator - = help: if this is intentional, try creating a new Path instead = note: `-D clippy::join-absolute-paths` implied by `-D warnings` +help: if this is unintentional, try removing the starting separator + | +LL | path.join("sh"); + | ~~~~ +help: if this is intentional, try creating a new Path instead + | +LL | PathBuf::from("/sh"); + | ~~~~~~~~~~~~~~~~~~~~ error: argument to `Path::join` starts with a path separator - --> $DIR/join_absolute_paths.rs:13:15 + --> $DIR/join_absolute_paths.rs:14:15 | LL | path.join("\\user"); | ^^^^^^^^ | = note: joining a path starting with separator will replace the path instead - = help: if this is unintentional, try removing the starting separator - = help: if this is intentional, try creating a new Path instead +help: if this is unintentional, try removing the starting separator + | +LL | path.join("\user"); + | ~~~~~~~ +help: if this is intentional, try creating a new Path instead + | +LL | PathBuf::from("\\user"); + | ~~~~~~~~~~~~~~~~~~~~~~~ + +error: argument to `Path::join` starts with a path separator + --> $DIR/join_absolute_paths.rs:18:15 + | +LL | path.join("/sh"); + | ^^^^^ + | + = note: joining a path starting with separator will replace the path instead +help: if this is unintentional, try removing the starting separator + | +LL | path.join("sh"); + | ~~~~ +help: if this is intentional, try creating a new Path instead + | +LL | PathBuf::from("/sh"); + | ~~~~~~~~~~~~~~~~~~~~ + +error: argument to `Path::join` starts with a path separator + --> $DIR/join_absolute_paths.rs:22:15 + | +LL | path.join(r#"/sh"#); + | ^^^^^^^^ + | + = note: joining a path starting with separator will replace the path instead +help: if this is unintentional, try removing the starting separator + | +LL | path.join(r#"sh"#); + | ~~~~~~~ +help: if this is intentional, try creating a new Path instead + | +LL | PathBuf::from(r#"/sh"#); + | ~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors