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

Enable more of the Allowed-by-default lints in rustc #6050

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

faern
Copy link
Member

@faern faern commented Apr 2, 2024

I went through the list at https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html and decided to warn/deny a few more lints. Mostly for cleanliness and consistency. The intention is to keep the code more modern, consistent and get rid of unused code.

  • macro_use_extern_crate - Forbid #[macro_use] to bring macros into global scope. Even using extern crate is deprecated by now, so just extra protection against that
  • explicit_outlives_requirements - Warn aginst explicit lifetime bounds that can be inferred from the code. Keeps noise away.
  • absolute_paths_not_starting_with_crate - Catches Rust 2015 style absolute paths and denies them.
  • missing_abi - Force explicitly stating the ABI of extern items. Less implicit code
  • unused_lifetimes - Warn if you have lifetimes that are not used. Same reason as warning against unused variables
  • unused_macro_rules - Warn if you have a declarative macro with a rule that is never used. Basically same reason as warning on unused variables. Removes dead code

I have tried to avoid allow-by-default lints that seem very nice but that has a warning that it has a fair amount of false positives. I don't want the linting to be a hindrance to productivity or force us to create weird workaround just to please a bad lint. Please tell me if you think some of these lints are going to be more of a problem than a solution.

As you can see, no code changes were needed. We already adhere to all these lints. So it could be argued that they are not needed also. But I think they are a security guarantee that we'll avoid these "mistakes" in the future also.


This change is Reviewable

* macro_use_extern_crate - Forbid #[macro_use] to bring macros into
  global scope. Even using `extern crate` is deprecated by now, so just
  extra protection against that
* explicit_outlives_requirements - Warn aginst explicit lifetime bounds
  that can be inferred from the code. Keeps noise away.
* absolute_paths_not_starting_with_crate - Catches Rust 2015 style
  absolute paths and denies them.
* missing_abi - Force explicitly stating the ABI of `extern` items. Less
  implicit code
* unused_lifetimes - Warn if you have lifetimes that are not used. Same
  reason as warning against unused variables
* unused_macro_rules - Warn if you have a declarative macro with a rule
  that is never used. Basically same reason as warning on unused
  variables. Removes dead code
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@faern faern merged commit ad1ac09 into main Apr 2, 2024
42 checks passed
@faern faern deleted the stricter-rustc-linting branch April 2, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants