Skip to content

warning: manual implementation of ok suggests broken code for fix #14533

Closed as duplicate
@hschimke

Description

@hschimke

Summary

When presented with an if/else-if block the suggestion to "fix" the code the suggested fix is not valid code.

The solution seems to break the if/else-if block by incorrectly placing the proposed blocks.

Reproducer

Given this code:

if let Some(user) = user {
            if let Ok(result) = stmt.query_row((palette_id, &user.id), |row| {
                Ok(ColorPalette {
                    id: row.get(0).unwrap(),
                    name: row.get(1).unwrap(),
                    colors: serde_json::from_str(&row.get::<usize, String>(2).unwrap()).unwrap(),
                    public: row.get(3).unwrap(),
                })
            }) {
                Some(result)
            } else {
                None
            }
        } else if let Ok(result) = stmt.query_row([palette_id], |row| {
            Ok(ColorPalette {
                id: row.get(0).unwrap(),
                name: row.get(1).unwrap(),
                colors: serde_json::from_str(&row.get::<usize, String>(2).unwrap()).unwrap(),
                public: row.get(3).unwrap(),
            })
        }) {
            Some(result)
        } else {
            None
        }

A reasonable expectation would be this suggested change:

if let Some(user) = user {
            if let Ok(result) = stmt.query_row((palette_id, &user.id), |row| {
                Ok(ColorPalette {
                    id: row.get(0).unwrap(),
                    name: row.get(1).unwrap(),
                    colors: serde_json::from_str(&row.get::<usize, String>(2).unwrap()).unwrap(),
                    public: row.get(3).unwrap(),
                })
            }) {
                Some(result)
            } else {
                None
            }
        } else { stmt.query_row([palette_id], |row| {
            Ok(ColorPalette {
                id: row.get(0).unwrap(),
                name: row.get(1).unwrap(),
                colors: serde_json::from_str(&row.get::<usize, String>(2).unwrap()).unwrap(),
                public: row.get(3).unwrap(),
            })
        }).ok()
    }

Instead the suggestion renders this as an option:

warning: manual implementation of `ok`
   --> crates/ok_color_match_datasources/src/sqlite_datasource.rs:479:16
    |
479 |           } else if let Ok(result) = stmt.query_row([palette_id], |row| {
    |  ________________^
480 | |             Ok(ColorPalette {
481 | |                 id: row.get(0).unwrap(),
482 | |                 name: row.get(1).unwrap(),
...   |
489 | |             None
490 | |         }
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err
help: replace with
    |
479 ~         } else stmt.query_row([palette_id], |row| {
480 +             Ok(ColorPalette {
481 +                 id: row.get(0).unwrap(),
482 +                 name: row.get(1).unwrap(),
483 +                 colors: serde_json::from_str(&row.get::<usize, String>(2).unwrap()).unwrap(),
484 +                 public: row.get(3).unwrap(),
485 +             })
486 +         }).ok()
    |

Version

rustc 1.86.0 (05f9846f8 2025-03-31)
binary: rustc
commit-hash: 05f9846f893b09a1be1fc8560e33fc3c815cfecb
commit-date: 2025-03-31
host: aarch64-apple-darwin
release: 1.86.0
LLVM version: 19.1.7

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