- Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Summary
Given a loop that could be simplified using a flatten().iter().next() + if let Some()
statement, clippy suggests an invalid solution, which it then suggests a correction for if implemented as suggested. The intermediate (incorrect) solution should be omitted.
Full context for the example bellow is at: https://github.com/hschimke/rxing/blob/4bd2e53a5c26cd40edb0337ed61ffe3bedacc4a6/src/pdf417/decoder/pdf_417_scanning_decoder.rs#L619
Reproducer
Given the below code:
for previousRowCodeword in detectionRXingResult .getDetectionRXingResultColumn(barcodeColumn as usize) .as_ref() .unwrap() .getCodewords() { // for (Codeword previousRowCodeword : detectionRXingResult.getDetectionRXingResultColumn(barcodeColumn).getCodewords()) { if let Some(previousRowCodeword) = previousRowCodeword { // if previousRowCodeword.is_some() { return ((if leftToRight { previousRowCodeword.getEndX() } else { previousRowCodeword.getStartX() }) as isize + offset * skippedColumns as isize * (previousRowCodeword.getEndX() - previousRowCodeword.getStartX()) as isize) as u32; } } skippedColumns += 1; }
clippy suggests:
warning: unnecessary `if let` since only the `Some` variant of the iterator element is used --> src/pdf417/decoder/pdf_417_scanning_decoder.rs:619:9 | 619 | / for previousRowCodeword in detectionRXingResult 620 | | .getDetectionRXingResultColumn(barcodeColumn as usize) 621 | | .as_ref() 622 | | .unwrap() ... | 637 | | } 638 | | } | |_________^ | help: remove the `if let` statement in the for loop and then... --> src/pdf417/decoder/pdf_417_scanning_decoder.rs:626:13 | 626 | / if let Some(previousRowCodeword) = previousRowCodeword { 627 | | // if previousRowCodeword.is_some() { 628 | | return ((if leftToRight { 629 | | previousRowCodeword.getEndX() ... | 636 | | as isize) as u32; 637 | | } | |_____________^ = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten = note: `#[warn(clippy::manual_flatten)]` on by default help: try | 619 ~ for previousRowCodeword in detectionRXingResult 620 + .getDetectionRXingResultColumn(barcodeColumn as usize) 621 + .as_ref() 622 + .unwrap() 623 + .getCodewords().iter().flatten() |
If I implement the change such that the code looks like:
for previousRowCodeword in detectionRXingResult .getDetectionRXingResultColumn(barcodeColumn as usize) .as_ref() .unwrap() .getCodewords().iter().flatten() { // for (Codeword previousRowCodeword : detectionRXingResult.getDetectionRXingResultColumn(barcodeColumn).getCodewords()) { // if let Some(previousRowCodeword) = previousRowCodeword { // if previousRowCodeword.is_some() { return ((if leftToRight { previousRowCodeword.getEndX() } else { previousRowCodeword.getStartX() }) as isize + offset * skippedColumns as isize * (previousRowCodeword.getEndX() - previousRowCodeword.getStartX()) as isize) as u32; // } }
This code does not necessarily make sense, but it is what was suggested by the lint. Rerunning clippy results in:
error: this loop never actually loops --> src/pdf417/decoder/pdf_417_scanning_decoder.rs:619:9 | 619 | / for previousRowCodeword in detectionRXingResult 620 | | .getDetectionRXingResultColumn(barcodeColumn as usize) 621 | | .as_ref() 622 | | .unwrap() ... | 637 | | // } 638 | | } | |_________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#never_loop = note: `#[deny(clippy::never_loop)]` on by default help: if you need the first element of the iterator, try writing | 619 ~ if let Some(previousRowCodeword) = detectionRXingResult 620 + .getDetectionRXingResultColumn(barcodeColumn as usize) 621 + .as_ref() 622 + .unwrap() 623 + .getCodewords().iter().flatten().next() |
Changing the code as suggested in the final error results in:
if let Some(previousRowCodeword) = detectionRXingResult .getDetectionRXingResultColumn(barcodeColumn as usize) .as_ref() .unwrap() .getCodewords().iter().flatten().next() { // for (Codeword previousRowCodeword : detectionRXingResult.getDetectionRXingResultColumn(barcodeColumn).getCodewords()) { // if let Some(previousRowCodeword) = previousRowCodeword { // if previousRowCodeword.is_some() { return ((if leftToRight { previousRowCodeword.getEndX() } else { previousRowCodeword.getStartX() }) as isize + offset * skippedColumns as isize * (previousRowCodeword.getEndX() - previousRowCodeword.getStartX()) as isize) as u32; // } }
This is correct, and will execute as expected.
Version
`rustc 1.66.0 (69f9c33d7 2022-12-12)` `cargo 1.66.0 (d65d197ad 2022-11-15)` `clippy 0.1.66 (69f9c33d 2022-12-12)`
Additional Labels
No response