mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

deprecate clippy::for_loops_over_fallibles

Browse Source
This commit is contained in:
Maybe Waffle 2022-10-07 17:08:29 +00:00
parent 40f36fac49
commit 5347c81924
16 changed files with 34 additions and 351 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/tools/clippy/clippy_lints/src/lib.register_all.rs
View File

@ -109,7 +109,6 @@
LintId::of(loops::EMPTY_LOOP),
LintId::of(loops::EXPLICIT_COUNTER_LOOP),
LintId::of(loops::FOR_KV_MAP),
LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
LintId::of(loops::ITER_NEXT_LOOP),
LintId::of(loops::MANUAL_FIND),
LintId::of(loops::MANUAL_FLATTEN),

1
src/tools/clippy/clippy_lints/src/lib.register_lints.rs
View File

@ -227,7 +227,6 @@
loops::EXPLICIT_INTO_ITER_LOOP,
loops::EXPLICIT_ITER_LOOP,
loops::FOR_KV_MAP,
loops::FOR_LOOPS_OVER_FALLIBLES,
loops::ITER_NEXT_LOOP,
loops::MANUAL_FIND,
loops::MANUAL_FLATTEN,

1
src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs
View File

@ -21,7 +21,6 @@
LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING),
LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
LintId::of(loops::EMPTY_LOOP),
LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
LintId::of(loops::MUT_RANGE_BOUND),
LintId::of(methods::NO_EFFECT_REPLACE),
LintId::of(methods::SUSPICIOUS_MAP),

65
src/tools/clippy/clippy_lints/src/loops/for_loops_over_fallibles.rs
View File

@ -1,65 +0,0 @@
use super::FOR_LOOPS_OVER_FALLIBLES;
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir::{Expr, Pat};
use rustc_lint::LateContext;
use rustc_span::symbol::sym;
/// Checks for `for` loops over `Option`s and `Result`s.
pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, method_name: Option<&str>) {
let ty = cx.typeck_results().expr_ty(arg);
if is_type_diagnostic_item(cx, ty, sym::Option) {
let help_string = if let Some(method_name) = method_name {
format!(
"consider replacing `for {0} in {1}.{method_name}()` with `if let Some({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
)
} else {
format!(
"consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
)
};
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is an `Option`. This is more readably written as an \
`if let` statement",
snippet(cx, arg.span, "_")
),
None,
&help_string,
);
} else if is_type_diagnostic_item(cx, ty, sym::Result) {
let help_string = if let Some(method_name) = method_name {
format!(
"consider replacing `for {0} in {1}.{method_name}()` with `if let Ok({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
)
} else {
format!(
"consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
)
};
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is a `Result`. This is more readably written as an \
`if let` statement",
snippet(cx, arg.span, "_")
),
None,
&help_string,
);
}
}

5
src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs
View File

@ -5,7 +5,7 @@
use rustc_lint::LateContext;
use rustc_span::sym;
pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) {
if is_trait_method(cx, arg, sym::Iterator) {
span_lint(
cx,
@ -14,8 +14,5 @@ pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
"you are iterating over `Iterator::next()` which is an Option; this will compile but is \
probably not what you want",
);
true
} else {
false
}
}

57
src/tools/clippy/clippy_lints/src/loops/mod.rs
View File

@ -3,7 +3,6 @@
mod explicit_into_iter_loop;
mod explicit_iter_loop;
mod for_kv_map;
mod for_loops_over_fallibles;
mod iter_next_loop;
mod manual_find;
mod manual_flatten;
@ -173,49 +172,6 @@
"for-looping over `_.next()` which is probably not intended"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `for` loops over `Option` or `Result` values.
///
/// ### Why is this bad?
/// Readability. This is more clearly expressed as an `if
/// let`.
///
/// ### Example
/// ```rust
/// # let opt = Some(1);
/// # let res: Result<i32, std::io::Error> = Ok(1);
/// for x in opt {
/// // ..
/// }
///
/// for x in &res {
/// // ..
/// }
///
/// for x in res.iter() {
/// // ..
/// }
/// ```
///
/// Use instead:
/// ```rust
/// # let opt = Some(1);
/// # let res: Result<i32, std::io::Error> = Ok(1);
/// if let Some(x) = opt {
/// // ..
/// }
///
/// if let Ok(x) = res {
/// // ..
/// }
/// ```
#[clippy::version = "1.45.0"]
pub FOR_LOOPS_OVER_FALLIBLES,
suspicious,
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
}
declare_clippy_lint! {
/// ### What it does
/// Detects `loop + match` combinations that are easier
@ -648,7 +604,6 @@
EXPLICIT_ITER_LOOP,
EXPLICIT_INTO_ITER_LOOP,
ITER_NEXT_LOOP,
FOR_LOOPS_OVER_FALLIBLES,
WHILE_LET_LOOP,
NEEDLESS_COLLECT,
EXPLICIT_COUNTER_LOOP,
@ -739,30 +694,22 @@ fn check_for_loop<'tcx>(
manual_find::check(cx, pat, arg, body, span, expr);
}
fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
fn check_for_loop_arg(cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
let method_name = method.ident.as_str();
// check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
match method_name {
"iter" | "iter_mut" => {
explicit_iter_loop::check(cx, self_arg, arg, method_name);
for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
},
"into_iter" => {
explicit_iter_loop::check(cx, self_arg, arg, method_name);
explicit_into_iter_loop::check(cx, self_arg, arg);
for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
},
"next" => {
next_loop_linted = iter_next_loop::check(cx, arg);
iter_next_loop::check(cx, arg);
},
_ => {},
}
}
if !next_loop_linted {
for_loops_over_fallibles::check(cx, pat, arg, None);
}
}

5
src/tools/clippy/clippy_lints/src/renamed_lints.rs
View File

@ -11,8 +11,8 @@
("clippy::disallowed_method", "clippy::disallowed_methods"),
("clippy::disallowed_type", "clippy::disallowed_types"),
("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"),
("clippy::for_loop_over_option", "clippy::for_loops_over_fallibles"),
("clippy::for_loop_over_result", "clippy::for_loops_over_fallibles"),
("clippy::for_loop_over_option", "for_loops_over_fallibles"),
("clippy::for_loop_over_result", "for_loops_over_fallibles"),
("clippy::identity_conversion", "clippy::useless_conversion"),
("clippy::if_let_some_result", "clippy::match_result_ok"),
("clippy::logic_bug", "clippy::overly_complex_bool_expr"),
@ -31,6 +31,7 @@
("clippy::to_string_in_display", "clippy::recursive_format_impl"),
("clippy::zero_width_space", "clippy::invisible_characters"),
("clippy::drop_bounds", "drop_bounds"),
("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"),
("clippy::into_iter_on_array", "array_into_iter"),
("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"),
("clippy::invalid_ref", "invalid_value"),

1
src/tools/clippy/src/docs.rs
View File

@ -170,7 +170,6 @@ pub fn explain(lint: &str) {
"fn_to_numeric_cast_any",
"fn_to_numeric_cast_with_truncation",
"for_kv_map",
"for_loops_over_fallibles",
"forget_copy",
"forget_non_drop",
"forget_ref",

32
src/tools/clippy/src/docs/for_loops_over_fallibles.txt
View File

@ -1,32 +0,0 @@
### What it does
Checks for `for` loops over `Option` or `Result` values.
### Why is this bad?
Readability. This is more clearly expressed as an `if
let`.
### Example
```
for x in opt {
// ..
}
for x in &res {
// ..
}
for x in res.iter() {
// ..
}
```
Use instead:
```
if let Some(x) = opt {
// ..
}
if let Ok(x) = res {
// ..
}
```

74
src/tools/clippy/tests/ui/for_loops_over_fallibles.rs
View File

@ -1,74 +0,0 @@
#![warn(clippy::for_loops_over_fallibles)]
#![allow(clippy::uninlined_format_args)]
#![allow(for_loops_over_fallibles)]
fn for_loops_over_fallibles() {
let option = Some(1);
let mut result = option.ok_or("x not found");
let v = vec![0, 1, 2];
// check over an `Option`
for x in option {
println!("{}", x);
}
// check over an `Option`
for x in option.iter() {
println!("{}", x);
}
// check over a `Result`
for x in result {
println!("{}", x);
}
// check over a `Result`
for x in result.iter_mut() {
println!("{}", x);
}
// check over a `Result`
for x in result.into_iter() {
println!("{}", x);
}
for x in option.ok_or("x not found") {
println!("{}", x);
}
// make sure LOOP_OVER_NEXT lint takes clippy::precedence when next() is the last call
// in the chain
for x in v.iter().next() {
println!("{}", x);
}
// make sure we lint when next() is not the last call in the chain
for x in v.iter().next().and(Some(0)) {
println!("{}", x);
}
for x in v.iter().next().ok_or("x not found") {
println!("{}", x);
}
// check for false positives
// for loop false positive
for x in v {
println!("{}", x);
}
// while let false positive for Option
while let Some(x) = option {
println!("{}", x);
break;
}
// while let false positive for Result
while let Ok(x) = result {
println!("{}", x);
break;
}
}
fn main() {}

95
src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr
View File

@ -1,95 +0,0 @@
error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:10:14
|
LL | for x in option {
| ^^^^^^
|
= help: consider replacing `for x in option` with `if let Some(x) = option`
= note: `-D clippy::for-loops-over-fallibles` implied by `-D warnings`
error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:15:14
|
LL | for x in option.iter() {
| ^^^^^^
|
= help: consider replacing `for x in option.iter()` with `if let Some(x) = option`
error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:20:14
|
LL | for x in result {
| ^^^^^^
|
= help: consider replacing `for x in result` with `if let Ok(x) = result`
error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:25:14
|
LL | for x in result.iter_mut() {
| ^^^^^^
|
= help: consider replacing `for x in result.iter_mut()` with `if let Ok(x) = result`
error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:30:14
|
LL | for x in result.into_iter() {
| ^^^^^^
|
= help: consider replacing `for x in result.into_iter()` with `if let Ok(x) = result`
error: for loop over `option.ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:34:14
|
LL | for x in option.ok_or("x not found") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider replacing `for x in option.ok_or("x not found")` with `if let Ok(x) = option.ok_or("x not found")`
error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
--> $DIR/for_loops_over_fallibles.rs:40:14
|
LL | for x in v.iter().next() {
| ^^^^^^^^^^^^^^^
|
= note: `#[deny(clippy::iter_next_loop)]` on by default
error: for loop over `v.iter().next().and(Some(0))`, which is an `Option`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:45:14
|
LL | for x in v.iter().next().and(Some(0)) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider replacing `for x in v.iter().next().and(Some(0))` with `if let Some(x) = v.iter().next().and(Some(0))`
error: for loop over `v.iter().next().ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement
--> $DIR/for_loops_over_fallibles.rs:49:14
|
LL | for x in v.iter().next().ok_or("x not found") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider replacing `for x in v.iter().next().ok_or("x not found")` with `if let Ok(x) = v.iter().next().ok_or("x not found")`
error: this loop never actually loops
--> $DIR/for_loops_over_fallibles.rs:61:5
|
LL | / while let Some(x) = option {
LL | | println!("{}", x);
LL | | break;
LL | | }
| |_____^
|
= note: `#[deny(clippy::never_loop)]` on by default
error: this loop never actually loops
--> $DIR/for_loops_over_fallibles.rs:67:5
|
LL | / while let Ok(x) = result {
LL | | println!("{}", x);
LL | | break;
LL | | }
| |_____^
error: aborting due to 11 previous errors

2
src/tools/clippy/tests/ui/manual_map_option.fixed
View File

@ -7,7 +7,7 @@
clippy::unit_arg,
clippy::match_ref_pats,
clippy::redundant_pattern_matching,
clippy::for_loops_over_fallibles,
for_loops_over_fallibles,
dead_code
)]

2
src/tools/clippy/tests/ui/manual_map_option.rs
View File

@ -7,7 +7,7 @@
clippy::unit_arg,
clippy::match_ref_pats,
clippy::redundant_pattern_matching,
clippy::for_loops_over_fallibles,
for_loops_over_fallibles,
dead_code
)]

7
src/tools/clippy/tests/ui/rename.fixed
View File

@ -12,7 +12,7 @@
#![allow(clippy::disallowed_methods)]
#![allow(clippy::disallowed_types)]
#![allow(clippy::mixed_read_write_in_expression)]
#![allow(clippy::for_loops_over_fallibles)]
#![allow(for_loops_over_fallibles)]
#![allow(clippy::useless_conversion)]
#![allow(clippy::match_result_ok)]
#![allow(clippy::overly_complex_bool_expr)]
@ -45,8 +45,8 @@
#![warn(clippy::disallowed_methods)]
#![warn(clippy::disallowed_types)]
#![warn(clippy::mixed_read_write_in_expression)]
#![warn(clippy::for_loops_over_fallibles)]
#![warn(clippy::for_loops_over_fallibles)]
#![warn(for_loops_over_fallibles)]
#![warn(for_loops_over_fallibles)]
#![warn(clippy::useless_conversion)]
#![warn(clippy::match_result_ok)]
#![warn(clippy::overly_complex_bool_expr)]
@ -65,6 +65,7 @@
#![warn(clippy::recursive_format_impl)]
#![warn(clippy::invisible_characters)]
#![warn(drop_bounds)]
#![warn(for_loops_over_fallibles)]
#![warn(array_into_iter)]
#![warn(invalid_atomic_ordering)]
#![warn(invalid_value)]

3
src/tools/clippy/tests/ui/rename.rs
View File

@ -12,7 +12,7 @@
#![allow(clippy::disallowed_methods)]
#![allow(clippy::disallowed_types)]
#![allow(clippy::mixed_read_write_in_expression)]
#![allow(clippy::for_loops_over_fallibles)]
#![allow(for_loops_over_fallibles)]
#![allow(clippy::useless_conversion)]
#![allow(clippy::match_result_ok)]
#![allow(clippy::overly_complex_bool_expr)]
@ -65,6 +65,7 @@
#![warn(clippy::to_string_in_display)]
#![warn(clippy::zero_width_space)]
#![warn(clippy::drop_bounds)]
#![warn(clippy::for_loops_over_fallibles)]
#![warn(clippy::into_iter_on_array)]
#![warn(clippy::invalid_atomic_ordering)]
#![warn(clippy::invalid_ref)]

34
src/tools/clippy/tests/ui/rename.stderr
View File

@ -54,17 +54,17 @@ error: lint `clippy::eval_order_dependence` has been renamed to `clippy::mixed_r
LL | #![warn(clippy::eval_order_dependence)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::mixed_read_write_in_expression`
error: lint `clippy::for_loop_over_option` has been renamed to `clippy::for_loops_over_fallibles`
error: lint `clippy::for_loop_over_option` has been renamed to `for_loops_over_fallibles`
--> $DIR/rename.rs:48:9
|
LL | #![warn(clippy::for_loop_over_option)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles`
error: lint `clippy::for_loop_over_result` has been renamed to `clippy::for_loops_over_fallibles`
error: lint `clippy::for_loop_over_result` has been renamed to `for_loops_over_fallibles`
--> $DIR/rename.rs:49:9
|
LL | #![warn(clippy::for_loop_over_result)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles`
error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion`
--> $DIR/rename.rs:50:9
@ -174,59 +174,65 @@ error: lint `clippy::drop_bounds` has been renamed to `drop_bounds`
LL | #![warn(clippy::drop_bounds)]
| ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds`
error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter`
error: lint `clippy::for_loops_over_fallibles` has been renamed to `for_loops_over_fallibles`
--> $DIR/rename.rs:68:9
|
LL | #![warn(clippy::for_loops_over_fallibles)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles`
error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter`
--> $DIR/rename.rs:69:9
|
LL | #![warn(clippy::into_iter_on_array)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter`
error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering`
--> $DIR/rename.rs:69:9
--> $DIR/rename.rs:70:9
|
LL | #![warn(clippy::invalid_atomic_ordering)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering`
error: lint `clippy::invalid_ref` has been renamed to `invalid_value`
--> $DIR/rename.rs:70:9
--> $DIR/rename.rs:71:9
|
LL | #![warn(clippy::invalid_ref)]
| ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value`
error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums`
--> $DIR/rename.rs:71:9
--> $DIR/rename.rs:72:9
|
LL | #![warn(clippy::mem_discriminant_non_enum)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums`
error: lint `clippy::panic_params` has been renamed to `non_fmt_panics`
--> $DIR/rename.rs:72:9
--> $DIR/rename.rs:73:9
|
LL | #![warn(clippy::panic_params)]
| ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics`
error: lint `clippy::positional_named_format_parameters` has been renamed to `named_arguments_used_positionally`
--> $DIR/rename.rs:73:9
--> $DIR/rename.rs:74:9
|
LL | #![warn(clippy::positional_named_format_parameters)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally`
error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr`
--> $DIR/rename.rs:74:9
--> $DIR/rename.rs:75:9
|
LL | #![warn(clippy::temporary_cstring_as_ptr)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr`
error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints`
--> $DIR/rename.rs:75:9
--> $DIR/rename.rs:76:9
|
LL | #![warn(clippy::unknown_clippy_lints)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints`
error: lint `clippy::unused_label` has been renamed to `unused_labels`
--> $DIR/rename.rs:76:9
--> $DIR/rename.rs:77:9
|
LL | #![warn(clippy::unused_label)]
| ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels`
error: aborting due to 38 previous errors
error: aborting due to 39 previous errors