Skip to content

Commit 1fa6ea2

Browse files
Add needless_character_iteration lint
1 parent caad063 commit 1fa6ea2

9 files changed

+217
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5567,6 +5567,7 @@ Released 2018-09-13
55675567
[`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
55685568
[`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
55695569
[`needless_borrows_for_generic_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
5570+
[`needless_character_iteration`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_character_iteration
55705571
[`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
55715572
[`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
55725573
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
418418
crate::methods::MAP_UNWRAP_OR_INFO,
419419
crate::methods::MUT_MUTEX_LOCK_INFO,
420420
crate::methods::NAIVE_BYTECOUNT_INFO,
421+
crate::methods::NEEDLESS_CHARACTER_ITERATION_INFO,
421422
crate::methods::NEEDLESS_COLLECT_INFO,
422423
crate::methods::NEEDLESS_OPTION_AS_DEREF_INFO,
423424
crate::methods::NEEDLESS_OPTION_TAKE_INFO,

clippy_lints/src/methods/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ mod map_flatten;
6666
mod map_identity;
6767
mod map_unwrap_or;
6868
mod mut_mutex_lock;
69+
mod needless_character_iteration;
6970
mod needless_collect;
7071
mod needless_option_as_deref;
7172
mod needless_option_take;
@@ -4089,6 +4090,27 @@ declare_clippy_lint! {
40894090
"is_empty() called on strings known at compile time"
40904091
}
40914092

4093+
declare_clippy_lint! {
4094+
/// ### What it does
4095+
/// Checks if an iterator is used to check if a string is ascii.
4096+
///
4097+
/// ### Why is this bad?
4098+
/// The `str` type already implements the `is_ascii` method.
4099+
///
4100+
/// ### Example
4101+
/// ```no_run
4102+
/// "foo".chars().all(|c| c.is_ascii());
4103+
/// ```
4104+
/// Use instead:
4105+
/// ```no_run
4106+
/// "foo".is_ascii();
4107+
/// ```
4108+
#[clippy::version = "1.80.0"]
4109+
pub NEEDLESS_CHARACTER_ITERATION,
4110+
suspicious,
4111+
"is_ascii() called on a char iterator"
4112+
}
4113+
40924114
pub struct Methods {
40934115
avoid_breaking_exported_api: bool,
40944116
msrv: Msrv,
@@ -4254,6 +4276,7 @@ impl_lint_pass!(Methods => [
42544276
UNNECESSARY_RESULT_MAP_OR_ELSE,
42554277
MANUAL_C_STR_LITERALS,
42564278
UNNECESSARY_GET_THEN_CHECK,
4279+
NEEDLESS_CHARACTER_ITERATION,
42574280
]);
42584281

42594282
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4461,6 +4484,7 @@ impl Methods {
44614484
},
44624485
("all", [arg]) => {
44634486
unused_enumerate_index::check(cx, expr, recv, arg);
4487+
needless_character_iteration::check(cx, expr, recv, arg);
44644488
if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
44654489
iter_overeager_cloned::check(
44664490
cx,
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir::{Closure, Expr, ExprKind, HirId, UnOp};
3+
use rustc_lint::LateContext;
4+
use rustc_middle::ty;
5+
use rustc_span::Span;
6+
7+
use super::utils::get_last_chain_binding_hir_id;
8+
use super::NEEDLESS_CHARACTER_ITERATION;
9+
use clippy_utils::diagnostics::span_lint_and_sugg;
10+
use clippy_utils::source::snippet_opt;
11+
use clippy_utils::{path_to_local_id, peel_blocks};
12+
13+
fn handle_expr(
14+
cx: &LateContext<'_>,
15+
expr: &Expr<'_>,
16+
first_param: HirId,
17+
span: Span,
18+
before_chars: Span,
19+
revert: bool,
20+
) {
21+
match expr.kind {
22+
ExprKind::MethodCall(method, receiver, [], _) => {
23+
if method.ident.name.as_str() == "is_ascii"
24+
&& path_to_local_id(receiver, first_param)
25+
&& let char_arg_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs()
26+
&& *char_arg_ty.kind() == ty::Char
27+
&& let Some(snippet) = snippet_opt(cx, before_chars)
28+
{
29+
span_lint_and_sugg(
30+
cx,
31+
NEEDLESS_CHARACTER_ITERATION,
32+
span,
33+
"checking if a string is ascii using iterators",
34+
"try",
35+
format!("{}{snippet}.is_ascii()", if revert { "!" } else { "" }),
36+
Applicability::MachineApplicable,
37+
);
38+
}
39+
},
40+
ExprKind::Block(block, _) => {
41+
if let Some(block_expr) = block.expr
42+
// First we ensure that this is a "binding chain" (each statement is a binding
43+
// of the previous one) and that it is a binding of the closure argument.
44+
&& let Some(last_chain_binding_id) =
45+
get_last_chain_binding_hir_id(first_param, block.stmts)
46+
{
47+
handle_expr(cx, &block_expr, last_chain_binding_id, span, before_chars, revert);
48+
}
49+
},
50+
ExprKind::Unary(UnOp::Not, expr) => handle_expr(cx, expr, first_param, span, before_chars, !revert),
51+
_ => {},
52+
}
53+
}
54+
55+
pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
56+
if let ExprKind::Closure(&Closure { body, .. }) = closure_arg.kind
57+
&& let body = cx.tcx.hir().body(body)
58+
&& let Some(first_param) = body.params.first()
59+
&& let ExprKind::MethodCall(method, mut recv, [], _) = recv.kind
60+
&& method.ident.name.as_str() == "chars"
61+
&& let str_ty = cx.typeck_results().expr_ty_adjusted(recv).peel_refs()
62+
&& *str_ty.kind() == ty::Str
63+
{
64+
let expr_start = recv.span;
65+
while let ExprKind::MethodCall(_, new_recv, _, _) = recv.kind {
66+
recv = new_recv;
67+
}
68+
let body_expr = peel_blocks(body.value);
69+
70+
handle_expr(
71+
cx,
72+
&body_expr,
73+
first_param.pat.hir_id,
74+
recv.span.with_hi(call_expr.span.hi()),
75+
recv.span.with_hi(expr_start.hi()),
76+
false,
77+
);
78+
}
79+
}

clippy_lints/src/methods/unnecessary_result_map_or_else.rs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ use clippy_utils::source::snippet;
44
use clippy_utils::ty::is_type_diagnostic_item;
55
use rustc_errors::Applicability;
66
use rustc_hir as hir;
7-
use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath, Stmt, StmtKind};
7+
use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath};
88
use rustc_lint::LateContext;
99
use rustc_span::symbol::sym;
1010

11+
use super::utils::get_last_chain_binding_hir_id;
1112
use super::UNNECESSARY_RESULT_MAP_OR_ELSE;
1213

1314
fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>) {
@@ -25,22 +26,6 @@ fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &E
2526
);
2627
}
2728

28-
fn get_last_chain_binding_hir_id(mut hir_id: HirId, statements: &[Stmt<'_>]) -> Option<HirId> {
29-
for stmt in statements {
30-
if let StmtKind::Let(local) = stmt.kind
31-
&& let Some(init) = local.init
32-
&& let ExprKind::Path(QPath::Resolved(_, path)) = init.kind
33-
&& let hir::def::Res::Local(local_hir_id) = path.res
34-
&& local_hir_id == hir_id
35-
{
36-
hir_id = local.pat.hir_id;
37-
} else {
38-
return None;
39-
}
40-
}
41-
Some(hir_id)
42-
}
43-
4429
fn handle_qpath(
4530
cx: &LateContext<'_>,
4631
expr: &Expr<'_>,

clippy_lints/src/methods/utils.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::{get_parent_expr, path_to_local_id, usage};
44
use rustc_ast::ast;
55
use rustc_errors::Applicability;
66
use rustc_hir::intravisit::{walk_expr, Visitor};
7-
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat};
7+
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, QPath, Stmt, StmtKind};
88
use rustc_lint::LateContext;
99
use rustc_middle::hir::nested_filter;
1010
use rustc_middle::ty::{self, Ty};
@@ -171,3 +171,19 @@ impl<'cx, 'tcx> CloneOrCopyVisitor<'cx, 'tcx> {
171171
.any(|hir_id| path_to_local_id(expr, *hir_id))
172172
}
173173
}
174+
175+
pub(super) fn get_last_chain_binding_hir_id(mut hir_id: HirId, statements: &[Stmt<'_>]) -> Option<HirId> {
176+
for stmt in statements {
177+
if let StmtKind::Let(local) = stmt.kind
178+
&& let Some(init) = local.init
179+
&& let ExprKind::Path(QPath::Resolved(_, path)) = init.kind
180+
&& let rustc_hir::def::Res::Local(local_hir_id) = path.res
181+
&& local_hir_id == hir_id
182+
{
183+
hir_id = local.pat.hir_id;
184+
} else {
185+
return None;
186+
}
187+
}
188+
Some(hir_id)
189+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![warn(clippy::needless_character_iteration)]
2+
#![allow(clippy::unnecessary_operation)]
3+
4+
fn main() {
5+
"foo".is_ascii();
6+
//~^ ERROR: checking if a string is ascii using iterators
7+
!"foo".is_ascii();
8+
//~^ ERROR: checking if a string is ascii using iterators
9+
10+
let s = String::new();
11+
s.is_ascii();
12+
//~^ ERROR: checking if a string is ascii using iterators
13+
!"foo".to_string().is_ascii();
14+
//~^ ERROR: checking if a string is ascii using iterators
15+
16+
"foo".is_ascii();
17+
!"foo".is_ascii();
18+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#![warn(clippy::needless_character_iteration)]
2+
#![allow(clippy::unnecessary_operation)]
3+
4+
fn main() {
5+
"foo".chars().all(|c| c.is_ascii());
6+
//~^ ERROR: checking if a string is ascii using iterators
7+
"foo".chars().all(|c| !c.is_ascii());
8+
//~^ ERROR: checking if a string is ascii using iterators
9+
10+
let s = String::new();
11+
s.chars().all(|c| c.is_ascii());
12+
//~^ ERROR: checking if a string is ascii using iterators
13+
"foo".to_string().chars().all(|c| !c.is_ascii());
14+
//~^ ERROR: checking if a string is ascii using iterators
15+
16+
"foo".chars().all(|c| {
17+
//~^ ERROR: checking if a string is ascii using iterators
18+
let x = c;
19+
x.is_ascii()
20+
});
21+
"foo".chars().all(|c| {
22+
//~^ ERROR: checking if a string is ascii using iterators
23+
let x = c;
24+
!x.is_ascii()
25+
});
26+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
error: checking if a string is ascii using iterators
2+
--> tests/ui/needless_character_iteration.rs:5:5
3+
|
4+
LL | "foo".chars().all(|c| c.is_ascii());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"foo".is_ascii()`
6+
|
7+
= note: `-D clippy::needless-character-iteration` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::needless_character_iteration)]`
9+
10+
error: checking if a string is ascii using iterators
11+
--> tests/ui/needless_character_iteration.rs:7:5
12+
|
13+
LL | "foo".chars().all(|c| !c.is_ascii());
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!"foo".is_ascii()`
15+
16+
error: checking if a string is ascii using iterators
17+
--> tests/ui/needless_character_iteration.rs:11:5
18+
|
19+
LL | s.chars().all(|c| c.is_ascii());
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `s.is_ascii()`
21+
22+
error: checking if a string is ascii using iterators
23+
--> tests/ui/needless_character_iteration.rs:13:5
24+
|
25+
LL | "foo".to_string().chars().all(|c| !c.is_ascii());
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!"foo".to_string().is_ascii()`
27+
28+
error: checking if a string is ascii using iterators
29+
--> tests/ui/needless_character_iteration.rs:16:5
30+
|
31+
LL | / "foo".chars().all(|c| {
32+
LL | |
33+
LL | | let x = c;
34+
LL | | x.is_ascii()
35+
LL | | });
36+
| |______^ help: try: `"foo".is_ascii()`
37+
38+
error: checking if a string is ascii using iterators
39+
--> tests/ui/needless_character_iteration.rs:21:5
40+
|
41+
LL | / "foo".chars().all(|c| {
42+
LL | |
43+
LL | | let x = c;
44+
LL | | !x.is_ascii()
45+
LL | | });
46+
| |______^ help: try: `!"foo".is_ascii()`
47+
48+
error: aborting due to 6 previous errors
49+

0 commit comments

Comments
 (0)