Skip to content

Commit 10d1f32

Browse files
borsbors
committed
Auto merge of #12886 - GuillaumeGomez:fix-needless_character_iteration, r=blyxyas
Fix false positive for `needless_character_iteration` lint Fixes #12879. changelog: Fix false positive for `needless_character_iteration` lint
2 parents 46e3304 + 158b658 commit 10d1f32

5 files changed

+43
-14
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4485,7 +4485,7 @@ impl Methods {
44854485
},
44864486
("all", [arg]) => {
44874487
unused_enumerate_index::check(cx, expr, recv, arg);
4488-
needless_character_iteration::check(cx, expr, recv, arg);
4488+
needless_character_iteration::check(cx, expr, recv, arg, true);
44894489
if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
44904490
iter_overeager_cloned::check(
44914491
cx,
@@ -4506,6 +4506,7 @@ impl Methods {
45064506
},
45074507
("any", [arg]) => {
45084508
unused_enumerate_index::check(cx, expr, recv, arg);
4509+
needless_character_iteration::check(cx, expr, recv, arg, false);
45094510
match method_call(recv) {
45104511
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
45114512
cx,

‎clippy_lints/src/methods/needless_character_iteration.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,14 @@ fn handle_expr(
2424
span: Span,
2525
before_chars: Span,
2626
revert: bool,
27+
is_all: bool,
2728
) {
2829
match expr.kind {
2930
ExprKind::MethodCall(method, receiver, [], _) => {
30-
if method.ident.name.as_str() == "is_ascii"
31+
// If we have `!is_ascii`, then only `.any()` should warn. And if the condition is
32+
// `is_ascii`, then only `.all()` should warn.
33+
if revert != is_all
34+
&& method.ident.name.as_str() == "is_ascii"
3135
&& path_to_local_id(receiver, first_param)
3236
&& let char_arg_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs()
3337
&& *char_arg_ty.kind() == ty::Char
@@ -55,12 +59,23 @@ fn handle_expr(
5559
&& let Some(last_chain_binding_id) =
5660
get_last_chain_binding_hir_id(first_param, block.stmts)
5761
{
58-
handle_expr(cx, block_expr, last_chain_binding_id, span, before_chars, revert);
62+
handle_expr(
63+
cx,
64+
block_expr,
65+
last_chain_binding_id,
66+
span,
67+
before_chars,
68+
revert,
69+
is_all,
70+
);
5971
}
6072
},
61-
ExprKind::Unary(UnOp::Not, expr) => handle_expr(cx, expr, first_param, span, before_chars, !revert),
73+
ExprKind::Unary(UnOp::Not, expr) => handle_expr(cx, expr, first_param, span, before_chars, !revert, is_all),
6274
ExprKind::Call(fn_path, [arg]) => {
63-
if let ExprKind::Path(path) = fn_path.kind
75+
// If we have `!is_ascii`, then only `.any()` should warn. And if the condition is
76+
// `is_ascii`, then only `.all()` should warn.
77+
if revert != is_all
78+
&& let ExprKind::Path(path) = fn_path.kind
6479
&& let Some(fn_def_id) = cx.qpath_res(&path, fn_path.hir_id).opt_def_id()
6580
&& match_def_path(cx, fn_def_id, &["core", "char", "methods", "<impl char>", "is_ascii"])
6681
&& path_to_local_id(peels_expr_ref(arg), first_param)
@@ -81,7 +96,7 @@ fn handle_expr(
8196
}
8297
}
8398

84-
pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
99+
pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>, is_all: bool) {
85100
if let ExprKind::Closure(&Closure { body, .. }) = closure_arg.kind
86101
&& let body = cx.tcx.hir().body(body)
87102
&& let Some(first_param) = body.params.first()
@@ -103,6 +118,7 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>,
103118
recv.span.with_hi(call_expr.span.hi()),
104119
recv.span.with_hi(expr_start.hi()),
105120
false,
121+
is_all,
106122
);
107123
}
108124
}

‎tests/ui/needless_character_iteration.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,10 @@ fn main() {
4848

4949
// Should not lint!
5050
"foo".chars().map(|c| c).all(|c| !char::is_ascii(&c));
51+
52+
// Should not lint!
53+
"foo".chars().all(|c| !c.is_ascii());
54+
55+
// Should not lint!
56+
"foo".chars().any(|c| c.is_ascii());
5157
}

‎tests/ui/needless_character_iteration.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,25 @@ fn magic(_: char) {}
1717
fn main() {
1818
"foo".chars().all(|c| c.is_ascii());
1919
//~^ ERROR: checking if a string is ascii using iterators
20-
"foo".chars().all(|c| !c.is_ascii());
20+
"foo".chars().any(|c| !c.is_ascii());
2121
//~^ ERROR: checking if a string is ascii using iterators
2222
"foo".chars().all(|c| char::is_ascii(&c));
2323
//~^ ERROR: checking if a string is ascii using iterators
24-
"foo".chars().all(|c| !char::is_ascii(&c));
24+
"foo".chars().any(|c| !char::is_ascii(&c));
2525
//~^ ERROR: checking if a string is ascii using iterators
2626

2727
let s = String::new();
2828
s.chars().all(|c| c.is_ascii());
2929
//~^ ERROR: checking if a string is ascii using iterators
30-
"foo".to_string().chars().all(|c| !c.is_ascii());
30+
"foo".to_string().chars().any(|c| !c.is_ascii());
3131
//~^ ERROR: checking if a string is ascii using iterators
3232

3333
"foo".chars().all(|c| {
3434
//~^ ERROR: checking if a string is ascii using iterators
3535
let x = c;
3636
x.is_ascii()
3737
});
38-
"foo".chars().all(|c| {
38+
"foo".chars().any(|c| {
3939
//~^ ERROR: checking if a string is ascii using iterators
4040
let x = c;
4141
!x.is_ascii()
@@ -56,4 +56,10 @@ fn main() {
5656

5757
// Should not lint!
5858
"foo".chars().map(|c| c).all(|c| !char::is_ascii(&c));
59+
60+
// Should not lint!
61+
"foo".chars().all(|c| !c.is_ascii());
62+
63+
// Should not lint!
64+
"foo".chars().any(|c| c.is_ascii());
5965
}

‎tests/ui/needless_character_iteration.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ LL | "foo".chars().all(|c| c.is_ascii());
1010
error: checking if a string is ascii using iterators
1111
--> tests/ui/needless_character_iteration.rs:20:5
1212
|
13-
LL | "foo".chars().all(|c| !c.is_ascii());
13+
LL | "foo".chars().any(|c| !c.is_ascii());
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!"foo".is_ascii()`
1515

1616
error: checking if a string is ascii using iterators
@@ -22,7 +22,7 @@ LL | "foo".chars().all(|c| char::is_ascii(&c));
2222
error: checking if a string is ascii using iterators
2323
--> tests/ui/needless_character_iteration.rs:24:5
2424
|
25-
LL | "foo".chars().all(|c| !char::is_ascii(&c));
25+
LL | "foo".chars().any(|c| !char::is_ascii(&c));
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!"foo".is_ascii()`
2727

2828
error: checking if a string is ascii using iterators
@@ -34,7 +34,7 @@ LL | s.chars().all(|c| c.is_ascii());
3434
error: checking if a string is ascii using iterators
3535
--> tests/ui/needless_character_iteration.rs:30:5
3636
|
37-
LL | "foo".to_string().chars().all(|c| !c.is_ascii());
37+
LL | "foo".to_string().chars().any(|c| !c.is_ascii());
3838
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!"foo".to_string().is_ascii()`
3939

4040
error: checking if a string is ascii using iterators
@@ -50,7 +50,7 @@ LL | | });
5050
error: checking if a string is ascii using iterators
5151
--> tests/ui/needless_character_iteration.rs:38:5
5252
|
53-
LL | / "foo".chars().all(|c| {
53+
LL | / "foo".chars().any(|c| {
5454
LL | |
5555
LL | | let x = c;
5656
LL | | !x.is_ascii()

0 commit comments

Comments
 (0)