Skip to content

Fix Clippy lints #55

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

Closed
wants to merge 1 commit into from

Conversation

evading
Copy link
Contributor

@evading evading commented Apr 15, 2025

Clippy lints suggesting parenthesizing expressions are fixed.

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

  • The code compiles without errors or warnings.
  • All tests pass and in the best case you also added new tests.
  • cargo +stable fmt was run.
  • cargo +stable clippy yields no warnings.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You add a description of your work to this PR.
  • You added proper docs (in code, rustdoc and README.md) for your
    newly added features and code.

@evading evading requested a review from a team as a code owner April 15, 2025 12:51
Clippy lints suggesting parenthesizing expressions are fixed.
@evading evading force-pushed the work/evading/clippy-lints branch from 1074028 to ae3552d Compare April 15, 2025 13:01
Copy link
Collaborator

@epontan epontan left a comment

Choose a reason for hiding this comment

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

Nice, thank you! LGTM 👍

@Ironedde
Copy link
Contributor

Hey @evading!

Always nice to fix additional lints 👍
I'm assuming this is a fix for clippy::precedence_bits[1]?

Since these lints are currently not part of this regular checks, is there some specific linting group, or subset, that you would propose us adding to the GitHub actions?

[1] https://rust-lang.github.io/rust-clippy/master/index.html#precedence_bits

@evading
Copy link
Contributor Author

evading commented Apr 16, 2025

clippy::precedence_bits

It's actually clippy:precedence and I find it weird that the check doesn't get it. I'm not that well versed on GitHub actions but there is some error reported in the clippy action.

I just ran cargo +stable clippy in the repo, using Cargo 1.84.0 and it reported all these lints.

@Ironedde
Copy link
Contributor

Ironedde commented Apr 16, 2025

Interesting. When I run it locally with the same toolchain: cargo +1.84.0 clippy I don't get any warnings.
And I don't get any warnings on 1.86.0 either.

Do you have some global config that enables a larger subset of lints?
And was it only this singular lint that showed up for you?

Furthermore, clippy::precedence_bits was added as part of rust toolchain 1.86.0 (set as allow) so that would explain why you don't see it.

Regarding why the GitHub action isn't working. We have seen the issue before.
It is actually reporting the clippy check correctly (pass or fail), but (for some reason) when running a check against a fork with the same name as the main repo mcan it runs into issues displaying the errors.

@evading
Copy link
Contributor Author

evading commented Apr 16, 2025

Weird thing is that now after running rustup update I can't reproduce the lint either. Not even with cargo +1.84.0 clippy. Even though 1.84.0 was the version I was running before.

@Ironedde
Copy link
Contributor

Interesting. I thought that the clippy version was tied to the rust toolchain version. But from what you see, it seams like clippy has been updated independently from rust.

At least I know why the lint doesn't show up on 1.86.0, some functionality from precedence was moved into the new precedence_bits[1]. Which was set to allow.

But I guess this means that there is no need for a clippy fix anymore?

[1] rust-lang/rust-clippy#14115

@evading
Copy link
Contributor Author

evading commented Apr 23, 2025

It's weird indeed. I actually checked the clippy version and that seems to be tied to the toolchain. I still can't reproduce though. I'll close this.

@evading evading closed this Apr 23, 2025
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