diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index cf58d1bc518..5750f914ac5 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index d6590192655..dc50816452f 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/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), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0d2cc07504a..44cfe0d00ec 100644 --- a/clippy_lints/src/lib.rs +++ b/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))); diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3532aa17afd..545cf7918af 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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, @@ -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); diff --git a/clippy_lints/src/methods/path_buf_push_overwrite.rs b/clippy_lints/src/methods/path_buf_push_overwrite.rs new file mode 100644 index 00000000000..0cc28c0dcb3 --- /dev/null +++ b/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, + ); + } + } +} diff --git a/clippy_lints/src/path_buf_push_overwrite.rs b/clippy_lints/src/path_buf_push_overwrite.rs deleted file mode 100644 index bc6a918f703..00000000000 --- a/clippy_lints/src/path_buf_push_overwrite.rs +++ /dev/null @@ -1,72 +0,0 @@ -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, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::symbol::sym; -use std::path::{Component, Path}; - -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" -} - -declare_lint_pass!(PathBufPushOverwrite => [PATH_BUF_PUSH_OVERWRITE]); - -impl<'tcx> LateLintPass<'tcx> for PathBufPushOverwrite { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if let ExprKind::MethodCall(path, [recv, get_index_arg], _) = expr.kind; - if path.ident.name == sym!(push); - if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv).peel_refs(), sym::PathBuf); - if let ExprKind::Lit(ref lit) = get_index_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, - ); - } - } - } -}