Skip to content

fix: semicolon_inside_block don't fire if block is surrounded by parens #15421

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Aug 5, 2025

changelog: [semicolon_inside_block]: don't fire if block is surrounded by parens

supersedes #15391, fixes #15380

I changed to approach to ignore blocks surrounded by parens, as suggested in #15391 (comment).

To do that, I took inspiration from https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/attempted.20fix.20for.20disallowed_macros/near/532461016 and realized that this lint would benefit from being early-pass rather than late-pass, since that surfaces ExprKind::Paren which we now can explicitly ignore. Well, implicitly rather -- the lint now matches only on a StmtKind::Semi that contains a ExprKind::Block, without a ExprKind::Paren layer inbetween.

@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2025

r? @Alexendoo

rustbot has assigned @Alexendoo.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 5, 2025
Copy link

github-actions bot commented Aug 5, 2025

Lintcheck changes for 168f1c5

Lint Added Removed Changed
clippy::semicolon_outside_block 192 0 17

This comment will be updated if you push new changes

@ada4a
Copy link
Contributor Author

ada4a commented Aug 6, 2025

So apparently the lint got smarter as a side effect of this PR.. I don't really understand what exactly caused that, but the suggestions do seem to be correct, so I've added a quick test case based on one of the added lint firings. If/when we figure out what caused the change, I'd of course give the commits and the test case more sensible names, and maybe minimize the test case further

@ada4a ada4a force-pushed the semicolon-inside-block-2 branch from cf93045 to 620e596 Compare August 6, 2025 19:14
@ada4a
Copy link
Contributor Author

ada4a commented Aug 6, 2025

also no idea why the note: [...] implied by [...] thing disappeared -- probably because of the late-to-early change as well?..

ada4a added 6 commits August 7, 2025 23:44
no change in stderr as one can see
will make it easier to switch to `EarlyContext` later
will make it easier to switch to `EarlyContext` later
- replace the check for trailing expr / last stmt with a check on last
`block.stmts.last()`

this by itself fixes the issue
@ada4a ada4a force-pushed the semicolon-inside-block-2 branch from 620e596 to 168f1c5 Compare August 7, 2025 21:56
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.

semicolon-inside-block removes parens breaking code
3 participants