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

needless_else: new lint to check for empty else clauses #10810

Merged
merged 1 commit into from
May 23, 2023

Conversation

samueltardieu
Copy link
Contributor

Empty else clauses are useless. They happen in the wild and are not linted yet: https://github.com/uutils/coreutils/pull/4880/files

else clauses containing or preceded by comments are not linted as the comments might be important.

changelog: [needless_else]: new lint

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2023

r? @llogiq

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 22, 2023
@sylvestre
Copy link
Contributor

i wonder how often it is happening :)

/// ```
#[clippy::version = "1.71.0"]
pub NEEDLESS_ELSE,
style,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how often that happens. Given that it's a style lint (which is warn by default), I worry that it might drown out more important messages if we leave it that way. On the other hand, if it's rare anyway, perhaps it's not a big deal. I'll run lintcheck and report back.

@llogiq
Copy link
Contributor

llogiq commented May 23, 2023

Ok, so lintcheck didn't find any instance. Instances will probably be mostly in half-refactored code. Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2023

📌 Commit e6646eb has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 23, 2023

⌛ Testing commit e6646eb with merge 97598e9...

@bors
Copy link
Collaborator

bors commented May 23, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 97598e9 to master...

@samueltardieu
Copy link
Contributor Author

Ok, so lintcheck didn't find any instance. Instances will probably be mostly in half-refactored code. Thank you!

Yeah, I should have reported that I ran lintcheck with the 500 most downloaded crates and found no occurrences.

bors added a commit that referenced this pull request May 27, 2023
Ignore `#[cfg]`'d out code in `needless_else`

changelog: none (same release as #10810)

`#[cfg]` making things fun once more

This lead me to think about macro calls that expand to nothing as well, but apparently they produce an empty stmt in the AST so are already handled, added a test for that

r? `@llogiq`
@samueltardieu samueltardieu deleted the needless-else branch November 29, 2023 22:41
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.

5 participants