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

Lint negation "followed" by method call #3903

Closed
oli-obk opened this issue Mar 24, 2019 · 4 comments
Closed

Lint negation "followed" by method call #3903

oli-obk opened this issue Mar 24, 2019 · 4 comments
Labels
L-complexity Lint: Belongs in the complexity lint group

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 24, 2019

Calling -x.foo() will first call foo and then negate. While this is correct as per the precedence rules, it can be surprising (see rust-lang/rust#59388). We should suggest a cleanup either by writing (-x).foo() if that was intended, by writing -(x.foo()) for by introducing an intermediate variable:

let x_foo = x.foo();
-x_foo
@oli-obk oli-obk added the L-complexity Lint: Belongs in the complexity lint group label Mar 24, 2019
@felix91gr
Copy link
Contributor

Yeah, this makes a lot of sense. The precedence rules are perfect for compilers, but people don't necessarily have them ingrained. And code is read much more times than it is written 👍

@Centril
Copy link
Contributor

Centril commented Apr 3, 2019

In my mind -x.foo() is no different than !x.foo(). This is the normal precedence of unary operators which many will understand. So I think this should not be in the complexity category as that would declare -x.foo() to be mostly unidiomatic.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 4, 2019

I agree. The original issue was on numeric literals, not on variables. On variables it seems pretty clear. Linting -12.foo() but not -x.foo() seems like a middle ground to me.

Note that literals other than integers or floats should not be linted. -"text".some_method() is not negating the string literal and I think that's obvious enough

@memoryruins
Copy link

Numeric literals currently emit a precedence lint:

trait Trait { fn method(&self) -> i32 { 42 } }
impl Trait for &str {}

fn main() {
    let a = -42_i32.abs();
    let b = -42_f32.abs();
    let c = -"".method();
    dbg!(a, b, c);
}
warning: unary minus has lower precedence than method call
 --> src/main.rs:5:13
  |
5 |     let a = -42_i32.abs();
  |             ^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(42_i32.abs())`
  |
  = note: `#[warn(clippy::precedence)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#precedence

warning: unary minus has lower precedence than method call
 --> src/main.rs:6:13
  |
6 |     let b = -42_f32.abs();
  |             ^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(42_f32.abs())`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#precedence

Is there anything left to do for this issue?

@oli-obk oli-obk closed this as completed Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L-complexity Lint: Belongs in the complexity lint group
Projects
None yet
Development

No branches or pull requests

4 participants