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

Add Collapsible match lint #6402

Merged
merged 6 commits into from
Dec 3, 2020
Merged

Conversation

camsteffen
Copy link
Contributor

@camsteffen camsteffen commented Nov 29, 2020

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:

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

to

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.

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 29, 2020
@camsteffen
Copy link
Contributor Author

Hmm my stderr is too long? I'm not sure what to do about that.

@giraffate
Copy link
Contributor

How about splitting a test file into two?

@camsteffen
Copy link
Contributor Author

Done.

@llogiq
Copy link
Contributor

llogiq commented Nov 30, 2020

Good stuff! I especially like the SpanlessEq extension. I would start in the style category until we get reports about it swamping the output. @bors r+

@bors
Copy link
Collaborator

bors commented Nov 30, 2020

📌 Commit 0e20788 has been approved by llogiq

@bors
Copy link
Collaborator

bors commented Nov 30, 2020

⌛ Testing commit 0e20788 with merge 6d847c4...

bors added a commit that referenced this pull request 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
Copy link
Collaborator

bors commented Nov 30, 2020

💔 Test failed - checks-action_test

@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 2, 2020
@flip1995
Copy link
Member

flip1995 commented Dec 3, 2020

@bors retry rollup

bors added a commit that referenced this pull request Dec 3, 2020
Rollup of 4 pull requests

Successful merges:

 - #6308 (add `internal-lints` feature to enable clippys internal lints (off by default))
 - #6395 (switch Version/VersionReq usages to RustcVersion )
 - #6402 (Add Collapsible match lint)
 - #6407 (CONTRIBUTING: update bors queue url from buildbot2.rlo to bors.rlo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

changelog: rollup
@bors bors merged commit e2ecc4a into rust-lang:master Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Single if let instead of nested if let { if let Merge nested matches
6 participants