Auto merge of #11453 - xFrednet:11208-path-join-correct, r=blyxyas

New lint `clippy::join_absolute_paths`

Hey `@ofeeg,` this PR is a copy of all the changes you did in 47171d3c63 from PR https://github.com/rust-lang/rust-clippy/pull/11208. I wasn't sure how to fix the git history. Hope you're okay with me figuring this out in this separate PR

---

changelog: New lint [`join_absolute_paths`]
[#11453](https://github.com/rust-lang/rust-clippy/pull/11453)

Closes: https://github.com/rust-lang/rust-clippy/issues/10655

r? `@Centri3` Since you also gave feedback on the other PR. I hope that I copied everything correctly, but a third pair of eyes would be appreciated :D
This commit is contained in:
bors 2023-11-20 12:30:03 +00:00
commit 11a2eb03fa
6 changed files with 196 additions and 0 deletions

View file

@ -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

View file

@ -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,

View file

@ -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)
};
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,
);
},
);
}
}

View file

@ -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;
@ -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,
@ -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.
@ -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", []) => {

View file

@ -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");
//~^ 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");
}

View file

@ -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