Skip to content
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

New lint clippy::join_absolute_paths #11453

Merged
merged 1 commit into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5190,6 +5190,7 @@ Released 2018-09-13
[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
[`iter_without_into_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_without_into_iter
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`join_absolute_paths`]: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths
[`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 @@ -377,6 +377,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::ITER_SKIP_NEXT_INFO,
crate::methods::ITER_SKIP_ZERO_INFO,
crate::methods::ITER_WITH_DRAIN_INFO,
crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
crate::methods::MANUAL_FILTER_MAP_INFO,
crate::methods::MANUAL_FIND_MAP_INFO,
crate::methods::MANUAL_NEXT_BACK_INFO,
Expand Down
52 changes: 52 additions & 0 deletions clippy_lints/src/methods/join_absolute_paths.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::expr_or_init;
use clippy_utils::source::snippet_opt;
use clippy_utils::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;
use rustc_span::Span;

use super::JOIN_ABSOLUTE_PATHS;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, recv: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, expr_span: Span) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
if (is_type_diagnostic_item(cx, ty, sym::Path) || is_type_diagnostic_item(cx, ty, sym::PathBuf))
&& let ExprKind::Lit(spanned) = expr_or_init(cx, join_arg).kind
&& let LitKind::Str(symbol, _) = spanned.node
&& let sym_str = symbol.as_str()
&& sym_str.starts_with(['/', '\\'])
{
span_lint_and_then(
cx,
JOIN_ABSOLUTE_PATHS,
join_arg.span,
"argument to `Path::join` starts with a path separator",
|diag| {
let arg_str = snippet_opt(cx, spanned.span).unwrap_or_else(|| "..".to_string());

let no_separator = if sym_str.starts_with('/') {
arg_str.replacen('/', "", 1)
} else {
arg_str.replacen('\\', "", 1)
};
xFrednet marked this conversation as resolved.
Show resolved Hide resolved

diag.note("joining a path starting with separator will replace the path instead")
.span_suggestion(
spanned.span,
"if this is unintentional, try removing the starting separator",
no_separator,
Applicability::Unspecified,
)
.span_suggestion(
expr_span,
"if this is intentional, try using `Path::new` instead",
format!("PathBuf::from({arg_str})"),
Applicability::Unspecified,
);
},
);
}
}
44 changes: 44 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ mod iter_skip_next;
mod iter_skip_zero;
mod iter_with_drain;
mod iterator_step_by_zero;
mod join_absolute_paths;
mod manual_next_back;
mod manual_ok_or;
mod manual_saturating_arithmetic;
Expand Down Expand Up @@ -3684,6 +3685,46 @@ declare_clippy_lint! {
"calling the `try_from` and `try_into` trait methods when `From`/`Into` is implemented"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `Path::join` that start with a path separator (`\\` or `/`).
///
/// ### Why is this bad?
/// If the argument to `Path::join` starts with a separator, it will overwrite
/// the original path. If this is intentional, prefer using `Path::new` instead.
///
/// Note the behavior is platform dependent. A leading `\\` will be accepted
/// on unix systems as part of the file name
///
/// See [`Path::join`](https://doc.rust-lang.org/std/path/struct.Path.html#method.join)
///
/// ### Example
/// ```rust
/// # use std::path::{Path, PathBuf};
/// let path = Path::new("/bin");
/// let joined_path = path.join("/sh");
/// assert_eq!(joined_path, PathBuf::from("/sh"));
/// ```
///
/// Use instead;
/// ```rust
/// # use std::path::{Path, PathBuf};
/// let path = Path::new("/bin");
///
/// // If this was unintentional, remove the leading separator
/// let joined_path = path.join("sh");
/// assert_eq!(joined_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"));
/// ```
#[clippy::version = "1.76.0"]
pub JOIN_ABSOLUTE_PATHS,
suspicious,
"calls to `Path::join` which will overwrite the original path"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -3833,6 +3874,7 @@ impl_lint_pass!(Methods => [
REDUNDANT_AS_STR,
WAKER_CLONE_WAKE,
UNNECESSARY_FALLIBLE_CONVERSIONS,
JOIN_ABSOLUTE_PATHS,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4235,6 +4277,8 @@ impl Methods {
("join", [join_arg]) => {
if let Some(("collect", _, _, span, _)) = method_call(recv) {
unnecessary_join::check(cx, expr, recv, join_arg, span);
} else {
join_absolute_paths::check(cx, recv, join_arg, expr.span);
}
},
("last", []) => {
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/join_absolute_paths.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//@no-rustfix

#![allow(clippy::needless_raw_string_hashes)]
#![warn(clippy::join_absolute_paths)]

use std::path::{Path, PathBuf};

fn main() {
let path = Path::new("/bin");
path.join("/sh");
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
//~^ ERROR: argument to `Path::join` starts with a path separator

let path = Path::new("C:\\Users");
path.join("\\user");
//~^ ERROR: argument to `Path::join` starts with a path separator

let path = PathBuf::from("/bin");
path.join("/sh");
//~^ ERROR: argument to `Path::join` starts with a path separator

let path = PathBuf::from("/bin");
path.join(r#"/sh"#);
//~^ ERROR: argument to `Path::join` starts with a path separator

let path: &[&str] = &["/bin"];
path.join("/sh");

let path = Path::new("/bin");
path.join("sh");
}
68 changes: 68 additions & 0 deletions tests/ui/join_absolute_paths.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
error: argument to `Path::join` starts with a path separator
--> $DIR/join_absolute_paths.rs:10:15
|
LL | path.join("/sh");
| ^^^^^
|
= note: joining a path starting with separator will replace the path instead
= note: `-D clippy::join-absolute-paths` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::join_absolute_paths)]`
help: if this is unintentional, try removing the starting separator
|
LL | path.join("sh");
| ~~~~
help: if this is intentional, try using `Path::new` instead
|
LL | PathBuf::from("/sh");
| ~~~~~~~~~~~~~~~~~~~~

error: argument to `Path::join` starts with a path separator
--> $DIR/join_absolute_paths.rs:14:15
|
LL | path.join("\\user");
| ^^^^^^^^
|
= note: joining a path starting with separator will replace the path instead
help: if this is unintentional, try removing the starting separator
|
LL | path.join("\user");
| ~~~~~~~
help: if this is intentional, try using `Path::new` instead
|
LL | PathBuf::from("\\user");
| ~~~~~~~~~~~~~~~~~~~~~~~

error: argument to `Path::join` starts with a path separator
--> $DIR/join_absolute_paths.rs:18:15
|
LL | path.join("/sh");
| ^^^^^
|
= note: joining a path starting with separator will replace the path instead
help: if this is unintentional, try removing the starting separator
|
LL | path.join("sh");
| ~~~~
help: if this is intentional, try using `Path::new` instead
|
LL | PathBuf::from("/sh");
| ~~~~~~~~~~~~~~~~~~~~

error: argument to `Path::join` starts with a path separator
--> $DIR/join_absolute_paths.rs:22:15
|
LL | path.join(r#"/sh"#);
| ^^^^^^^^
|
= note: joining a path starting with separator will replace the path instead
help: if this is unintentional, try removing the starting separator
|
LL | path.join(r#"sh"#);
| ~~~~~~~
help: if this is intentional, try using `Path::new` instead
|
LL | PathBuf::from(r#"/sh"#);
| ~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 4 previous errors