Move PathBufPushOverwrite into Methods lint group

Author: Jarcho

clippy_lints/src/lib.register_lints.rs

@@ -344,6 +344,7 @@ store.register_lints(&[
     methods::OPTION_MAP_OR_NONE,
     methods::OR_FUN_CALL,
     methods::OR_THEN_UNWRAP,
+    methods::PATH_BUF_PUSH_OVERWRITE,
     methods::RESULT_MAP_OR_INTO_OPTION,
     methods::SEARCH_IS_SOME,
     methods::SHOULD_IMPLEMENT_TRAIT,
@@ -460,7 +461,6 @@ store.register_lints(&[
     partialeq_to_none::PARTIALEQ_TO_NONE,
     pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE,
     pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF,
-    path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,
     pattern_type_mismatch::PATTERN_TYPE_MISMATCH,
     precedence::PRECEDENCE,
     ptr::CMP_NULL,

clippy_lints/src/lib.register_nursery.rs

@@ -17,6 +17,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(methods::ITER_ON_EMPTY_COLLECTIONS),
     LintId::of(methods::ITER_ON_SINGLE_ITEMS),
     LintId::of(methods::ITER_WITH_DRAIN),
+    LintId::of(methods::PATH_BUF_PUSH_OVERWRITE),
     LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
     LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
     LintId::of(mutex_atomic::MUTEX_ATOMIC),
@@ -25,7 +26,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
     LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
-    LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
     LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),
     LintId::of(regex::TRIVIAL_REGEX),
     LintId::of(strings::STRING_LIT_AS_BYTES),

clippy_lints/src/lib.rs

@@ -324,7 +324,6 @@ mod panic_unimplemented;
 mod partialeq_ne_impl;
 mod partialeq_to_none;
 mod pass_by_ref_or_value;
-mod path_buf_push_overwrite;
 mod pattern_type_mismatch;
 mod precedence;
 mod ptr;
@@ -727,7 +726,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(assertions_on_constants::AssertionsOnConstants));
     store.register_late_pass(|| Box::new(assertions_on_result_states::AssertionsOnResultStates));
     store.register_late_pass(|| Box::new(transmuting_null::TransmutingNull));
-    store.register_late_pass(|| Box::new(path_buf_push_overwrite::PathBufPushOverwrite));
     store.register_late_pass(|| Box::new(inherent_to_string::InherentToString));
     let max_trait_bounds = conf.max_trait_bounds;
     store.register_late_pass(move || Box::new(trait_bounds::TraitBounds::new(max_trait_bounds)));

clippy_lints/src/methods/mod.rs

@@ -63,6 +63,7 @@ mod option_map_or_none;
 mod option_map_unwrap_or;
 mod or_fun_call;
 mod or_then_unwrap;
+mod path_buf_push_overwrite;
 mod search_is_some;
 mod single_char_add_str;
 mod single_char_insert_string;
@@ -2701,6 +2702,38 @@ declare_clippy_lint! {
     "nonsensical combination of options for opening a file"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///* Checks for [push](https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.push)
+    /// calls on `PathBuf` that can cause overwrites.
+    ///
+    /// ### Why is this bad?
+    /// Calling `push` with a root path at the start can overwrite the
+    /// previous defined path.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::path::PathBuf;
+    ///
+    /// let mut x = PathBuf::from("/foo");
+    /// x.push("/bar");
+    /// assert_eq!(x, PathBuf::from("/bar"));
+    /// ```
+    /// Could be written:
+    ///
+    /// ```rust
+    /// use std::path::PathBuf;
+    ///
+    /// let mut x = PathBuf::from("/foo");
+    /// x.push("bar");
+    /// assert_eq!(x, PathBuf::from("/foo/bar"));
+    /// ```
+    #[clippy::version = "1.36.0"]
+    pub PATH_BUF_PUSH_OVERWRITE,
+    nursery,
+    "calling `push` with file system root on `PathBuf` can overwrite it"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2814,6 +2847,7 @@ impl_lint_pass!(Methods => [
     MAP_ERR_IGNORE,
     MUT_MUTEX_LOCK,
     NONSENSICAL_OPEN_OPTIONS,
+    PATH_BUF_PUSH_OVERWRITE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3199,6 +3233,9 @@ impl Methods {
                         unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
                     }
                 },
+                ("push", [arg]) => {
+                    path_buf_push_overwrite::check(cx, expr, arg);
+                },
                 ("splitn" | "rsplitn", [count_arg, pat_arg]) => {
                     if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
                         suspicious_splitn::check(cx, name, expr, recv, count);

clippy_lints/src/methods/path_buf_push_overwrite.rs

@@ -0,0 +1,37 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::ty::is_type_diagnostic_item;
+use if_chain::if_chain;
+use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+use std::path::{Component, Path};
+
+use super::PATH_BUF_PUSH_OVERWRITE;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'tcx Expr<'_>) {
+    if_chain! {
+        if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+        if let Some(impl_id) = cx.tcx.impl_of_method(method_id);
+        if is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id), sym::PathBuf);
+        if let ExprKind::Lit(ref lit) = arg.kind;
+        if let LitKind::Str(ref path_lit, _) = lit.node;
+        if let pushed_path = Path::new(path_lit.as_str());
+        if let Some(pushed_path_lit) = pushed_path.to_str();
+        if pushed_path.has_root();
+        if let Some(root) = pushed_path.components().next();
+        if root == Component::RootDir;
+        then {
+            span_lint_and_sugg(
+                cx,
+                PATH_BUF_PUSH_OVERWRITE,
+                lit.span,
+                "calling `push` with '/' or '\\' (file system root) will overwrite the previous path definition",
+                "try",
+                format!("\"{}\"", pushed_path_lit.trim_start_matches(|c| c == '/' || c == '\\')),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}