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

Suggested lint for numeric literals with leading unary negation #4228

Closed
PhilipDaniels opened this issue Jun 23, 2019 · 4 comments · Fixed by #4615
Closed

Suggested lint for numeric literals with leading unary negation #4228

PhilipDaniels opened this issue Jun 23, 2019 · 4 comments · Fixed by #4615
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group

Comments

@PhilipDaniels
Copy link

I recently posted a thread on Rust Users when I was tripped up by this piece of code

if a >- 150 {

It's a simple typo (it should be >=), but quite difficult to spot. It's not currently warned on by clippy.
A user on the thread did point out that the current behaviour is similar to what happens in other languages, and is not in fact ambiguous with respect to precedence, however, I wondered if people think it worth a lint? It took me an hour or so to find this error.

I can think of two 'problems' with the above that could be converted to a lint:

  • The - should not be separated from the literal
  • >- is not a valid operator.

Of the two, the first is probably a better candidate, as I know there is a tendency for some programmmers to eschew whitespace when writing mathematial expressions making things such as if a>-150 { or x*x>90 quite common (perhaps more so in C than in Rust).

The title says unary negation, but I guess + 200 ought to be considered as well.

@flip1995
Copy link
Member

We could add a (pedantic) lint for the first case you described.

But as already pointed out in the forum thread, rustfmt automatically formats this to if a > -150 { (Playground example). Since many people run rustfmt on save, this lint would not trigger or them and I don't really see another way to detect this.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints labels Jun 23, 2019
@ghost
Copy link

ghost commented Jun 23, 2019

This is similar to suspicious_assignment_formatting. IMO, it would make sense to call it 'suspicious_comparison_formatting' and put it in the style group.

@ghost
Copy link

ghost commented Jun 23, 2019

Or maybe this check can be applied to all binary operations. 🤔

@flip1995 flip1995 added the L-style Lint: Belongs in the style lint group label Jun 23, 2019
nikofil added a commit to nikofil/rust-clippy that referenced this issue Oct 3, 2019
Lints when, on the RHS of a BinOp, there is a UnOp without a space
before the operator but with a space after (e.g. foo >- 1).
(Closes rust-lang#4228)

Signed-off-by: Nikos Filippakis <[email protected]>
@nikofil
Copy link
Contributor

nikofil commented Oct 3, 2019

Hey! I made a PR about this issue to get my feet wet with this project. I hope I understood @mikerite's suggestion about doing this check on all binary operations correctly - this way makes sense to me. All feedback welcome!

@bors bors closed this as completed in 0583181 Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants