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

Running cargo clippy after cargo new fails with eq_op #6635

Closed
whatisaphone opened this issue Jan 24, 2021 · 5 comments
Closed

Running cargo clippy after cargo new fails with eq_op #6635

whatisaphone opened this issue Jan 24, 2021 · 5 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have T-macros Type: Issues with macros and macro expansion

Comments

@whatisaphone
Copy link
Contributor

(Issue originally mentioned here)

Running cargo clippy directly after cargo new fails. This is maybe just my opinion, but it seems a little unfortunate that the default project template doesn't pass the linter.

It fails on assert_eq!(2 + 2, 4). I'm pretty sure that used to be fine. I would expect clippy to flag assert_eq!(4, 4), but not assert_eq!(2 + 2, 4).


Lint name: eq_op

I tried this code:

cargo +nightly-2021-01-23 new --lib foo
cd foo
cargo +nightly-2021-01-23 clippy --all-targets
(This is the default `src/lib.rs`, for reference)
#[cfg(test)]
mod tests {
    #[test]
    fn it_works() {
        assert_eq!(2 + 2, 4);
    }
}

I expected to see this happen:

no errors

Instead, this happened:

error: identical args used in this `assert_eq!` macro call
 --> src\lib.rs:5:20
  |
5 |         assert_eq!(2 + 2, 4);
  |                    ^^^^^^^^
  |
  = note: `#[deny(clippy::eq_op)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#eq_op

error: aborting due to previous error

error: could not compile `foo`

Meta

clippy 0.1.51 (22ddcd1a 2021-01-22)
rustc 1.51.0-nightly (22ddcd1a1 2021-01-22)
binary: rustc
commit-hash: 22ddcd1a13082b7be0fc99b720677efd2b733816
commit-date: 2021-01-22
host: x86_64-pc-windows-msvc
release: 1.51.0-nightly
LLVM version: 11.0.1
@whatisaphone whatisaphone added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jan 24, 2021
@camsteffen camsteffen added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-macros Type: Issues with macros and macro expansion labels Jan 26, 2021
@rail-rain
Copy link
Contributor

Linting cases of equality in terms of value instead of tokens like 2 + 2 == 4 is a feature of this lint from the beginning. Such cases in the test, and the original author spent time to write a fair amount of code to cover these cases. I think what made difference is #6167, which added assert_eq! and its variation to the lint. I agree with that this is "a little unfortunate", but I don't consider this is a false positive that needs to be fixed. Now this lint defines assert_eq!(2 + 2, 4); is problematic, and it would be if it was in production.

@rail-rain
Copy link
Contributor

I had a second thought that this lint might need to ignore everthing in tests, and opened a new issue, which also solves this issue. I still think it is fine to lint assert_eq!(2 + 2, 4) if it's in somewhere other than tests though.

@DevinR528
Copy link
Contributor

Is this something that can be fixed? I would be interested in working on it, something along the lines of

if expr_in_fn_with_name("it_works", expr) && in_file_named("lib", expr.span) { return; }
// or
if snippet(expr.span) == "assert_eq!(2+2, 4)" && in_file_named("lib", expr.span) { return; }

Basically just so there are no errors on a brand new project, the other option would be to change cargo new to do something more interesting?

@rail-rain
Copy link
Contributor

Update: eq_op now ignores all test code (#7811), and therefore Clippy no longer emits an error on it_works. This issue can be closed.

@giraffate
Copy link
Contributor

Thanks for letting us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have T-macros Type: Issues with macros and macro expansion
Projects
None yet
Development

No branches or pull requests

5 participants