false positives in eq_op lint

[birkenfeld]

I just got this false positive when running clippy over racer:

src/racer/cargo.rs:314:12: 314:30 warning: equal expressions as operands to &&, #[warn(eq_op)] on by default
src/racer/cargo.rs:314         if f.pop() && f.pop() {
                                  ^~~~~~~~~~~~~~~~~~

I think the eq_op lint should be reduced a lot. Equal expressions at the AST level are not guaranteed to have the same result, and of course may have side effects too.

The lint should only look for simple expressions like consts (using const eval), variable lookups, field access and maybe casts to primitive types. Even operator application can have side effects.

[llogiq]

I'm not sure if completely pruning the lint is the right answer to one false positive. The code certainly could do with some let-binding for the two popped bools, that would introduce a readable name for each and thus increase readability.

That said, even if we consider this false positive to be so serious we want to pare back eq_op, I'd at least keep a blacklist (similar to your inherent_methods lint) of pure functions we intend to catch, e.g. Display::to_string(), Option::as_ref(), etc.

Such a list would probably be useful in other settings, too.

[Thoralf-M]

I just came across if alias_output.state_index() == alias_output.state_index() { and wondered why clippy didn't give a warning for that. Wouldn't it be better to just add #[allow(clippy::eq_op)] in cases where it's fine?