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

Lint pass to warn about catchall cases #3482

Closed
catamorphism opened this issue Sep 13, 2012 · 6 comments
Closed

Lint pass to warn about catchall cases #3482

catamorphism opened this issue Sep 13, 2012 · 6 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@catamorphism
Copy link
Contributor

#3481 arose because kind::check_expr had an _ case in its match, so when expr_struct got added as a case, the _ case covered it, which is not what was intended. A lint pass to warn about _ cases would be good. It could be less annoying if it only complained about a _ where the scrutinee has an enum type and the _ might match two or more different variants.

Since whether to use _s is a matter of taste, this should be off by default.

@catamorphism
Copy link
Contributor Author

far-future

@emberian
Copy link
Member

It'd be useful if the lint listed which cases weren't covered.

@flaper87
Copy link
Contributor

Triage bump. I just created an issue about this and turned out being a dup. This still needs to be done.

@lilyball
Copy link
Contributor

One alternative to warning on all catchalls is to have an attribute that marks an enum such that you aren't allowed to use a catchall with that particular enum.

@steveklabnik
Copy link
Member

Triage: no change

@steveklabnik
Copy link
Member

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 6, 2018
Fixes clippy toolstate.

Changes:
````
Remove -preview suffix from README
rustup clippy build with latest rustc (breakage due to rust-lang@08f8fae )
Forgot to remove some debugging code ...
Improved code noted by clippy.
Fix bug in `implicit_return`. Bug was already covered by test, but test was not checked for.
fix rust-lang#3482 and add ui test for it
Don't change current working directory of cargo tests
Use cargo's "PROFILE" envvar and set CLIPPY_DOGFOOD
Use dogfood_runner for deterministic test ordering
Remove unnecessary documentation
Fix dogfood tests.
Added additional reasoning to `Why is this bad?`. Added comment to explain usage of MIR.
Renamed to `implicit_return`. Covered all other kinds besides `ExprKind::Lit`. Added check for replacing `break` with `return`.
Appeasing the Test Gods. Seems I'm not smart enough to run the tests locally before committing.
Renamed `forced_return` to `missing_returns`. Better clarification in the docs. Ran `update_lints`.
Added `FORCED_RETURN` lint.
````
bors added a commit that referenced this issue Dec 6, 2018
submodules: update clippy from 29bf75c to 1df5766

Fixes clippy toolstate.

Changes:
````
Remove -preview suffix from README
rustup clippy build with latest rustc (breakage due to 08f8fae )
Forgot to remove some debugging code ...
Improved code noted by clippy.
Fix bug in `implicit_return`. Bug was already covered by test, but test was not checked for.
fix #3482 and add ui test for it
Don't change current working directory of cargo tests
Use cargo's "PROFILE" envvar and set CLIPPY_DOGFOOD
Use dogfood_runner for deterministic test ordering
Remove unnecessary documentation
Fix dogfood tests.
Added additional reasoning to `Why is this bad?`. Added comment to explain usage of MIR.
Renamed to `implicit_return`. Covered all other kinds besides `ExprKind::Lit`. Added check for replacing `break` with `return`.
Appeasing the Test Gods. Seems I'm not smart enough to run the tests locally before committing.
Renamed `forced_return` to `missing_returns`. Better clarification in the docs. Ran `update_lints`.
Added `FORCED_RETURN` lint.
````
r? @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

5 participants