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

Pedantic lint against unreachable!() #4959

Closed
Luro02 opened this issue Dec 26, 2019 · 6 comments
Closed

Pedantic lint against unreachable!() #4959

Luro02 opened this issue Dec 26, 2019 · 6 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@Luro02
Copy link

Luro02 commented Dec 26, 2019

It would be nice to have a lint, that detects any use of the unreachable! macro without arguments.

This is useful for procedural macros, where only the following is shown

error: proc-macro derive panicked
 --> tests/test_union.rs:3:10
  |
3 | #[derive(ShortHand)]
  |          ^^^^^^^^^
  |
  = help: message: internal error: entered unreachable code

error: aborting due to previous error

error: could not compile `shorthand`.

To learn more, run the command again with --verbose.

This is fine, if you only have one or two uses of unreachable, but it becomes very difficult in larger codebases, because you have to check every occurrence of unreachable in the codebase.

Adding some text to the macro will be a lot more helpful!
unreachable!("ident is neither enable nor disable")

error: proc-macro derive panicked
 --> tests/test_union.rs:3:10
  |
3 | #[derive(ShortHand)]
  |          ^^^^^^^^^
  |
  = help: message: internal error: entered unreachable code: ident is neither enable nor disable

error: aborting due to previous error

error: could not compile `shorthand`.

To learn more, run the command again with --verbose.

I think panic! in a proc-macro has the same issue.

@JohnTitor JohnTitor added the A-lint Area: New lints label Dec 27, 2019
@flip1995 flip1995 added the good-first-issue These issues are a good way to get started with Clippy label Dec 28, 2019
@flip1995
Copy link
Member

Easy extension of #4657

@chansuke
Copy link
Contributor

chansuke commented Jan 3, 2020

I'd like to work on this issue

@flip1995
Copy link
Member

flip1995 commented Jan 3, 2020

I just realized, that this lint already exists, but is allow-by-default (and will stay that way). So you have to enable it with #[warn(clippy::unreachable)]. The only thing, I can think of on how to improve this lint, is to add a configuration option to disable the lint if an explanation is provided. Would that help?

@Luro02
Copy link
Author

Luro02 commented Jan 3, 2020

I think the opposite would be better, add a lint, that will warn if unreachable! has no explanation?

@flip1995
Copy link
Member

flip1995 commented Jan 3, 2020

That lint already exists. But it warns regardless of if unreachable! has an explanation or not. Playground

@flip1995
Copy link
Member

flip1995 commented Jan 8, 2020

Just realized, that an issue exists for this (the lint doesn't trigger, when a message is passed)

Duplicate of #3528

@flip1995 flip1995 closed this as completed Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants