summary refs log tree commit diff
path: root/src/tools/clippy/tests
diff options
context:
space:
mode:
authorDylan DPC <99973273+Dylan-DPC@users.noreply.github.com>2022-10-10 13:43:40 +0530
committerGitHub <noreply@github.com>2022-10-10 13:43:40 +0530
commit7e16f9f1eaea28e7fe2fc93319fdfcd4e1968c91 (patch)
treeb96433616143a8f0a449cec841fc9be07de08d4b /src/tools/clippy/tests
parente495b37c9a301d776a7bd0c72d5c4a202e159032 (diff)
parent9d4edff1b0179e3514bcc27618edd6348903e99a (diff)
downloadrust-7e16f9f1eaea28e7fe2fc93319fdfcd4e1968c91.tar.gz
rust-7e16f9f1eaea28e7fe2fc93319fdfcd4e1968c91.zip
Rollup merge of #99696 - WaffleLapkin:uplift, r=fee1-dead
Uplift `clippy::for_loops_over_fallibles` lint into rustc

This PR, as the title suggests, uplifts [`clippy::for_loops_over_fallibles`] lint into rustc. This lint warns for code like this:
```rust
for _ in Some(1) {}
for _ in Ok::<_, ()>(1) {}
```
i.e. directly iterating over `Option` and `Result` using `for` loop.

There are a number of suggestions that this PR adds (on top of what clippy suggested):
1. If the argument (? is there a better name for that expression) of a `for` loop is a `.next()` call, then we can suggest removing it (or rather replacing with `.by_ref()` to allow iterator being used later)
   ```rust
    for _ in iter.next() {}
    // turns into
    for _ in iter.by_ref() {}
    ```
2. (otherwise) We can suggest using `while let`, this is useful for non-iterator, iterator-like things like [async] channels
   ```rust
   for _ in rx.recv() {}
   // turns into
   while let Some(_) = rx.recv() {}
   ```
3. If the argument type is `Result<impl IntoIterator, _>` and the body has a `Result<_, _>` type, we can suggest using `?`
   ```rust
   for _ in f() {}
   // turns into
   for _ in f()? {}
   ```
4. To preserve the original behavior and clear intent, we can suggest using `if let`
   ```rust
   for _ in f() {}
   // turns into
   if let Some(_) = f() {}
   ```
(P.S. `Some` and `Ok` are interchangeable depending on the type)

I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)!

Resolves #99272

[`clippy::for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
Diffstat (limited to 'src/tools/clippy/tests')
-rw-r--r--src/tools/clippy/tests/ui/for_loop_unfixable.rs1
-rw-r--r--src/tools/clippy/tests/ui/for_loop_unfixable.stderr2
-rw-r--r--src/tools/clippy/tests/ui/for_loops_over_fallibles.rs73
-rw-r--r--src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr95
-rw-r--r--src/tools/clippy/tests/ui/manual_map_option.fixed2
-rw-r--r--src/tools/clippy/tests/ui/manual_map_option.rs2
-rw-r--r--src/tools/clippy/tests/ui/rename.fixed7
-rw-r--r--src/tools/clippy/tests/ui/rename.rs3
-rw-r--r--src/tools/clippy/tests/ui/rename.stderr34
9 files changed, 30 insertions, 189 deletions
diff --git a/src/tools/clippy/tests/ui/for_loop_unfixable.rs b/src/tools/clippy/tests/ui/for_loop_unfixable.rs
index efcaffce24e..55fb3788a8b 100644
--- a/src/tools/clippy/tests/ui/for_loop_unfixable.rs
+++ b/src/tools/clippy/tests/ui/for_loop_unfixable.rs
@@ -8,6 +8,7 @@
     clippy::for_kv_map
 )]
 #[allow(clippy::linkedlist, clippy::unnecessary_mut_passed, clippy::similar_names)]
+#[allow(for_loops_over_fallibles)]
 fn main() {
     let vec = vec![1, 2, 3, 4];
 
diff --git a/src/tools/clippy/tests/ui/for_loop_unfixable.stderr b/src/tools/clippy/tests/ui/for_loop_unfixable.stderr
index f769b4bdc94..50a86eaa68f 100644
--- a/src/tools/clippy/tests/ui/for_loop_unfixable.stderr
+++ b/src/tools/clippy/tests/ui/for_loop_unfixable.stderr
@@ -1,5 +1,5 @@
 error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
-  --> $DIR/for_loop_unfixable.rs:14:15
+  --> $DIR/for_loop_unfixable.rs:15:15
    |
 LL |     for _v in vec.iter().next() {}
    |               ^^^^^^^^^^^^^^^^^
diff --git a/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs b/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs
deleted file mode 100644
index 4b2a9297d08..00000000000
--- a/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs
+++ /dev/null
@@ -1,73 +0,0 @@
-#![warn(clippy::for_loops_over_fallibles)]
-#![allow(clippy::uninlined_format_args)]
-
-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() {}
diff --git a/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr b/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr
deleted file mode 100644
index f09adccabd1..00000000000
--- a/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr
+++ /dev/null
@@ -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
-
diff --git a/src/tools/clippy/tests/ui/manual_map_option.fixed b/src/tools/clippy/tests/ui/manual_map_option.fixed
index a59da4ae10b..e12ea7ec145 100644
--- a/src/tools/clippy/tests/ui/manual_map_option.fixed
+++ b/src/tools/clippy/tests/ui/manual_map_option.fixed
@@ -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
 )]
 
diff --git a/src/tools/clippy/tests/ui/manual_map_option.rs b/src/tools/clippy/tests/ui/manual_map_option.rs
index 0bdbefa51e8..325a6db06c4 100644
--- a/src/tools/clippy/tests/ui/manual_map_option.rs
+++ b/src/tools/clippy/tests/ui/manual_map_option.rs
@@ -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
 )]
 
diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed
index a6e7bdba77c..8beae8dee08 100644
--- a/src/tools/clippy/tests/ui/rename.fixed
+++ b/src/tools/clippy/tests/ui/rename.fixed
@@ -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)]
diff --git a/src/tools/clippy/tests/ui/rename.rs b/src/tools/clippy/tests/ui/rename.rs
index e8f57597d02..9e665047baa 100644
--- a/src/tools/clippy/tests/ui/rename.rs
+++ b/src/tools/clippy/tests/ui/rename.rs
@@ -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)]
diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr
index 31865a7f66d..63eb565185f 100644
--- a/src/tools/clippy/tests/ui/rename.stderr
+++ b/src/tools/clippy/tests/ui/rename.stderr
@@ -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