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

precedence should not warn for unary minus before odd function #4892

Closed
mad-s opened this issue Dec 9, 2019 · 4 comments
Closed

precedence should not warn for unary minus before odd function #4892

mad-s opened this issue Dec 9, 2019 · 4 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy

Comments

@mad-s
Copy link

mad-s commented Dec 9, 2019

The clippy:precedence list currently warns on every instance of the form -literal.method(), as precedence looks ambiguous. However, for an odd function, i.e. a function f(x) where f(-x)=-f(x), the precedence doesn't matter. Many functions have this property, such as sin(), atan(), cbrt(), to_radians() (and round() i think).

@flip1995 flip1995 added C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy labels Dec 22, 2019
@flip1995
Copy link
Member

Basically only a whitelist of functions has to be added.

@ghost
Copy link

ghost commented Apr 7, 2020

Hi! Can I work on this issue ? (first issue/contribution). Thanks.

@flip1995
Copy link
Member

flip1995 commented Apr 8, 2020

Sure! To get started with Clippy development you can read up in https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md. If you have any questions, just ask here or open a WIP PR.

@ghost ghost mentioned this issue Apr 10, 2020
bors added a commit that referenced this issue Apr 17, 2020
Fixes issue #4892.

First contribution here 😊 ! Do not hesitate to correct me.

This PR is related to issue #4892 .

# Summary

```rust
-literal.method_call(args)
```
The main idea is to not trigger `clippy:precedence` when the method call is an odd function.

# Example

```rust
// should trigger lint
let _ = -1.0_f64.abs() //precedence of method call abs() and neg ('-') is ambiguous

// should not trigger lint
let _ = -1.0_f64.sin() // sin is an odd function => -sin(x) = sin(-x)
```

# Theory

Rust allows following literals:
- char
- string
- integers
- floats
- byte
- bool

Only integers/floats implements the relevant `std::ops::Neg`.
Following odd functions are implemented on i[8-128] and/or f[32-64]:
- `asin`
- `asinh`
- `atan`
- `atanh`
- `cbrt`
- `fract`
- `round`
- `signum`
- `sin`
- `sinh`
- `tan`
- `tanh `
- `to_degrees`
- `to_radians`

# Implementation

As suggested by `flip1995` in [comment](#4892 (comment)), this PR add a whitelist of odd functions and compare method call to the the whitelist before triggering lint.
bors added a commit that referenced this issue Apr 17, 2020
Fixes issue #4892.

First contribution here 😊 ! Do not hesitate to correct me.

This PR is related to issue #4892 .

# Summary

```rust
-literal.method_call(args)
```
The main idea is to not trigger `clippy::precedence` when the method call is an odd function.

# Example

```rust
// should trigger lint
let _ = -1.0_f64.abs() //precedence of method call abs() and neg ('-') is ambiguous

// should not trigger lint
let _ = -1.0_f64.sin() // sin is an odd function => -sin(x) = sin(-x)
```

# Theory

Rust allows following literals:
- char
- string
- integers
- floats
- byte
- bool

Only integers/floats implements the relevant `std::ops::Neg`.
Following odd functions are implemented on i[8-128] and/or f[32-64]:
- `asin`
- `asinh`
- `atan`
- `atanh`
- `cbrt`
- `fract`
- `round`
- `signum`
- `sin`
- `sinh`
- `tan`
- `tanh `
- `to_degrees`
- `to_radians`

# Implementation

As suggested by `flip1995` in [comment](#4892 (comment)), this PR add a whitelist of odd functions and compare method call to the the whitelist before triggering lint.

changelog: Don't trigger [`clippy::precedence`] on odd functions.
@ghost
Copy link

ghost commented Apr 25, 2020

Fix was merged. Is it possible to close this issue pls ?

flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue May 5, 2020
Changes:
````
Polished lint and tests
Added final lint and tests
Added basic lint and tests
fix redundant_pattern_matching lint
add lint futures_not_send
Integrate more idiomatic rust changes.
Fix issue rust-lang#4892.
cargo dev fmt
Cleanup: Rename 'db' variable to 'diag'
question_mark: don't add `as_ref()` for a call expression
unit_arg suggestion may be incorrect
readme: update to cargo clippy --fix command
CI: performing system upgrade fixes broken apt deps on ubuntu 32bit
Do not lint in macros for match lints
[fix] Minor typo in GH Actions 'clippy_dev'
Reenable rustfmt integration test
Add test to map_flatten with an Option
Lint map_flatten if caller is an Option
Apply suggestions from code review
manually fixing formatting at this point lol
fmt
rename field
revert the damn fmt changes
add some tests
split it up for testing but the merge broke tests
dogfood tasted bad
fix rustfmt issue
boycott manish
check for unstable options
Start work on clippy-fix as subcommand
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

2 participants