-
Notifications
You must be signed in to change notification settings - Fork 12.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand for_loops_over_fallibles
lint to lint on fallibles behind references.
#125156
Expand for_loops_over_fallibles
lint to lint on fallibles behind references.
#125156
Conversation
…t T = Result/Option.
…&mut T, not just T = Result/Option.'
r? @nnethercote rustbot has assigned @nnethercote. Use |
Some changes occurred in match lowering cc @Nadrieril |
r=me after fixing the article |
7803d8b
to
66573b7
Compare
@bors r+ |
…ind_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..c33b1a2 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 an `&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 an `&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.08s ``` </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`) `@rustbot` label +A-lint
too much hope @bors r- |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@rustbot ready CI is green so probably that's all the tests fixed? |
@bors r=Nilstrieb |
…ind_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 comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Some changes occurred in src/tools/compiletest cc @jieyouxu |
@bors r+ |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#122665 (Add some tests for public-private dependencies.) - rust-lang#123623 (Fix OutsideLoop's error suggestion: adding label `'block` for `if` block.) - rust-lang#125054 (Handle `ReVar` in `note_and_explain_region`) - rust-lang#125156 (Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.) - rust-lang#125222 (Migrate `run-make/issue-46239` to `rmake`) - rust-lang#125316 (Tweak `Spacing` use) - rust-lang#125392 (Wrap Context.ext in AssertUnwindSafe) - rust-lang#125417 (self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#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
Extends the scope of the (warn-by-default) lint
for_loops_over_fallibles
from justfor _ in x
wherex: Option<_>/Result<_, _>
to also coverx: &(mut) Option<_>/Result<_>
cargo build
diffQuestion:
Currently, the articleThe articlean
is used for&Option
, and&mut Option
in the lint diagnostic, since that's whatOption
uses. Is this okay or should it be changed? (likewise,a
is used for&Result
and&mut Result
)a
is used for&Option
,&mut Option
,&Result
,&mut Result
and (as before)Result
. OnlyOption
usesan
(as before).@rustbot label +A-lint