From 9a7bf41c8965015d0c679554d0a16ac7cfd5726f Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 17 Apr 2023 16:46:48 -0400 Subject: [PATCH 01/15] Added new lint for issue #10655 --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 25 +++++++++++++ .../src/methods/path_join_correction.rs | 35 +++++++++++++++++++ tests/ui/path_join_correction.rs | 15 ++++++++ tests/ui/path_join_correction.stderr | 10 ++++++ 6 files changed, 87 insertions(+) create mode 100644 clippy_lints/src/methods/path_join_correction.rs create mode 100644 tests/ui/path_join_correction.rs create mode 100644 tests/ui/path_join_correction.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 559b560dde4b..b868f1db606c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4814,6 +4814,7 @@ Released 2018-09-13 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite +[`path_join_correction`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_join_correction [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f24dab627809..c3e51a354aa5 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -376,6 +376,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::OR_FUN_CALL_INFO, crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, + crate::methods::PATH_JOIN_CORRECTION_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c213030f6a8f..4fdc0384f2d4 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,3 +1,4 @@ +mod path_join_correction; mod bind_instead_of_map; mod bytecount; mod bytes_count_to_len; @@ -3216,6 +3217,25 @@ declare_clippy_lint! { "calling `drain` in order to `clear` a container" } +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.70.0"] + pub PATH_JOIN_CORRECTION, + pedantic, + "default lint description" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3345,6 +3365,7 @@ impl_lint_pass!(Methods => [ NEEDLESS_COLLECT, SUSPICIOUS_COMMAND_ARG_SPACE, CLEAR_WITH_DRAIN, + PATH_JOIN_CORRECTION, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3655,7 +3676,11 @@ impl Methods { if let Some(("collect", _, _, span, _)) = method_call(recv) { unnecessary_join::check(cx, expr, recv, join_arg, span); } + else {path_join_correction::check(cx, expr, join_arg, span);} }, + /*("join", [join_arg]) => { + path_join_correction::check(cx, expr, join_arg, span); + },*/ ("last", []) | ("skip", [_]) => { if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { if let ("cloned", []) = (name2, args2) { diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs new file mode 100644 index 000000000000..aa2d2a644ff9 --- /dev/null +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -0,0 +1,35 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_lint::{LateContext}; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_span::Span; + +use super::PATH_JOIN_CORRECTION; + +// TODO: Adjust the parameters as necessary + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + join_arg: &'tcx Expr<'tcx>, + span: Span, +) { + let applicability = Applicability::MachineApplicable; + if_chain!( + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.as_str().starts_with('/'); + then { + span_lint_and_sugg( + cx, + PATH_JOIN_CORRECTION, + span.with_hi(expr.span.hi()), + r#"argument in join called on path contains a starting '/'"#, + "try removing first '/'", + "join(\"your/path/here\")".to_owned(), + applicability + ); + } + ); +} diff --git a/tests/ui/path_join_correction.rs b/tests/ui/path_join_correction.rs new file mode 100644 index 000000000000..aaa33b9fb4db --- /dev/null +++ b/tests/ui/path_join_correction.rs @@ -0,0 +1,15 @@ +#![allow(unused)] +#![warn(clippy::path_join_correction)] + +fn main() { + // should be linted + let path = std::path::Path::new("/bin"); + path.join("/sh"); + println!("{}", path.display()); + + //should not be linted + let path = std::path::Path::new("/bin"); + path.join("sh"); + println!("{}", path.display()); + +} diff --git a/tests/ui/path_join_correction.stderr b/tests/ui/path_join_correction.stderr new file mode 100644 index 000000000000..c73dff01e28a --- /dev/null +++ b/tests/ui/path_join_correction.stderr @@ -0,0 +1,10 @@ +error: argument in join called on path contains a starting '/' + --> $DIR/path_join_correction.rs:7:8 + | +LL | path.join("/sh"); + | ^^^^^^^^^^^ help: try removing first '/': `join("your/path/here")` + | + = note: `-D clippy::path-join-correction` implied by `-D warnings` + +error: aborting due to previous error + From 5de0ee9f643613b781a810e807aa4aa095699578 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 17 Apr 2023 16:54:52 -0400 Subject: [PATCH 02/15] This commit resolves issue #10655 --- clippy_lints/src/methods/path_join_correction.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index aa2d2a644ff9..6ed8c06ad7ae 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -7,7 +7,6 @@ use rustc_span::Span; use super::PATH_JOIN_CORRECTION; -// TODO: Adjust the parameters as necessary pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, From 327cc322a9d500ad28a3319638464c4af51f7934 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 17 Apr 2023 18:18:29 -0400 Subject: [PATCH 03/15] Fixed errors and lack of documentation in mod.rs. --- clippy_lints/src/methods/mod.rs | 15 +++---- .../src/methods/path_join_correction.rs | 44 ++++++++----------- tests/ui/path_join_correction.fixed | 15 +++++++ tests/ui/path_join_correction.rs | 18 ++++---- tests/ui/path_join_correction.stderr | 6 +-- 5 files changed, 53 insertions(+), 45 deletions(-) create mode 100644 tests/ui/path_join_correction.fixed diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4fdc0384f2d4..2904727fec76 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,4 +1,3 @@ -mod path_join_correction; mod bind_instead_of_map; mod bytecount; mod bytes_count_to_len; @@ -69,6 +68,7 @@ mod option_map_unwrap_or; mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; +mod path_join_correction; mod range_zip_with_len; mod repeat_once; mod search_is_some; @@ -3219,21 +3219,23 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// + /// Checks for initial '/' in an argument to .join on a path. /// ### Why is this bad? - /// + /// .join() calls on paths already attach the '/'. /// ### Example /// ```rust /// // example code where clippy issues a warning + /// let path = std::path::Path::new("/bin"); + /// path.join("/sh"); /// ``` /// Use instead: /// ```rust - /// // example code which does not raise clippy warning + /// // path.join("sh"); /// ``` #[clippy::version = "1.70.0"] pub PATH_JOIN_CORRECTION, pedantic, - "default lint description" + "arg to .join called on a Path contains '/' at the start" } pub struct Methods { @@ -3678,9 +3680,6 @@ impl Methods { } else {path_join_correction::check(cx, expr, join_arg, span);} }, - /*("join", [join_arg]) => { - path_join_correction::check(cx, expr, join_arg, span); - },*/ ("last", []) | ("skip", [_]) => { if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { if let ("cloned", []) = (name2, args2) { diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index 6ed8c06ad7ae..236c0ce7b912 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -1,34 +1,28 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use rustc_lint::{LateContext}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; use rustc_span::Span; use super::PATH_JOIN_CORRECTION; - -pub(super) fn check<'tcx>( - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'tcx>, - join_arg: &'tcx Expr<'tcx>, - span: Span, -) { - let applicability = Applicability::MachineApplicable; - if_chain!( - if let ExprKind::Lit(spanned) = &join_arg.kind; - if let LitKind::Str(symbol, _) = spanned.node; - if symbol.as_str().starts_with('/'); - then { - span_lint_and_sugg( - cx, - PATH_JOIN_CORRECTION, - span.with_hi(expr.span.hi()), - r#"argument in join called on path contains a starting '/'"#, - "try removing first '/'", - "join(\"your/path/here\")".to_owned(), - applicability - ); - } - ); +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, span: Span) { + let applicability = Applicability::MachineApplicable; + if_chain!( + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.as_str().starts_with('/'); + then { + span_lint_and_sugg( + cx, + PATH_JOIN_CORRECTION, + span.with_hi(expr.span.hi()), + r#"argument in join called on path contains a starting '/'"#, + "try removing first '/'", + "join(\"your/path/here\")".to_owned(), + applicability + ); + } + ); } diff --git a/tests/ui/path_join_correction.fixed b/tests/ui/path_join_correction.fixed new file mode 100644 index 000000000000..73a47ea625c0 --- /dev/null +++ b/tests/ui/path_join_correction.fixed @@ -0,0 +1,15 @@ +// run-rustfix +#![allow(unused)] +#![warn(clippy::path_join_correction)] + +fn main() { + // should be linted + let path = std::path::Path::new("/bin"); + path.join("your/path/here"); + println!("{}", path.display()); + + //should not be linted + let path = std::path::Path::new("/bin"); + path.join("sh"); + println!("{}", path.display()); +} diff --git a/tests/ui/path_join_correction.rs b/tests/ui/path_join_correction.rs index aaa33b9fb4db..b1ee188b9f11 100644 --- a/tests/ui/path_join_correction.rs +++ b/tests/ui/path_join_correction.rs @@ -1,15 +1,15 @@ +// run-rustfix #![allow(unused)] #![warn(clippy::path_join_correction)] fn main() { - // should be linted - let path = std::path::Path::new("/bin"); - path.join("/sh"); - println!("{}", path.display()); + // should be linted + let path = std::path::Path::new("/bin"); + path.join("/sh"); + println!("{}", path.display()); - //should not be linted - let path = std::path::Path::new("/bin"); - path.join("sh"); - println!("{}", path.display()); - + //should not be linted + let path = std::path::Path::new("/bin"); + path.join("sh"); + println!("{}", path.display()); } diff --git a/tests/ui/path_join_correction.stderr b/tests/ui/path_join_correction.stderr index c73dff01e28a..6c197322145a 100644 --- a/tests/ui/path_join_correction.stderr +++ b/tests/ui/path_join_correction.stderr @@ -1,8 +1,8 @@ error: argument in join called on path contains a starting '/' - --> $DIR/path_join_correction.rs:7:8 + --> $DIR/path_join_correction.rs:8:10 | -LL | path.join("/sh"); - | ^^^^^^^^^^^ help: try removing first '/': `join("your/path/here")` +LL | path.join("/sh"); + | ^^^^^^^^^^^ help: try removing first '/': `join("your/path/here")` | = note: `-D clippy::path-join-correction` implied by `-D warnings` From 436115190b93e99d35a8d5cf20d4633bf1508fb8 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 24 Apr 2023 21:22:51 -0400 Subject: [PATCH 04/15] Added type check and test case for string slice. --- .../src/methods/path_join_correction.rs | 39 ++++++++++--------- tests/ui/path_join_correction.fixed | 13 +++++-- tests/ui/path_join_correction.rs | 11 +++++- tests/ui/path_join_correction.stderr | 10 ----- 4 files changed, 40 insertions(+), 33 deletions(-) delete mode 100644 tests/ui/path_join_correction.stderr diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index 236c0ce7b912..4bc6876ce764 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -1,28 +1,31 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +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::Span; +use rustc_span::{symbol::sym::Path, Span}; use super::PATH_JOIN_CORRECTION; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, span: Span) { - let applicability = Applicability::MachineApplicable; - if_chain!( - if let ExprKind::Lit(spanned) = &join_arg.kind; - if let LitKind::Str(symbol, _) = spanned.node; - if symbol.as_str().starts_with('/'); - then { - span_lint_and_sugg( - cx, - PATH_JOIN_CORRECTION, - span.with_hi(expr.span.hi()), - r#"argument in join called on path contains a starting '/'"#, - "try removing first '/'", - "join(\"your/path/here\")".to_owned(), - applicability + let ty = cx.typeck_results().expr_ty(expr); + if is_type_diagnostic_item(cx, ty, Path) { + let applicability = Applicability::MachineApplicable; + if_chain!( + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.as_str().starts_with('/'); + then { + span_lint_and_sugg( + cx, + PATH_JOIN_CORRECTION, + span.with_hi(expr.span.hi()), + r#"argument in join called on path contains a starting '/'"#, + "try removing first '/'", + "join(\"your/path/here\")".to_owned(), + applicability + ); + } ); - } - ); + } } diff --git a/tests/ui/path_join_correction.fixed b/tests/ui/path_join_correction.fixed index 73a47ea625c0..83dad36b06d1 100644 --- a/tests/ui/path_join_correction.fixed +++ b/tests/ui/path_join_correction.fixed @@ -1,15 +1,22 @@ // run-rustfix #![allow(unused)] #![warn(clippy::path_join_correction)] +use std::path::Path; +//use std::string::String; fn main() { // should be linted - let path = std::path::Path::new("/bin"); - path.join("your/path/here"); + let path = Path::new("/bin"); + path.join("/sh"); println!("{}", path.display()); + // should not be linted + let path: &[&str] = &["/bin"]; + path.join("/sh"); + println!("{:?}", path); + //should not be linted - let path = std::path::Path::new("/bin"); + let path = Path::new("/bin"); path.join("sh"); println!("{}", path.display()); } diff --git a/tests/ui/path_join_correction.rs b/tests/ui/path_join_correction.rs index b1ee188b9f11..83dad36b06d1 100644 --- a/tests/ui/path_join_correction.rs +++ b/tests/ui/path_join_correction.rs @@ -1,15 +1,22 @@ // run-rustfix #![allow(unused)] #![warn(clippy::path_join_correction)] +use std::path::Path; +//use std::string::String; fn main() { // should be linted - let path = std::path::Path::new("/bin"); + let path = Path::new("/bin"); path.join("/sh"); println!("{}", path.display()); + // should not be linted + let path: &[&str] = &["/bin"]; + path.join("/sh"); + println!("{:?}", path); + //should not be linted - let path = std::path::Path::new("/bin"); + let path = Path::new("/bin"); path.join("sh"); println!("{}", path.display()); } diff --git a/tests/ui/path_join_correction.stderr b/tests/ui/path_join_correction.stderr deleted file mode 100644 index 6c197322145a..000000000000 --- a/tests/ui/path_join_correction.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: argument in join called on path contains a starting '/' - --> $DIR/path_join_correction.rs:8:10 - | -LL | path.join("/sh"); - | ^^^^^^^^^^^ help: try removing first '/': `join("your/path/here")` - | - = note: `-D clippy::path-join-correction` implied by `-D warnings` - -error: aborting due to previous error - From 25ee5878f9cd251488f38cc59077a7e0e9d9c27d Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Wed, 26 Apr 2023 11:25:42 -0400 Subject: [PATCH 05/15] Added supposed check for windows paths. --- clippy_lints/src/methods/path_join_correction.rs | 4 ++-- tests/ui/path_join_correction.fixed | 5 +++++ tests/ui/path_join_correction.rs | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index 4bc6876ce764..7c245ad2ca2c 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -14,14 +14,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_a if_chain!( if let ExprKind::Lit(spanned) = &join_arg.kind; if let LitKind::Str(symbol, _) = spanned.node; - if symbol.as_str().starts_with('/'); + if symbol.as_str().starts_with('/') || symbol.as_str().starts_with('\\'); then { span_lint_and_sugg( cx, PATH_JOIN_CORRECTION, span.with_hi(expr.span.hi()), r#"argument in join called on path contains a starting '/'"#, - "try removing first '/'", + "try removing first '/' or '\\'", "join(\"your/path/here\")".to_owned(), applicability ); diff --git a/tests/ui/path_join_correction.fixed b/tests/ui/path_join_correction.fixed index 83dad36b06d1..ffac21149c01 100644 --- a/tests/ui/path_join_correction.fixed +++ b/tests/ui/path_join_correction.fixed @@ -10,6 +10,11 @@ fn main() { 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"); diff --git a/tests/ui/path_join_correction.rs b/tests/ui/path_join_correction.rs index 83dad36b06d1..ffac21149c01 100644 --- a/tests/ui/path_join_correction.rs +++ b/tests/ui/path_join_correction.rs @@ -10,6 +10,11 @@ fn main() { 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"); From c5b5560ddf84d684ed2dab4f66736083bd73cf9d Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Wed, 26 Apr 2023 11:33:35 -0400 Subject: [PATCH 06/15] Build failure fix. --- tests/ui/path_join_correction.fixed | 1 - tests/ui/path_join_correction.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/ui/path_join_correction.fixed b/tests/ui/path_join_correction.fixed index ffac21149c01..c0f98ff2fd6a 100644 --- a/tests/ui/path_join_correction.fixed +++ b/tests/ui/path_join_correction.fixed @@ -2,7 +2,6 @@ #![allow(unused)] #![warn(clippy::path_join_correction)] use std::path::Path; -//use std::string::String; fn main() { // should be linted diff --git a/tests/ui/path_join_correction.rs b/tests/ui/path_join_correction.rs index ffac21149c01..c0f98ff2fd6a 100644 --- a/tests/ui/path_join_correction.rs +++ b/tests/ui/path_join_correction.rs @@ -2,7 +2,6 @@ #![allow(unused)] #![warn(clippy::path_join_correction)] use std::path::Path; -//use std::string::String; fn main() { // should be linted From 412dfa6508bced40091e69c7f817ad3d684e3bc6 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Wed, 26 Apr 2023 11:52:55 -0400 Subject: [PATCH 07/15] Build fix attempt #2. --- .../src/methods/path_join_correction.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index 7c245ad2ca2c..1be8e79e262c 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -9,17 +9,17 @@ use super::PATH_JOIN_CORRECTION; 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 is_type_diagnostic_item(cx, ty, Path) { - let applicability = Applicability::MachineApplicable; - if_chain!( - 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, - PATH_JOIN_CORRECTION, - span.with_hi(expr.span.hi()), + 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, + PATH_JOIN_CORRECTION, + 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(), @@ -27,5 +27,4 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_a ); } ); - } } From e1718e0dab285ece82b3f50b326027629b95dfa8 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 1 May 2023 11:09:05 -0400 Subject: [PATCH 08/15] Fixed formatting. --- clippy_lints/src/methods/mod.rs | 27 ++++++++++++++----- .../src/methods/path_join_correction.rs | 24 ++++++++--------- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 2904727fec76..0f651c760f81 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3218,19 +3218,32 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// ### What it does - /// Checks for initial '/' in an argument to .join on a path. + /// Checks for initial `'/'` in an argument to `.join()` on a `Path`. + /// /// ### Why is this bad? - /// .join() calls on paths already attach the '/'. + /// `.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 - /// // example code where clippy issues a warning /// let path = std::path::Path::new("/bin"); - /// path.join("/sh"); + /// let res = path.join("/sh"); + /// assert_eq!(res, PathBuf::from("/sh")); /// ``` + /// /// Use instead: /// ```rust - /// // path.join("sh"); + /// let path = std::path::Path::new("/bin"); + /// + /// // If this was unintentional, remove the leading separator + /// let extend = path.join("sh"); + /// assert_eq!(extend, 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.70.0"] pub PATH_JOIN_CORRECTION, @@ -3678,7 +3691,7 @@ impl Methods { if let Some(("collect", _, _, span, _)) = method_call(recv) { unnecessary_join::check(cx, expr, recv, join_arg, span); } - else {path_join_correction::check(cx, expr, join_arg, span);} + else {path_join_correction::check(cx, expr, join_arg, span);} }, ("last", []) | ("skip", [_]) => { if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/path_join_correction.rs index 1be8e79e262c..441eef7124d6 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/path_join_correction.rs @@ -10,21 +10,21 @@ use super::PATH_JOIN_CORRECTION; 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('\\'); + 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, PATH_JOIN_CORRECTION, 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 - ); - } - ); + r#"argument in join called on path contains a starting '/'"#, + "try removing first '/' or '\\'", + "join(\"your/path/here\")".to_owned(), + applicability + ); + } + ); } From 08a8a81af2a0709f6505805d82330892d87a8b4f Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 15 May 2023 12:12:04 -0400 Subject: [PATCH 09/15] Rebase and merge attempt number 2. --- clippy_lints/src/methods/mod.rs | 16 +++++++--------- tests/ui/path_join_correction.fixed | 26 -------------------------- 2 files changed, 7 insertions(+), 35 deletions(-) delete mode 100644 tests/ui/path_join_correction.fixed diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4eac643adf5a..c755e610807a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3196,7 +3196,7 @@ declare_clippy_lint! { } declare_clippy_lint! { -<<<<<<< HEAD + /// ### What it does /// Checks for initial `'/'` in an argument to `.join()` on a `Path`. /// /// ### Why is this bad? @@ -3208,11 +3208,11 @@ declare_clippy_lint! { /// ### Example /// ```rust /// let path = std::path::Path::new("/bin"); - /// let res = path.join("/sh"); + /// let res = path.join("/sh"); /// assert_eq!(res, PathBuf::from("/sh")); /// ``` /// - /// Use instead: + /// Use instead; /// ```rust /// let path = std::path::Path::new("/bin"); /// @@ -3227,8 +3227,10 @@ declare_clippy_lint! { #[clippy::version = "1.70.0"] pub PATH_JOIN_CORRECTION, pedantic, - "arg to .join called on a Path contains '/' at the start" -======= + "arg to .join called on a Path contains '/' at the start" +} + +declare_clippy_lint! { /// ### What it does /// Checks for `.rev().next()` on a `DoubleEndedIterator` /// @@ -3249,7 +3251,6 @@ declare_clippy_lint! { pub MANUAL_NEXT_BACK, style, "manual reverse iteration of `DoubleEndedIterator`" ->>>>>>> upstream/master } pub struct Methods { @@ -3380,11 +3381,8 @@ impl_lint_pass!(Methods => [ NEEDLESS_COLLECT, SUSPICIOUS_COMMAND_ARG_SPACE, CLEAR_WITH_DRAIN, -<<<<<<< HEAD PATH_JOIN_CORRECTION, -======= MANUAL_NEXT_BACK, ->>>>>>> upstream/master ]); /// Extracts a method call name, args, and `Span` of the method name. diff --git a/tests/ui/path_join_correction.fixed b/tests/ui/path_join_correction.fixed deleted file mode 100644 index c0f98ff2fd6a..000000000000 --- a/tests/ui/path_join_correction.fixed +++ /dev/null @@ -1,26 +0,0 @@ -// run-rustfix -#![allow(unused)] -#![warn(clippy::path_join_correction)] -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()); -} From 606d80132a87c5a770d13a3a56e758a7360411fc Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 15 May 2023 12:39:18 -0400 Subject: [PATCH 10/15] Included sources to std::path in lint string. --- clippy_lints/src/methods/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c755e610807a..0a098623d418 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3208,8 +3208,8 @@ declare_clippy_lint! { /// ### Example /// ```rust /// let path = std::path::Path::new("/bin"); - /// let res = path.join("/sh"); - /// assert_eq!(res, PathBuf::from("/sh")); + /// let res = path.join("/sh"); + /// assert_eq!(res, std::path::PathBuf::from("/sh")); /// ``` /// /// Use instead; @@ -3218,11 +3218,11 @@ declare_clippy_lint! { /// /// // If this was unintentional, remove the leading separator /// let extend = path.join("sh"); - /// assert_eq!(extend, PathBuf::from("/bin/sh")); + /// assert_eq!(extend, std::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")); + /// let new = std::path::Path::new("/sh") + /// assert_eq!(new std::path::PathBuf::from("/sh")); /// ``` #[clippy::version = "1.70.0"] pub PATH_JOIN_CORRECTION, From 048bcc2bcee6436abfeb3e223c28165ace7b6552 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 15 May 2023 12:12:04 -0400 Subject: [PATCH 11/15] Rebase and merge attempt number 2. --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 0a098623d418..bb6949468b53 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3221,7 +3221,7 @@ declare_clippy_lint! { /// 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") + /// let new = std::path::Path::new("/sh"); /// assert_eq!(new std::path::PathBuf::from("/sh")); /// ``` #[clippy::version = "1.70.0"] From c36de321f35f6f54e81edb089ace9b83b989f7d6 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Mon, 15 May 2023 12:59:30 -0400 Subject: [PATCH 12/15] Fixed arguments in lint string. --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index bb6949468b53..528d471ba8c1 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3222,7 +3222,7 @@ declare_clippy_lint! { /// /// // If this was intentional, create a new path instead /// let new = std::path::Path::new("/sh"); - /// assert_eq!(new std::path::PathBuf::from("/sh")); + /// assert_eq!(new, std::path::PathBuf::from("/sh")); /// ``` #[clippy::version = "1.70.0"] pub PATH_JOIN_CORRECTION, From 18bd97fdd44562769a11a92c0cbef40497bc0f87 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Fri, 14 Jul 2023 18:24:35 -0400 Subject: [PATCH 13/15] Renamed lint to join_absolute_path as per request. --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- .../{path_join_correction.rs => join_absolute_path.rs} | 4 ++-- clippy_lints/src/methods/mod.rs | 8 ++++---- .../ui/{path_join_correction.rs => join_absolute_path.rs} | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename clippy_lints/src/methods/{path_join_correction.rs => join_absolute_path.rs} (94%) rename tests/ui/{path_join_correction.rs => join_absolute_path.rs} (93%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54fc6318f50a..e5325d3f024a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4935,7 +4935,7 @@ Released 2018-09-13 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite -[`path_join_correction`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_join_correction +[`join_absolute_path`]: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_path [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 94ecc79c6550..684f0202e877 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -380,7 +380,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::OR_FUN_CALL_INFO, crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, - crate::methods::PATH_JOIN_CORRECTION_INFO, + crate::methods::JOIN_ABSOLUTE_PATH_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, diff --git a/clippy_lints/src/methods/path_join_correction.rs b/clippy_lints/src/methods/join_absolute_path.rs similarity index 94% rename from clippy_lints/src/methods/path_join_correction.rs rename to clippy_lints/src/methods/join_absolute_path.rs index 441eef7124d6..4788540a9e65 100644 --- a/clippy_lints/src/methods/path_join_correction.rs +++ b/clippy_lints/src/methods/join_absolute_path.rs @@ -5,7 +5,7 @@ use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; use rustc_span::{symbol::sym::Path, Span}; -use super::PATH_JOIN_CORRECTION; +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); @@ -18,7 +18,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_a then { span_lint_and_sugg( cx, - PATH_JOIN_CORRECTION, + JOIN_ABSOLUTE_PATH, span.with_hi(expr.span.hi()), r#"argument in join called on path contains a starting '/'"#, "try removing first '/' or '\\'", diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 528d471ba8c1..b2630ee24048 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -69,7 +69,7 @@ mod option_map_unwrap_or; mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; -mod path_join_correction; +mod join_absolute_path; mod range_zip_with_len; mod repeat_once; mod search_is_some; @@ -3225,7 +3225,7 @@ declare_clippy_lint! { /// assert_eq!(new, std::path::PathBuf::from("/sh")); /// ``` #[clippy::version = "1.70.0"] - pub PATH_JOIN_CORRECTION, + pub JOIN_ABSOLUTE_PATH, pedantic, "arg to .join called on a Path contains '/' at the start" } @@ -3381,7 +3381,7 @@ impl_lint_pass!(Methods => [ NEEDLESS_COLLECT, SUSPICIOUS_COMMAND_ARG_SPACE, CLEAR_WITH_DRAIN, - PATH_JOIN_CORRECTION, + JOIN_ABSOLUTE_PATH, MANUAL_NEXT_BACK, ]); @@ -3691,7 +3691,7 @@ impl Methods { if let Some(("collect", _, _, span, _)) = method_call(recv) { unnecessary_join::check(cx, expr, recv, join_arg, span); } - else {path_join_correction::check(cx, expr, 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/path_join_correction.rs b/tests/ui/join_absolute_path.rs similarity index 93% rename from tests/ui/path_join_correction.rs rename to tests/ui/join_absolute_path.rs index c0f98ff2fd6a..457a5528b41c 100644 --- a/tests/ui/path_join_correction.rs +++ b/tests/ui/join_absolute_path.rs @@ -1,6 +1,6 @@ // run-rustfix #![allow(unused)] -#![warn(clippy::path_join_correction)] +#![warn(clippy::join_absolute_path)] use std::path::Path; fn main() { From b694718cfbdf8455743061e69f7d1072c2792fa8 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Fri, 14 Jul 2023 18:28:27 -0400 Subject: [PATCH 14/15] Ran cargo dev update_lints --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5325d3f024a..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 @@ -4935,7 +4936,6 @@ Released 2018-09-13 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite -[`join_absolute_path`]: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_path [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 684f0202e877..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, @@ -380,7 +381,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::OR_FUN_CALL_INFO, crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, - crate::methods::JOIN_ABSOLUTE_PATH_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, From b91f9e0b2086d3de36b8de79a4f1702e4b92b8a9 Mon Sep 17 00:00:00 2001 From: Mason Ray Hanna III Date: Fri, 14 Jul 2023 18:34:06 -0400 Subject: [PATCH 15/15] Ran cargo dev bless after a full cargo test --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index b2630ee24048..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; @@ -69,7 +70,6 @@ mod option_map_unwrap_or; mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; -mod join_absolute_path; mod range_zip_with_len; mod repeat_once; mod search_is_some;