Skip to content

Extend branches_sharing_code to match arms with a shared tail#17313

Open
sylvestre wants to merge 1 commit into
rust-lang:masterfrom
sylvestre:match-branches-sharing-code
Open

Extend branches_sharing_code to match arms with a shared tail#17313
sylvestre wants to merge 1 commit into
rust-lang:masterfrom
sylvestre:match-branches-sharing-code

Conversation

@sylvestre

Copy link
Copy Markdown
Contributor

This extends branches_sharing_code to also fire on match expressions, not just if/else.

What it does

When every arm of a match is a block ending in the same trailing expression, and the match is itself in tail position, that expression can be hoisted out below the match:

match mode {
    Mode::A => { a(); Ok(()) }
    Mode::B => { b(); Ok(()) }
}

becomes

match mode {
    Mode::A => { a(); }
    Mode::B => { b(); }
}
Ok(())

This came up from a real cleanup (uutils/coreutils#13129) where a function repeated Ok(()) in every arm.

Guards (to keep it correct and quiet)

  • Every arm body must be a block with a trailing expression; all tails must be HirEq.
  • The tail must not reference any binding introduced inside the arm (pattern bindings or block lets).
  • Trivial () tails and the all-arms-identical case are skipped.
  • A machine-applicable fix is only offered when the match is the trailing expression of its enclosing block, so appending the expression after the match is always valid. When the value is used elsewhere (e.g. bound in a let), the lint stays quiet.

Lintcheck

Ran filtered to this lint:

  • Default crate set (25 crates): 5 new match hits (all in wasmi), 0 false positives.
  • Top 200 most-downloaded crates: 3 new match hits (cc, regex-syntax ×2), 0 false positives.

All inspected hits are genuine and the fixes compile.

Notes

  • This does not catch the exact uutils PR above: there one arm's tail was an if/else with Err(..) in one branch, so the shared Ok(()) wasn't a direct arm tail — that needs early-return restructuring, out of scope here. It catches the simpler, common sibling of that pattern.
  • branches_sharing_code is nursery, so this is opt-in.

changelog: [branches_sharing_code]: also lint match expressions whose arms end with the same expression

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 28, 2026
@rustbot

rustbot commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 8 candidates
  • 8 candidates expanded to 8 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown

Lintcheck changes for 4dd09d6

Lint Added Removed Changed
clippy::branches_sharing_code 8 0 2

This comment will be updated if you push new changes

@sylvestre
sylvestre marked this pull request as draft June 28, 2026 09:16
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 28, 2026
@sylvestre
sylvestre force-pushed the match-branches-sharing-code branch from 8240051 to 572a588 Compare June 28, 2026 11:52
Comment thread clippy_lints/src/ifs/branches_sharing_code.rs
Comment thread clippy_lints/src/ifs/branches_sharing_code.rs
Comment thread tests/ui/branches_sharing_code/shared_match_tail.rs
Comment thread clippy_lints/src/ifs/mod.rs
@sylvestre
sylvestre force-pushed the match-branches-sharing-code branch from b79aae8 to 3f94bf0 Compare June 29, 2026 13:49
@sylvestre
sylvestre requested a review from llogiq June 29, 2026 14:18
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 29, 2026
@sylvestre
sylvestre marked this pull request as ready for review June 29, 2026 14:20

@llogiq llogiq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the extension. The only thing I'd like to see is a few test cases with comments in the match arms (or even before the =>).

View changes since this review

Detect `match` expressions where every arm is a block ending in the same
trailing expression and the `match` is in tail position, suggesting the
expression be hoisted out below the `match`.

changelog: [`branches_sharing_code`]: also lint `match` expressions whose
arms end with the same expression
@sylvestre
sylvestre force-pushed the match-branches-sharing-code branch from 3f94bf0 to 4dd09d6 Compare July 5, 2026 13:03
@rustbot

rustbot commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@sylvestre

Copy link
Copy Markdown
Contributor Author

@llogiq done! thanks

@sylvestre

Copy link
Copy Markdown
Contributor Author

@llogiq can we merge it now? :)
Thanks sorry for the pressure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants