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
This commit is contained in:
Matthias Krüger
GitHub
commit
4af1c31fcf
11 changed files with 112 additions and 15 deletions
|
|
@ -263,7 +263,7 @@ lint_extern_without_abi = extern declarations without an explicit ABI are deprec
|
|||
.help = the default ABI is {$default_abi}
|
||||
|
||||
lint_for_loops_over_fallibles =
|
||||
for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement
|
||||
for loop over {$article} `{$ref_prefix}{$ty}`. This is more readably written as an `if let` statement
|
||||
.suggestion = consider using `if let` to clear intent
|
||||
.remove_next = to iterate over `{$recv_snip}` remove the call to `next`
|
||||
.use_while_let = to check pattern in a loop use `while let`
|
||||
|
|
|
|||
|
|
@ -52,14 +52,27 @@ impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles {
|
|||
|
||||
let ty = cx.typeck_results().expr_ty(arg);
|
||||
|
||||
let &ty::Adt(adt, args) = ty.kind() else { return };
|
||||
let (adt, args, ref_mutability) = match ty.kind() {
|
||||
&ty::Adt(adt, args) => (adt, args, None),
|
||||
&ty::Ref(_, ty, mutability) => match ty.kind() {
|
||||
&ty::Adt(adt, args) => (adt, args, Some(mutability)),
|
||||
_ => return,
|
||||
},
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let (article, ty, var) = match adt.did() {
|
||||
did if cx.tcx.is_diagnostic_item(sym::Option, did) && ref_mutability.is_some() => ("a", "Option", "Some"),
|
||||
did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
|
||||
did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let ref_prefix = match ref_mutability {
|
||||
None => "",
|
||||
Some(ref_mutability) => ref_mutability.ref_prefix_str(),
|
||||
};
|
||||
|
||||
let sub = if let Some(recv) = extract_iterator_next_call(cx, arg)
|
||||
&& let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span)
|
||||
{
|
||||
|
|
@ -85,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles {
|
|||
cx.emit_span_lint(
|
||||
FOR_LOOPS_OVER_FALLIBLES,
|
||||
arg.span,
|
||||
ForLoopsOverFalliblesDiag { article, ty, sub, question_mark, suggestion },
|
||||
ForLoopsOverFalliblesDiag { article, ref_prefix, ty, sub, question_mark, suggestion },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -620,6 +620,7 @@ pub enum PtrNullChecksDiag<'a> {
|
|||
#[diag(lint_for_loops_over_fallibles)]
|
||||
pub struct ForLoopsOverFalliblesDiag<'a> {
|
||||
pub article: &'static str,
|
||||
pub ref_prefix: &'static str,
|
||||
pub ty: &'static str,
|
||||
#[subdiagnostic]
|
||||
pub sub: ForLoopsOverFalliblesLoopSub<'a>,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue