Skip to content
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

Suggestion: Single if let instead of nested if let { if let #2521

Closed
pickfire opened this issue Mar 11, 2018 · 1 comment · Fixed by #6402
Closed

Suggestion: Single if let instead of nested if let { if let #2521

pickfire opened this issue Mar 11, 2018 · 1 comment · Fixed by #6402
Labels
A-lint Area: New lints L-style Lint: Belongs in the style lint group T-AST Type: Requires working with the AST

Comments

@pickfire
Copy link
Contributor

Use a single if let instead of a nested one whenever possible:

let value = Some(Err("Some error"));
if let Some(Err(ref error)) = value { // TODO
    return Err(format!("Error parsing 'value': {}", error));
}

Instead of:

let value = Some(Err("Some error"));
if let Some(ref result) = value {
    if let Err(ref error) = *result {
        return Err(format!("Error parsing 'value': {}", error));
    }
}
@oli-obk
Copy link
Contributor

oli-obk commented Mar 11, 2018

Note that this doesn't work out nicely if fields of structs are accessed, so these should probably be skipped

@oli-obk oli-obk added T-AST Type: Requires working with the AST A-lint Area: New lints L-style Lint: Belongs in the style lint group labels Mar 11, 2018
bors added a commit that referenced this issue Nov 30, 2020
Add Collapsible match lint

changelog: Add collapsible_match lint

Closes #1252
Closes #2521

This lint finds nested `match` or `if let` patterns that can be squashed together. It is designed to be very conservative to only find cases where merging the patterns would most likely reduce cognitive complexity.

Example:

```rust
match result {
    Ok(opt) => match opt {
        Some(x) => x,
        _ => return,
    }
    _ => return,
}
```
to
```rust
match result {
    Ok(Some(x)) => x,
    _ => return,
}
```

These criteria must be met for the lint to fire:

* The inner match has exactly 2 branches.
* Both the outer and inner match have a "wild" branch like `_ => ..`. There is a special case for `None => ..` to also be considered "wild-like".
* The contents of the wild branches are identical.
* The binding which "links" the matches is never used elsewhere.

Thanks to the hir, `if let`'s are easily included with this lint since they are desugared into equivalent `match`'es.

I think this would fit into the style category, but I would also understand changing it to pedantic.
@bors bors closed this as completed in e2ecc4a Dec 3, 2020
@bors bors closed this as completed in #6402 Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-style Lint: Belongs in the style lint group T-AST Type: Requires working with the AST
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants