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

clippy1 no longer fails linting #906

Closed
radicale opened this issue Jan 6, 2022 · 5 comments
Closed

clippy1 no longer fails linting #906

radicale opened this issue Jan 6, 2022 · 5 comments

Comments

@radicale
Copy link
Contributor

radicale commented Jan 6, 2022

The relative float_cmp lint has been demoted from the correctness category (default deny) to pedantic (default allow) in the following PR:

rust-lang/rust-clippy#7692

Consequently, the clippy1 exercise doesn't fail linting and leaves the user with nothing to fix. Possible solutions include rewriting the exercise to leverage another lint or adding the -D clippy::float_cmp arguments when invoking clippy.

@radicale
Copy link
Contributor Author

radicale commented Jan 6, 2022

Or perhaps it would be more appropriate to setup the lints in the source files themselves with something like:
#![deny(clippy::all, clippy::float_cmp)]

@qq8244353
Copy link

I was working on the exercise and this problem confused me a lot, so I think it needs to be fixed.
I confirmed attaching the attribute mentioned above to the main function will work, tho I think adding some message in the source code that kindly explains the situation something goes like "This is required because the linter no longer fails due to the recent update" is better.
If you don’t mind, please let me make a pull request with this content.

@hilary
Copy link

hilary commented Apr 11, 2022

I came to look at the issues today due to clippy not failing. FWIW, of the options discussed, I like adding

#![deny(clippy::float_cmp)]

best as a quick fix, as it expands on the lesson.

@qq8244353
Copy link

Thanks for the advice.
I tried to make a change, but it seems like the exercise we are talking about has been replaced.
And I guess why the merge above doesn't fix the problem is because change has not been implemented to the latest version.

@shadows-withal
Copy link
Member

This is a duplicate of #888, which has been closed by #890, which will make it into the next version.

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

No branches or pull requests

4 participants