[`pathbuf_init_then_push`]: Checks for calls to `push` immediately after creating a new `PathBuf`

Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>

3 files changed, 196 insertions, 0 deletions

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index eabc67601a2..69f9eb6842b 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -598,6 +598,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::partialeq_to_none::PARTIALEQ_TO_NONE_INFO,
     crate::pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE_INFO,
     crate::pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF_INFO,
+    crate::pathbuf_init_then_push::PATHBUF_INIT_THEN_PUSH_INFO,
     crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO,
     crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
     crate::precedence::PRECEDENCE_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 25cd7610400..5b40e362bca 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -287,6 +287,7 @@ mod partial_pub_fields;
 mod partialeq_ne_impl;
 mod partialeq_to_none;
 mod pass_by_ref_or_value;
+mod pathbuf_init_then_push;
 mod pattern_type_mismatch;
 mod permissions_set_readonly_false;
 mod precedence;
@@ -887,6 +888,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     });
     store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(conf)));
     store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
+    store.register_late_pass(|_| Box::<pathbuf_init_then_push::PathbufThenPush<'_>>::default());
     store.register_late_pass(|_| Box::new(iter_over_hash_type::IterOverHashType));
     store.register_late_pass(|_| Box::new(impl_hash_with_borrow_str_and_bytes::ImplHashWithBorrowStrBytes));
     store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity));
diff --git a/clippy_lints/src/pathbuf_init_then_push.rs b/clippy_lints/src/pathbuf_init_then_push.rs
new file mode 100644
index 00000000000..0008f154ae3
--- /dev/null
+++ b/clippy_lints/src/pathbuf_init_then_push.rs
@@ -0,0 +1,193 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::path_to_local_id;
+use clippy_utils::source::{snippet, snippet_opt};
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_ast::{LitKind, StrStyle};
+use rustc_errors::Applicability;
+use rustc_hir::def::Res;
+use rustc_hir::{BindingMode, Block, Expr, ExprKind, HirId, LetStmt, PatKind, QPath, Stmt, StmtKind, TyKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::impl_lint_pass;
+use rustc_span::{sym, Span, Symbol};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for calls to `push` immediately after creating a new `PathBuf`.
+    ///
+    /// ### Why is this bad?
+    /// Multiple `.join()` calls are usually easier to read than multiple `.push`
+    /// calls across multiple statements. It might also be possible to use
+    /// `PathBuf::from` instead.
+    ///
+    /// ### Known problems
+    /// `.join()` introduces an implicit `clone()`. `PathBuf::from` can alternativly be
+    /// used when the `PathBuf` is newly constructed. This will avoild the implicit clone.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::path::PathBuf;
+    /// let mut path_buf = PathBuf::new();
+    /// path_buf.push("foo");
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::path::PathBuf;
+    /// let path_buf = PathBuf::from("foo");
+    /// // or
+    /// let path_buf = PathBuf::new().join("foo");
+    /// ```
+    #[clippy::version = "1.81.0"]
+    pub PATHBUF_INIT_THEN_PUSH,
+    restriction,
+    "`push` immediately after `PathBuf` creation"
+}
+
+impl_lint_pass!(PathbufThenPush<'_> => [PATHBUF_INIT_THEN_PUSH]);
+
+#[derive(Default)]
+pub struct PathbufThenPush<'tcx> {
+    searcher: Option<PathbufPushSearcher<'tcx>>,
+}
+
+struct PathbufPushSearcher<'tcx> {
+    local_id: HirId,
+    lhs_is_let: bool,
+    let_ty_span: Option<Span>,
+    init_val: Expr<'tcx>,
+    arg: Option<Expr<'tcx>>,
+    name: Symbol,
+    err_span: Span,
+}
+
+impl<'tcx> PathbufPushSearcher<'tcx> {
+    /// Try to generate a suggestion with `PathBuf::from`.
+    /// Returns `None` if the suggestion would be invalid.
+    fn gen_pathbuf_from(&self, cx: &LateContext<'_>) -> Option<String> {
+        if let ExprKind::Call(iter_expr, []) = &self.init_val.kind
+            && let ExprKind::Path(QPath::TypeRelative(ty, segment)) = &iter_expr.kind
+            && let TyKind::Path(ty_path) = &ty.kind
+            && let QPath::Resolved(None, path) = ty_path
+            && let Res::Def(_, def_id) = &path.res
+            && cx.tcx.is_diagnostic_item(sym::PathBuf, *def_id)
+            && segment.ident.name == sym::new
+            && let Some(arg) = self.arg
+            && let ExprKind::Lit(x) = arg.kind
+            && let LitKind::Str(_, StrStyle::Cooked) = x.node
+            && let Some(s) = snippet_opt(cx, arg.span)
+        {
+            Some(format!(" = PathBuf::from({s});"))
+        } else {
+            None
+        }
+    }
+
+    fn gen_pathbuf_join(&self, cx: &LateContext<'_>) -> Option<String> {
+        let arg = self.arg?;
+        let arg_str = snippet_opt(cx, arg.span)?;
+        let init_val = snippet_opt(cx, self.init_val.span)?;
+        Some(format!(" = {init_val}.join({arg_str});"))
+    }
+
+    fn display_err(&self, cx: &LateContext<'_>) {
+        if clippy_utils::attrs::span_contains_cfg(cx, self.err_span) {
+            return;
+        }
+        let mut sugg = if self.lhs_is_let {
+            String::from("let mut ")
+        } else {
+            String::new()
+        };
+        sugg.push_str(self.name.as_str());
+        if let Some(span) = self.let_ty_span {
+            sugg.push_str(": ");
+            sugg.push_str(&snippet(cx, span, "_"));
+        }
+        match self.gen_pathbuf_from(cx) {
+            Some(value) => {
+                sugg.push_str(&value);
+            },
+            None => {
+                if let Some(value) = self.gen_pathbuf_join(cx) {
+                    sugg.push_str(&value);
+                } else {
+                    return;
+                }
+            },
+        }
+
+        span_lint_and_sugg(
+            cx,
+            PATHBUF_INIT_THEN_PUSH,
+            self.err_span,
+            "calls to `push` immediately after creation",
+            "consider using the `.join()`",
+            sugg,
+            Applicability::HasPlaceholders,
+        );
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for PathbufThenPush<'tcx> {
+    fn check_block(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
+        self.searcher = None;
+    }
+
+    fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx LetStmt<'tcx>) {
+        if let Some(init_expr) = local.init
+            && let PatKind::Binding(BindingMode::MUT, id, name, None) = local.pat.kind
+            && !in_external_macro(cx.sess(), local.span)
+            && let ty = cx.typeck_results().pat_ty(local.pat)
+            && is_type_diagnostic_item(cx, ty, sym::PathBuf)
+        {
+            self.searcher = Some(PathbufPushSearcher {
+                local_id: id,
+                lhs_is_let: true,
+                name: name.name,
+                let_ty_span: local.ty.map(|ty| ty.span),
+                err_span: local.span,
+                init_val: *init_expr,
+                arg: None,
+            });
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if let ExprKind::Assign(left, right, _) = expr.kind
+            && let ExprKind::Path(QPath::Resolved(None, path)) = left.kind
+            && let [name] = &path.segments
+            && let Res::Local(id) = path.res
+            && !in_external_macro(cx.sess(), expr.span)
+            && let ty = cx.typeck_results().expr_ty(left)
+            && is_type_diagnostic_item(cx, ty, sym::PathBuf)
+        {
+            self.searcher = Some(PathbufPushSearcher {
+                local_id: id,
+                lhs_is_let: false,
+                let_ty_span: None,
+                name: name.ident.name,
+                err_span: expr.span,
+                init_val: *right,
+                arg: None,
+            });
+        }
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
+        if let Some(mut searcher) = self.searcher.take() {
+            if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind
+                && let ExprKind::MethodCall(name, self_arg, [arg_expr], _) = expr.kind
+                && path_to_local_id(self_arg, searcher.local_id)
+                && name.ident.as_str() == "push"
+            {
+                searcher.err_span = searcher.err_span.to(stmt.span);
+                searcher.arg = Some(*arg_expr);
+                searcher.display_err(cx);
+            }
+        }
+    }
+
+    fn check_block_post(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
+        self.searcher = None;
+    }
+}