Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-26 22:50:56 +00:00
Code Issues Projects Releases Packages Wiki Activity

fixes: #7889

Browse source 
1. Fix the problem of manual_split_once changing the original behavior.
2. Add a new lint needless_splitn.

changelog: Fix the problem of manual_split_once changing the original behavior and add a new lint needless_splitn.
This commit is contained in:
surechen 2021-10-29 11:32:37 +08:00
parent ed71addee7
commit c051656c83
14 changed files with 276 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -3038,6 +3038,7 @@ Released 2018-09-13
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
[`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
[`needless_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_splitn
[`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
[`neg_cmp_op_on_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_cmp_op_on_partial_ord
[`neg_multiply`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply

1
clippy_lints/src/lib.register_all.rs
View file

@ -160,6 +160,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::MANUAL_STR_REPEAT),
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
LintId::of(methods::MAP_IDENTITY),
LintId::of(methods::NEEDLESS_SPLITN),
LintId::of(methods::NEW_RET_NO_SELF),
LintId::of(methods::OK_EXPECT),
LintId::of(methods::OPTION_AS_REF_DEREF),

1
clippy_lints/src/lib.register_complexity.rs
View file

@ -42,6 +42,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(methods::MANUAL_FIND_MAP),
LintId::of(methods::MANUAL_SPLIT_ONCE),
LintId::of(methods::MAP_IDENTITY),
LintId::of(methods::NEEDLESS_SPLITN),
LintId::of(methods::OPTION_AS_REF_DEREF),
LintId::of(methods::OPTION_FILTER_MAP),
LintId::of(methods::SEARCH_IS_SOME),

1
clippy_lints/src/lib.register_lints.rs
View file

@ -288,6 +288,7 @@ store.register_lints(&[
methods::MAP_FLATTEN,
methods::MAP_IDENTITY,
methods::MAP_UNWRAP_OR,
methods::NEEDLESS_SPLITN,
methods::NEW_RET_NO_SELF,
methods::OK_EXPECT,
methods::OPTION_AS_REF_DEREF,

34
clippy_lints/src/methods/mod.rs
View file

@ -33,7 +33,6 @@ mod iter_nth_zero;
mod iter_skip_next;
mod iterator_step_by_zero;
mod manual_saturating_arithmetic;
mod manual_split_once;
mod manual_str_repeat;
mod map_collect_result_unit;
mod map_flatten;
@ -50,6 +49,7 @@ mod single_char_insert_string;
mod single_char_pattern;
mod single_char_push_string;
mod skip_while_next;
mod str_splitn;
mod string_extend_chars;
mod suspicious_map;
mod suspicious_splitn;
@ -1798,6 +1798,30 @@ declare_clippy_lint! {
"replace `.splitn(2, pat)` with `.split_once(pat)`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `str::splitn` (or `str::rsplitn`) where using `str::split` would be the same.
/// ### Why is this bad?
/// The function `split` is simpler and there is no performance difference in these cases, considering
/// that both functions return a lazy iterator.
/// ### Example
/// ```rust
/// // Bad
/// let str = "key=value=add";
/// let _ = str.splitn(3, '=').next().unwrap();
/// ```
/// Use instead:
/// ```rust
/// // Good
/// let str = "key=value=add";
/// let _ = str.split('=').next().unwrap();
/// ```
#[clippy::version = "1.58.0"]
pub NEEDLESS_SPLITN,
complexity,
"usages of `str::splitn` that can be replaced with `str::split`"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -1876,7 +1900,8 @@ impl_lint_pass!(Methods => [
SUSPICIOUS_SPLITN,
MANUAL_STR_REPEAT,
EXTEND_WITH_DRAIN,
MANUAL_SPLIT_ONCE
MANUAL_SPLIT_ONCE,
NEEDLESS_SPLITN
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -2208,7 +2233,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
suspicious_splitn::check(cx, name, expr, recv, count);
if count == 2 && meets_msrv(msrv, &msrvs::STR_SPLIT_ONCE) {
manual_split_once::check(cx, name, expr, recv, pat_arg);
str_splitn::check_manual_split_once(cx, name, expr, recv, pat_arg);
}
if count >= 2 {
str_splitn::check_needless_splitn(cx, name, expr, recv, pat_arg, count);
}
}
},

150
clippy_lints/src/methods/manual_split_once.rs → clippy_lints/src/methods/str_splitn.rs
View file

@ -11,7 +11,13 @@ use rustc_span::{symbol::sym, Span, SyntaxContext};
use super::MANUAL_SPLIT_ONCE;
pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, self_arg: &Expr<'_>, pat_arg: &Expr<'_>) {
pub(super) fn check_manual_split_once(
cx: &LateContext<'_>,
method_name: &str,
expr: &Expr<'_>,
self_arg: &Expr<'_>,
pat_arg: &Expr<'_>,
) {
if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() {
return;
}
@ -36,7 +42,7 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se
format!("{}.{}({})", self_snip, method_name, pat_snip)
},
IterUsageKind::RNextTuple => format!("{}.{}({}).map(|(x, y)| (y, x))", self_snip, method_name, pat_snip),
IterUsageKind::Next => {
IterUsageKind::Next | IterUsageKind::Second => {
let self_deref = {
let adjust = cx.typeck_results().expr_adjustments(self_arg);
if adjust.is_empty() {
@ -51,26 +57,49 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se
"*".repeat(adjust.len() - 2)
}
};
if usage.unwrap_kind.is_some() {
format!(
"{}.{}({}).map_or({}{}, |x| x.0)",
&self_snip, method_name, pat_snip, self_deref, &self_snip
)
if matches!(usage.kind, IterUsageKind::Next) {
match usage.unwrap_kind {
Some(UnwrapKind::Unwrap) => {
if reverse {
format!("{}.{}({}).unwrap().0", self_snip, method_name, pat_snip)
} else {
format!(
"{}.{}({}).map_or({}{}, |x| x.0)",
self_snip, method_name, pat_snip, self_deref, &self_snip
)
}
},
Some(UnwrapKind::QuestionMark) => {
format!(
"{}.{}({}).map_or({}{}, |x| x.0)",
self_snip, method_name, pat_snip, self_deref, &self_snip
)
},
None => {
format!(
"Some({}.{}({}).map_or({}{}, |x| x.0))",
&self_snip, method_name, pat_snip, self_deref, &self_snip
)
},
}
} else {
format!(
"Some({}.{}({}).map_or({}{}, |x| x.0))",
&self_snip, method_name, pat_snip, self_deref, &self_snip
)
match usage.unwrap_kind {
Some(UnwrapKind::Unwrap) => {
if reverse {
// In this case, no better suggestion is offered.
return;
}
format!("{}.{}({}).unwrap().1", self_snip, method_name, pat_snip)
},
Some(UnwrapKind::QuestionMark) => {
format!("{}.{}({})?.1", self_snip, method_name, pat_snip)
},
None => {
format!("{}.{}({}).map(|x| x.1)", self_snip, method_name, pat_snip)
},
}
}
},
IterUsageKind::Second => {
let access_str = match usage.unwrap_kind {
Some(UnwrapKind::Unwrap) => ".unwrap().1",
Some(UnwrapKind::QuestionMark) => "?.1",
None => ".map(|x| x.1)",
};
format!("{}.{}({}){}", self_snip, method_name, pat_snip, access_str)
},
};
span_lint_and_sugg(cx, MANUAL_SPLIT_ONCE, usage.span, msg, "try this", sugg, app);
@ -209,3 +238,86 @@ fn parse_iter_usage(
span,
})
}
use super::NEEDLESS_SPLITN;
pub(super) fn check_needless_splitn(
cx: &LateContext<'_>,
method_name: &str,
expr: &Expr<'_>,
self_arg: &Expr<'_>,
pat_arg: &Expr<'_>,
count: u128,
) {
if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() {
return;
}
let ctxt = expr.span.ctxt();
let mut app = Applicability::MachineApplicable;
let (reverse, message) = if method_name == "splitn" {
(false, "unnecessary use of `splitn`")
} else {
(true, "unnecessary use of `rsplitn`")
};
if_chain! {
if count >= 2;
if check_iter(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id), count);
then {
span_lint_and_sugg(
cx,
NEEDLESS_SPLITN,
expr.span,
message,
"try this",
format!(
"{}.{}({})",
snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0,
if reverse {"rsplit"} else {"split"},
snippet_with_context(cx, pat_arg.span, ctxt, "..", &mut app).0
),
app,
);
}
}
}
fn check_iter(
cx: &LateContext<'tcx>,
ctxt: SyntaxContext,
mut iter: impl Iterator<Item = (HirId, Node<'tcx>)>,
count: u128,
) -> bool {
match iter.next() {
Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => {
let (name, args) = if let ExprKind::MethodCall(name, _, [_, args @ ..], _) = e.kind {
(name, args)
} else {
return false;
};
if_chain! {
if let Some(did) = cx.typeck_results().type_dependent_def_id(e.hir_id);
if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
then {
match (&*name.ident.as_str(), args) {
("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => {
return true;
},
("next_tuple", []) if count > 2 => {
return true;
},
("nth", [idx_expr]) if cx.tcx.trait_of_item(did) == Some(iter_id) => {
if let Some((Constant::Int(idx), _)) = constant(cx, cx.typeck_results(), idx_expr) {
if count > idx + 1 {
return true;
}
}
},
_ => return false,
}
}
}
},
_ => return false,
};
false
}

6
tests/ui/manual_split_once.fixed
View file

@ -2,7 +2,7 @@
#![feature(custom_inner_attributes)]
#![warn(clippy::manual_split_once)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::needless_splitn)]
extern crate itertools;
@ -38,8 +38,8 @@ fn main() {
let _ = [0, 1, 2].splitn(2, |&x| x == 1).nth(1).unwrap();
// `rsplitn` gives the results in the reverse order of `rsplit_once`
let _ = "key=value".rsplit_once('=').unwrap().1;
let _ = "key=value".rsplit_once('=').map_or("key=value", |x| x.0);
let _ = "key=value".rsplitn(2, '=').next().unwrap();
let _ = "key=value".rsplit_once('=').unwrap().0;
let _ = "key=value".rsplit_once('=').map(|x| x.1);
let (_, _) = "key=value".rsplit_once('=').map(|(x, y)| (y, x)).unwrap();
}

2
tests/ui/manual_split_once.rs
View file

@ -2,7 +2,7 @@
#![feature(custom_inner_attributes)]
#![warn(clippy::manual_split_once)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::needless_splitn)]
extern crate itertools;

10
tests/ui/manual_split_once.stderr
View file

@ -72,17 +72,11 @@ error: manual implementation of `split_once`
LL | let _ = s.splitn(2, "key=value").skip(1).next()?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once("key=value")?.1`
error: manual implementation of `rsplit_once`
--> $DIR/manual_split_once.rs:41:13
|
LL | let _ = "key=value".rsplitn(2, '=').next().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').unwrap().1`
error: manual implementation of `rsplit_once`
--> $DIR/manual_split_once.rs:42:13
|
LL | let _ = "key=value".rsplitn(2, '=').nth(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map_or("key=value", |x| x.0)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').unwrap().0`
error: manual implementation of `rsplit_once`
--> $DIR/manual_split_once.rs:43:13
@ -102,5 +96,5 @@ error: manual implementation of `split_once`
LL | let _ = "key=value".splitn(2, '=').nth(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1`
error: aborting due to 17 previous errors
error: aborting due to 16 previous errors

27
tests/ui/needless_splitn.fixed Normal file
View file

@ -0,0 +1,27 @@
// run-rustfix
// edition:2018
#![feature(custom_inner_attributes)]
#![warn(clippy::needless_splitn)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::manual_split_once)]
extern crate itertools;
#[allow(unused_imports)]
use itertools::Itertools;
fn main() {
let str = "key=value=end";
let _ = str.split('=').next();
let _ = str.split('=').nth(0);
let _ = str.splitn(2, '=').nth(1);
let (_, _) = str.splitn(2, '=').next_tuple().unwrap();
let (_, _) = str.split('=').next_tuple().unwrap();
let _: Vec<&str> = str.splitn(3, '=').collect();
let _ = str.rsplit('=').next();
let _ = str.rsplit('=').nth(0);
let _ = str.rsplitn(2, '=').nth(1);
let (_, _) = str.rsplitn(2, '=').next_tuple().unwrap();
let (_, _) = str.rsplit('=').next_tuple().unwrap();
}

27
tests/ui/needless_splitn.rs Normal file
View file

@ -0,0 +1,27 @@
// run-rustfix
// edition:2018
#![feature(custom_inner_attributes)]
#![warn(clippy::needless_splitn)]
#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::manual_split_once)]
extern crate itertools;
#[allow(unused_imports)]
use itertools::Itertools;
fn main() {
let str = "key=value=end";
let _ = str.splitn(2, '=').next();
let _ = str.splitn(2, '=').nth(0);
let _ = str.splitn(2, '=').nth(1);
let (_, _) = str.splitn(2, '=').next_tuple().unwrap();
let (_, _) = str.splitn(3, '=').next_tuple().unwrap();
let _: Vec<&str> = str.splitn(3, '=').collect();
let _ = str.rsplitn(2, '=').next();
let _ = str.rsplitn(2, '=').nth(0);
let _ = str.rsplitn(2, '=').nth(1);
let (_, _) = str.rsplitn(2, '=').next_tuple().unwrap();
let (_, _) = str.rsplitn(3, '=').next_tuple().unwrap();
}

40
tests/ui/needless_splitn.stderr Normal file
View file

@ -0,0 +1,40 @@
error: unnecessary use of `splitn`
--> $DIR/needless_splitn.rs:15:13
|
LL | let _ = str.splitn(2, '=').next();
| ^^^^^^^^^^^^^^^^^^ help: try this: `str.split('=')`
|
= note: `-D clippy::needless-splitn` implied by `-D warnings`
error: unnecessary use of `splitn`
--> $DIR/needless_splitn.rs:16:13
|
LL | let _ = str.splitn(2, '=').nth(0);
| ^^^^^^^^^^^^^^^^^^ help: try this: `str.split('=')`
error: unnecessary use of `splitn`
--> $DIR/needless_splitn.rs:19:18
|
LL | let (_, _) = str.splitn(3, '=').next_tuple().unwrap();
| ^^^^^^^^^^^^^^^^^^ help: try this: `str.split('=')`
error: unnecessary use of `rsplitn`
--> $DIR/needless_splitn.rs:22:13
|
LL | let _ = str.rsplitn(2, '=').next();
| ^^^^^^^^^^^^^^^^^^^ help: try this: `str.rsplit('=')`
error: unnecessary use of `rsplitn`
--> $DIR/needless_splitn.rs:23:13
|
LL | let _ = str.rsplitn(2, '=').nth(0);
| ^^^^^^^^^^^^^^^^^^^ help: try this: `str.rsplit('=')`
error: unnecessary use of `rsplitn`
--> $DIR/needless_splitn.rs:26:18
|
LL | let (_, _) = str.rsplitn(3, '=').next_tuple().unwrap();
| ^^^^^^^^^^^^^^^^^^^ help: try this: `str.rsplit('=')`
error: aborting due to 6 previous errors

1
tests/ui/suspicious_splitn.rs
View file

@ -1,4 +1,5 @@
#![warn(clippy::suspicious_splitn)]
#![allow(clippy::needless_splitn)]
fn main() {
let _ = "a,b,c".splitn(3, ',');

18
tests/ui/suspicious_splitn.stderr
View file

@ -1,5 +1,5 @@
error: `splitn` called with `0` splits
--> $DIR/suspicious_splitn.rs:9:13
--> $DIR/suspicious_splitn.rs:10:13
|
LL | let _ = "a,b".splitn(0, ',');
| ^^^^^^^^^^^^^^^^^^^^
@ -8,7 +8,7 @@ LL | let _ = "a,b".splitn(0, ',');
= note: the resulting iterator will always return `None`
error: `rsplitn` called with `0` splits
--> $DIR/suspicious_splitn.rs:10:13
--> $DIR/suspicious_splitn.rs:11:13
|
LL | let _ = "a,b".rsplitn(0, ',');
| ^^^^^^^^^^^^^^^^^^^^^
@ -16,7 +16,7 @@ LL | let _ = "a,b".rsplitn(0, ',');
= note: the resulting iterator will always return `None`
error: `splitn` called with `1` split
--> $DIR/suspicious_splitn.rs:11:13
--> $DIR/suspicious_splitn.rs:12:13
|
LL | let _ = "a,b".splitn(1, ',');
| ^^^^^^^^^^^^^^^^^^^^
@ -24,7 +24,7 @@ LL | let _ = "a,b".splitn(1, ',');
= note: the resulting iterator will always return the entire string followed by `None`
error: `splitn` called with `0` splits
--> $DIR/suspicious_splitn.rs:12:13
--> $DIR/suspicious_splitn.rs:13:13
|
LL | let _ = [0, 1, 2].splitn(0, |&x| x == 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -32,7 +32,7 @@ LL | let _ = [0, 1, 2].splitn(0, |&x| x == 1);
= note: the resulting iterator will always return `None`
error: `splitn_mut` called with `0` splits
--> $DIR/suspicious_splitn.rs:13:13
--> $DIR/suspicious_splitn.rs:14:13
|
LL | let _ = [0, 1, 2].splitn_mut(0, |&x| x == 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -40,7 +40,7 @@ LL | let _ = [0, 1, 2].splitn_mut(0, |&x| x == 1);
= note: the resulting iterator will always return `None`
error: `splitn` called with `1` split
--> $DIR/suspicious_splitn.rs:14:13
--> $DIR/suspicious_splitn.rs:15:13
|
LL | let _ = [0, 1, 2].splitn(1, |&x| x == 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -48,7 +48,7 @@ LL | let _ = [0, 1, 2].splitn(1, |&x| x == 1);
= note: the resulting iterator will always return the entire slice followed by `None`
error: `rsplitn_mut` called with `1` split
--> $DIR/suspicious_splitn.rs:15:13
--> $DIR/suspicious_splitn.rs:16:13
|
LL | let _ = [0, 1, 2].rsplitn_mut(1, |&x| x == 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -56,7 +56,7 @@ LL | let _ = [0, 1, 2].rsplitn_mut(1, |&x| x == 1);
= note: the resulting iterator will always return the entire slice followed by `None`
error: `splitn` called with `1` split
--> $DIR/suspicious_splitn.rs:18:13
--> $DIR/suspicious_splitn.rs:19:13
|
LL | let _ = "a,b".splitn(X + 1, ',');
| ^^^^^^^^^^^^^^^^^^^^^^^^
@ -64,7 +64,7 @@ LL | let _ = "a,b".splitn(X + 1, ',');
= note: the resulting iterator will always return the entire string followed by `None`
error: `splitn` called with `0` splits
--> $DIR/suspicious_splitn.rs:19:13
--> $DIR/suspicious_splitn.rs:20:13
|
LL | let _ = "a,b".splitn(X, ',');
| ^^^^^^^^^^^^^^^^^^^^