Skip to content

clippy suggests a change (use of iterator flatten) which it reports as an error if implemented #10186

@hschimke

Description

@hschimke

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: Clippy is not doing the correct thing

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions