Auto merge of #11453 - xFrednet:11208-path-join-correct, r=blyxyas

New lint `clippy::join_absolute_paths`

Hey `@ofeeg,` this PR is a copy of all the changes you did in 47171d3 from PR #11208. I wasn't sure how to fix the git history. Hope you're okay with me figuring this out in this separate PR

---

changelog: New lint [`join_absolute_paths`]
[#11453](#11453)

Closes: #10655

r? `@Centri3` Since you also gave feedback on the other PR. I hope that I copied everything correctly, but a third pair of eyes would be appreciated :D

Author: bors

CHANGELOG.md

@@ -5190,6 +5190,7 @@ Released 2018-09-13
 [`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
 [`iter_without_into_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_without_into_iter
 [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
+[`join_absolute_paths`]: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths
 [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
 [`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays
 [`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups

clippy_lints/src/declared_lints.rs

@@ -377,6 +377,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::ITER_SKIP_NEXT_INFO,
     crate::methods::ITER_SKIP_ZERO_INFO,
     crate::methods::ITER_WITH_DRAIN_INFO,
+    crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
     crate::methods::MANUAL_NEXT_BACK_INFO,

clippy_lints/src/methods/join_absolute_paths.rs

@@ -0,0 +1,52 @@
+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;
+use rustc_span::Span;
+
+use super::JOIN_ABSOLUTE_PATHS;
+
+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
+        && let sym_str = symbol.as_str()
+        && sym_str.starts_with(['/', '\\'])
+    {
+        span_lint_and_then(
+            cx,
+            JOIN_ABSOLUTE_PATHS,
+            join_arg.span,
+            "argument to `Path::join` starts with a path separator",
+            |diag| {
+                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 using `Path::new` instead",
+                        format!("PathBuf::from({arg_str})"),
+                        Applicability::Unspecified,
+                    );
+            },
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -49,6 +49,7 @@ mod iter_skip_next;
 mod iter_skip_zero;
 mod iter_with_drain;
 mod iterator_step_by_zero;
+mod join_absolute_paths;
 mod manual_next_back;
 mod manual_ok_or;
 mod manual_saturating_arithmetic;
@@ -3684,6 +3685,46 @@ declare_clippy_lint! {
     "calling the `try_from` and `try_into` trait methods when `From`/`Into` is implemented"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for calls to `Path::join` that start with a path separator (`\\` or `/`).
+    ///
+    /// ### Why is this bad?
+    /// 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)
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::path::{Path, PathBuf};
+    /// let path = Path::new("/bin");
+    /// let joined_path = path.join("/sh");
+    /// assert_eq!(joined_path, PathBuf::from("/sh"));
+    /// ```
+    ///
+    /// Use instead;
+    /// ```rust
+    /// # use std::path::{Path, PathBuf};
+    /// let path = Path::new("/bin");
+    ///
+    /// // If this was unintentional, remove the leading separator
+    /// let joined_path = path.join("sh");
+    /// assert_eq!(joined_path, PathBuf::from("/bin/sh"));
+    ///
+    /// // If this was intentional, create a new path instead
+    /// let new = Path::new("/sh");
+    /// assert_eq!(new, PathBuf::from("/sh"));
+    /// ```
+    #[clippy::version = "1.76.0"]
+    pub JOIN_ABSOLUTE_PATHS,
+    suspicious,
+    "calls to `Path::join` which will overwrite the original path"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3833,6 +3874,7 @@ impl_lint_pass!(Methods => [
     REDUNDANT_AS_STR,
     WAKER_CLONE_WAKE,
     UNNECESSARY_FALLIBLE_CONVERSIONS,
+    JOIN_ABSOLUTE_PATHS,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4235,6 +4277,8 @@ impl Methods {
                 ("join", [join_arg]) => {
                     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, expr.span);
                     }
                 },
                 ("last", []) => {

tests/ui/join_absolute_paths.rs

@@ -0,0 +1,30 @@
+//@no-rustfix
+
+#![allow(clippy::needless_raw_string_hashes)]
+#![warn(clippy::join_absolute_paths)]
+
+use std::path::{Path, PathBuf};
+
+fn main() {
+    let path = Path::new("/bin");
+    path.join("/sh");
+    //~^ ERROR: argument to `Path::join` starts with a path separator
+
+    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
+
+    let path: &[&str] = &["/bin"];
+    path.join("/sh");
+
+    let path = Path::new("/bin");
+    path.join("sh");
+}

tests/ui/join_absolute_paths.stderr

@@ -0,0 +1,68 @@
+error: argument to `Path::join` starts with a path separator
+  --> $DIR/join_absolute_paths.rs:10:15
+   |
+LL |     path.join("/sh");
+   |               ^^^^^
+   |
+   = note: joining a path starting with separator will replace the path instead
+   = note: `-D clippy::join-absolute-paths` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::join_absolute_paths)]`
+help: if this is unintentional, try removing the starting separator
+   |
+LL |     path.join("sh");
+   |               ~~~~
+help: if this is intentional, try using `Path::new` instead
+   |
+LL |     PathBuf::from("/sh");
+   |     ~~~~~~~~~~~~~~~~~~~~
+
+error: argument to `Path::join` starts with a path separator
+  --> $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
+   |
+LL |     path.join("\user");
+   |               ~~~~~~~
+help: if this is intentional, try using `Path::new` 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 using `Path::new` 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 using `Path::new` instead
+   |
+LL |     PathBuf::from(r#"/sh"#);
+   |     ~~~~~~~~~~~~~~~~~~~~~~~
+
+error: aborting due to 4 previous errors
+