Lint for use of operator's trait's method instead of the operator itself

[meithecatte]

What it does

If somebody calls std::ops::Add::add(foo, bar), suggest foo + bar.

In my experience with Exercism, beginners often find the API they need in the trait, not realizing its relation to the operator. Naming the trait method requires an import and is usually unnecessarily verbose, so we might want to lint against it.

Categories

• Kind: clippy::style

Drawbacks

None.

Example

use std::ops::Add;
use chrono::{DateTime, Duration, Utc};

pub fn time_travel(now: DateTime<Utc>, how_long: Duration)
    -> DateTime<Utc>
{
    now.add(how_long)
}

Could be written as:

use std::ops::Add;
use chrono::{DateTime, Duration, Utc};

pub fn time_travel(now: DateTime<Utc>, how_long: Duration)
    -> DateTime<Utc>
{
    now + how_long
}

Note

This is a follow-up to #5679, which described a similar issue for FromIterator, and the case of operator traits got mentioned in the comments. The PR that solved the FromIterator case didn't handle this case, so i'm opening a separate issue.

[xFrednet]

I would try to implement this 🙃. But I have some questions beforehand if you don't mind:

1. Should the lint include all traits that have an operation bound to them or do we want to focus on a specific subset?
   • I'm in favor of having one lint for all of them. The title also suggest this.
2. Do we maybe want to add a configuration to enable/disable specific operations?
3. Should this be a style lint?

---

This is a collected list of all traits in std::ops:: that have an operator bound to them:

Operation	Assign Operation
Add -> +	(AddAssign -> +=)
Div -> \	(DivAssign -> \=)
Mul -> *	(MulAssign -> *=)
Sub -> -	(SubAssign -> -=)
Rem -> %	(RemAssign -> %=)
Shl -> <<	(ShlAssign -> <<=)
Shr -> >>	(ShrAssign -> >>=)
BitAnd -> &	(BitAndAssign -> &=)
BitOr -> |	(BitOrAssign -> |=)
BitXor -> ^	(BitXorAssign -> ^=)
Neg -> -
Not -> !
Index -> []
IndexMut -> []

Excluded from the lint:

Operation	Assign Operation
Deref -> *
DerefMut -> *

[meithecatte]

Deref feels like something one could want to use as a function. Also, isn't the operator for BitXor a ^, not ~?

[xFrednet]

The operator was wrong thank you @NieDzejkob for pointing that point. I agree that you might want to use Deref as a function. This could maybe be a new lint :).

I've updated my comment with the correct operator and the Deref operator in a new list that is excluded from this lint.

[xFrednet]

I believe that this issue needs some more discussion before it can be completely implemented/introduced.

This conversation has already shown that some users could prefer x.deref() instead of *x. Here is a similar example where someone stated that they prefer a.not() over !a. I also remember using macros in C++ for binary operators (xor, invert, or, and) when I started out. These are examples where the lint would work against the user and would probably be disabled completely.

I even see a use case for the opposite, i.e. saying: "I want to disallow the use of the ^ operator in favor of the bitxor method for readability and to keep the code consistent!". This is something we should probably discuss before implementing this lint.

---

Proposal:

We implement a lint like operator_trait_usage¹. This lint disallows the use of the operator methods by default (As this lint suggested). I would also like to include some std::ops::* traits like Eq for ==, Ord for <, ==, > and they partial counterparts.

The lint additionally includes several configurations to enable the users to set up the lint to their liking:

• <lint-name>-ignore²: Operator traits that are in this list get ignored by the lint. (Both the operator and method use is allowed)
• <lint-name>-force-method-use²: The operators of the traits in this list will be disallowed in favor of the operator method.

I believe that this proposal would cover all uncases and give the use the most freedom. However, it also feels a bit different to me than other lints because everything is configurable.

---

^1. This is a placeholder name, and it open for suggestions. It's hard to follow the naming conventions as this proposal suggest covering both sides of the lint (disallowing methods in favor of the operator and the other way around).

^2. Also, a placeholder name suggestions are welcome.

@rustbot label +C-needs-discussion