about summary refs log tree commit diff
path: root/compiler/rustc_builtin_macros/src
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-05-23 07:41:17 +0200
committerGitHub <noreply@github.com>2024-05-23 07:41:17 +0200
commit4af1c31fcfecfeee82259cb0591b37b10d26ebe9 (patch)
treefd19890ec8722d0cd8403964bc978a3b8b52bd28 /compiler/rustc_builtin_macros/src
parent72fd85c617403af31b62c8b61f2dca36c6080e9a (diff)
parenta59d071e9caa140ddaff4a9e1b747dba0d95f0cd (diff)
downloadrust-4af1c31fcfecfeee82259cb0591b37b10d26ebe9.tar.gz
rust-4af1c31fcfecfeee82259cb0591b37b10d26ebe9.zip
Rollup merge of #125156 - zachs18:for_loops_over_fallibles_behind_refs, r=Nilstrieb
Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.

Extends the scope of the (warn-by-default) lint `for_loops_over_fallibles` from just `for _ in x` where `x: Option<_>/Result<_, _>` to also cover `x: &(mut) Option<_>/Result<_>`

```rs
fn main() {
    // Current lints
    for _ in Some(42) {}
    for _ in Ok::<_, i32>(42) {}

    // New lints
    for _ in &Some(42) {}
    for _ in &mut Some(42) {}
    for _ in &Ok::<_, i32>(42) {}
    for _ in &mut Ok::<_, i32>(42) {}

    // Should not lint
    for _ in Some(42).into_iter() {}
    for _ in Some(42).iter() {}
    for _ in Some(42).iter_mut() {}
    for _ in Ok::<_, i32>(42).into_iter() {}
    for _ in Ok::<_, i32>(42).iter() {}
    for _ in Ok::<_, i32>(42).iter_mut() {}
}
```

<details><summary><code>cargo build</code> diff</summary>

```diff
diff --git a/old.out b/new.out
index 84215aa..ca195a7 100644
--- a/old.out
+++ b/new.out
`@@` -1,33 +1,93 `@@`
 warning: for loop over an `Option`. This is more readably written as an `if let` statement
  --> src/main.rs:3:14
   |
 3 |     for _ in Some(42) {}
   |              ^^^^^^^^
   |
   = note: `#[warn(for_loops_over_fallibles)]` on by default
 help: to check pattern in a loop use `while let`
   |
 3 |     while let Some(_) = Some(42) {}
   |     ~~~~~~~~~~~~~~~ ~~~
 help: consider using `if let` to clear intent
   |
 3 |     if let Some(_) = Some(42) {}
   |     ~~~~~~~~~~~~ ~~~

 warning: for loop over a `Result`. This is more readably written as an `if let` statement
  --> src/main.rs:4:14
   |
 4 |     for _ in Ok::<_, i32>(42) {}
   |              ^^^^^^^^^^^^^^^^
   |
 help: to check pattern in a loop use `while let`
   |
 4 |     while let Ok(_) = Ok::<_, i32>(42) {}
   |     ~~~~~~~~~~~~~ ~~~
 help: consider using `if let` to clear intent
   |
 4 |     if let Ok(_) = Ok::<_, i32>(42) {}
   |     ~~~~~~~~~~ ~~~

-warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 2 warnings
-    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
+warning: for loop over a `&Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:7:14
+  |
+7 |     for _ in &Some(42) {}
+  |              ^^^^^^^^^
+  |
+help: to check pattern in a loop use `while let`
+  |
+7 |     while let Some(_) = &Some(42) {}
+  |     ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+  |
+7 |     if let Some(_) = &Some(42) {}
+  |     ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:8:14
+  |
+8 |     for _ in &mut Some(42) {}
+  |              ^^^^^^^^^^^^^
+  |
+help: to check pattern in a loop use `while let`
+  |
+8 |     while let Some(_) = &mut Some(42) {}
+  |     ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+  |
+8 |     if let Some(_) = &mut Some(42) {}
+  |     ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&Result`. This is more readably written as an `if let` statement
+ --> src/main.rs:9:14
+  |
+9 |     for _ in &Ok::<_, i32>(42) {}
+  |              ^^^^^^^^^^^^^^^^^
+  |
+help: to check pattern in a loop use `while let`
+  |
+9 |     while let Ok(_) = &Ok::<_, i32>(42) {}
+  |     ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+  |
+9 |     if let Ok(_) = &Ok::<_, i32>(42) {}
+  |     ~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Result`. This is more readably written as an `if let` statement
+  --> src/main.rs:10:14
+   |
+10 |     for _ in &mut Ok::<_, i32>(42) {}
+   |              ^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to check pattern in a loop use `while let`
+   |
+10 |     while let Ok(_) = &mut Ok::<_, i32>(42) {}
+   |     ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+   |
+10 |     if let Ok(_) = &mut Ok::<_, i32>(42) {}
+   |     ~~~~~~~~~~ ~~~
+
+warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 6 warnings
+    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s

```

</details>

-----

Question:

* ~~Currently, the article `an` is used for `&Option`, and `&mut Option` in the lint diagnostic, since that's what `Option` uses. Is this okay or should it be changed? (likewise, `a` is used for `&Result` and `&mut Result`)~~ The article `a` is used for `&Option`, `&mut Option`, `&Result`, `&mut Result` and (as before) `Result`. Only `Option` uses `an` (as before).

`@rustbot` label +A-lint
Diffstat (limited to 'compiler/rustc_builtin_macros/src')
-rw-r--r--compiler/rustc_builtin_macros/src/deriving/default.rs4
1 files changed, 2 insertions, 2 deletions
diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs
index bf92ddb3370..577523a1d5a 100644
--- a/compiler/rustc_builtin_macros/src/deriving/default.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/default.rs
@@ -3,7 +3,7 @@ use crate::deriving::generic::*;
 use crate::errors;
 use core::ops::ControlFlow;
 use rustc_ast as ast;
-use rustc_ast::visit::walk_list;
+use rustc_ast::visit::visit_opt;
 use rustc_ast::{attr, EnumDef, VariantData};
 use rustc_expand::base::{Annotatable, DummyResult, ExtCtxt};
 use rustc_span::symbol::Ident;
@@ -224,7 +224,7 @@ impl<'a, 'b> rustc_ast::visit::Visitor<'a> for DetectNonVariantDefaultAttr<'a, '
         self.visit_ident(v.ident);
         self.visit_vis(&v.vis);
         self.visit_variant_data(&v.data);
-        walk_list!(self, visit_anon_const, &v.disr_expr);
+        visit_opt!(self, visit_anon_const, &v.disr_expr);
         for attr in &v.attrs {
             rustc_ast::visit::walk_attribute(self, attr);
         }