diff --git a/CHANGELOG.md b/CHANGELOG.md index dafa3f3a1393..6f1317263a8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4747,6 +4747,7 @@ Released 2018-09-13 [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next [`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero +[`join_absolute_path`]: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_path [`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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a3a6f1746bcc..257f8bf0bda1 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -350,6 +350,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::ITER_OVEREAGER_CLONED_INFO, crate::methods::ITER_SKIP_NEXT_INFO, crate::methods::ITER_WITH_DRAIN_INFO, + crate::methods::JOIN_ABSOLUTE_PATH_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, crate::methods::MANUAL_NEXT_BACK_INFO, diff --git a/clippy_lints/src/methods/join_absolute_path.rs b/clippy_lints/src/methods/join_absolute_path.rs new file mode 100644 index 000000000000..4788540a9e65 --- /dev/null +++ b/clippy_lints/src/methods/join_absolute_path.rs @@ -0,0 +1,30 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, 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, Span}; + +use super::JOIN_ABSOLUTE_PATH; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, span: Span) { + let ty = cx.typeck_results().expr_ty(expr); + if_chain!( + if is_type_diagnostic_item(cx, ty, Path); + let applicability = Applicability::MachineApplicable; + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.as_str().starts_with('/') || symbol.as_str().starts_with('\\'); + then { + span_lint_and_sugg( + cx, + JOIN_ABSOLUTE_PATH, + span.with_hi(expr.span.hi()), + r#"argument in join called on path contains a starting '/'"#, + "try removing first '/' or '\\'", + "join(\"your/path/here\")".to_owned(), + applicability + ); + } + ); +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index cb86917464b4..8caca4d4f1de 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -45,6 +45,7 @@ mod iter_overeager_cloned; mod iter_skip_next; mod iter_with_drain; mod iterator_step_by_zero; +mod join_absolute_path; mod manual_next_back; mod manual_ok_or; mod manual_saturating_arithmetic; @@ -3194,6 +3195,41 @@ declare_clippy_lint! { "calling `drain` in order to `clear` a container" } +declare_clippy_lint! { + /// ### What it does + /// Checks for initial `'/'` in an argument to `.join()` on a `Path`. + /// + /// ### Why is this bad? + /// `.join()` comments starting with a separator, can replace the entire path. + /// If this is intentional, prefer creating a new `Path` instead. + /// + /// See [`Path::join()`](https://doc.rust-lang.org/std/path/struct.Path.html#method.join) + /// + /// ### Example + /// ```rust + /// let path = std::path::Path::new("/bin"); + /// let res = path.join("/sh"); + /// assert_eq!(res, std::path::PathBuf::from("/sh")); + /// ``` + /// + /// Use instead; + /// ```rust + /// let path = std::path::Path::new("/bin"); + /// + /// // If this was unintentional, remove the leading separator + /// let extend = path.join("sh"); + /// assert_eq!(extend, std::path::PathBuf::from("/bin/sh")); + /// + /// // If this was intentional, create a new path instead + /// let new = std::path::Path::new("/sh"); + /// assert_eq!(new, std::path::PathBuf::from("/sh")); + /// ``` + #[clippy::version = "1.70.0"] + pub JOIN_ABSOLUTE_PATH, + pedantic, + "arg to .join called on a Path contains '/' at the start" +} + declare_clippy_lint! { /// ### What it does /// Checks for `.rev().next()` on a `DoubleEndedIterator` @@ -3345,6 +3381,7 @@ impl_lint_pass!(Methods => [ NEEDLESS_COLLECT, SUSPICIOUS_COMMAND_ARG_SPACE, CLEAR_WITH_DRAIN, + JOIN_ABSOLUTE_PATH, MANUAL_NEXT_BACK, ]); @@ -3654,6 +3691,7 @@ impl Methods { if let Some(("collect", _, _, span, _)) = method_call(recv) { unnecessary_join::check(cx, expr, recv, join_arg, span); } + else {join_absolute_path::check(cx, expr, join_arg, span);} }, ("last", []) | ("skip", [_]) => { if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { diff --git a/tests/ui/join_absolute_path.rs b/tests/ui/join_absolute_path.rs new file mode 100644 index 000000000000..457a5528b41c --- /dev/null +++ b/tests/ui/join_absolute_path.rs @@ -0,0 +1,26 @@ +// run-rustfix +#![allow(unused)] +#![warn(clippy::join_absolute_path)] +use std::path::Path; + +fn main() { + // should be linted + let path = Path::new("/bin"); + path.join("/sh"); + println!("{}", path.display()); + + //should be linted + let path = Path::new("C:\\Users"); + path.join("\\user"); + println!("{}", path.display()); + + // should not be linted + let path: &[&str] = &["/bin"]; + path.join("/sh"); + println!("{:?}", path); + + //should not be linted + let path = Path::new("/bin"); + path.join("sh"); + println!("{}", path.display()); +}