Skip to content

Added new lint "path_join_correction" #10663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 16 commits into from
Closed
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions clippy_lints/src/methods/join_absolute_path.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
);
}
38 changes: 38 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Checks for initial `'/'` in an argument to `.join()` on a `Path`.
/// Checks for initial `/` or `\\` in an argument to `.join()` on a `Path`.

///
/// ### Why is this bad?
/// `.join()` comments starting with a separator, can replace the entire path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// `.join()` comments starting with a separator, can replace the entire path.
/// `.join()` comments starting with a separator (`/` or `\\`) 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`
Expand Down Expand Up @@ -3345,6 +3381,7 @@ impl_lint_pass!(Methods => [
NEEDLESS_COLLECT,
SUSPICIOUS_COMMAND_ARG_SPACE,
CLEAR_WITH_DRAIN,
JOIN_ABSOLUTE_PATH,
MANUAL_NEXT_BACK,
]);

Expand Down Expand Up @@ -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) {
Expand Down
26 changes: 26 additions & 0 deletions tests/ui/join_absolute_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// run-rustfix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// run-rustfix
//@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());
}