WIP: lint for the use of operator trait methods

[xFrednet]

Fixes #6286

---

This implements a lint for the operator traits (except Deref and DerefMut as discussed in the issue). This implementation is mostly working. There are a few points where I would like to get some feedback or support for:

1. I have the feeling that the lint description isn't the best. Some feedback and maybe suggestions how to improve it would be appreciated.
2. This lint could theoretically support rust-fixme quite well. But I had a problem where the conjunction of multiple trait uses causes multiple warning with overlapping spans. Fixme was unable to apply these changes due to this. Is there a simply way to fix this?

Example with warnings and logs:

The rust code:

let _ = 0xff_u8.bitand(0xf0).bitor(0x04).bitxor(0x80);

Lint output:

error: use of `bitxor` method of the operator trait `std::ops::BitXor`
  --> $DIR/use_of_operator_trait_method.rs:62:13
   |
LL |     let _ = 0xff_u8.bitand(0xf0).bitor(0x04).bitxor(0x80);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the operator like this: : `0xff_u8.bitand(0xf0).bitor(0x04) ^ 0x80`

error: use of `bitor` method of the operator trait `std::ops::BitOr`
  --> $DIR/use_of_operator_trait_method.rs:62:13
   |
LL |     let _ = 0xff_u8.bitand(0xf0).bitor(0x04).bitxor(0x80);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the operator like this: : `0xff_u8.bitand(0xf0) | 0x04`

error: use of `bitand` method of the operator trait `std::ops::BitAnd`
  --> $DIR/use_of_operator_trait_method.rs:62:13
   |
LL |     let _ = 0xff_u8.bitand(0xf0).bitor(0x04).bitxor(0x80);
   |             ^^^^^^^^^^^^^^^^^^^^ help: consider using the operator like this: : `0xff_u8 & 0xf0`

The fixme log from the CI:

---- compile_test stdout ----
thread '[ui] ui/use_of_operator_trait_method.rs' panicked at 'failed to apply suggestions for 
"tests/ui/use_of_operator_trait_method.rs" with rustfix: Could not replace range 965...996 in file
-- maybe parts of it were already replaced?

Stack backtrace:
   0: anyhow::private::new_adhoc
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/anyhow-1.0.35/src/lib.rs:623:36
   1: rustfix::replace::Data::replace_range::{{closure}}
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/rustfix-0.5.1/src/replace.rs:136:21
   2: core::option::Option<T>::ok_or_else
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/option.rs:561:25
error: test failed, to rerun pass '--test compile-test'
   3: rustfix::replace::Data::replace_range
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/rustfix-0.5.1/src/replace.rs:106:42
   4: rustfix::CodeFix::apply
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/rustfix-0.5.1/src/lib.rs:247:17
   5: rustfix::apply_suggestions
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/rustfix-0.5.1/src/lib.rs:265:9
   6: compiletest_rs::runtest::TestCx::run_ui_test
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:2312:30
   7: compiletest_rs::runtest::TestCx::run_revision
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:143:19
   8: compiletest_rs::runtest::run
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:83:9
   9: compiletest_rs::make_test_closure::{{closure}}
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/lib.rs:296:9
  10: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
  11: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/alloc/src/boxed.rs:1328:9
  12: tester::__rust_begin_short_backtrace
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/tester-0.7.0/src/lib.rs:543:5
  13: tester::run_test::{{closure}}
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/tester-0.7.0/src/lib.rs:527:34
  14: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
  15: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/alloc/src/boxed.rs:1328:9
  16: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panic.rs:322:9
  17: std::panicking::try::do_call
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:379:40
  18: __rust_try
  19: std::panicking::try
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:343:19
  20: std::panic::catch_unwind
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panic.rs:396:14
  21: tester::run_test_in_process
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/tester-0.7.0/src/lib.rs:578:18
  22: tester::run_test::run_test_inner::{{closure}}
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/tester-0.7.0/src/lib.rs:474:21
  23: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/sys_common/backtrace.rs:125:18
  24: std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/thread/mod.rs:474:17
  25: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panic.rs:322:9
  26: std::panicking::try::do_call
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:379:40
  27: __rust_try
  28: std::panicking::try
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:343:19
  29: std::panic::catch_unwind
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panic.rs:396:14
  30: std::thread::Builder::spawn_unchecked::{{closure}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/thread/mod.rs:473:30
  31: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
  32: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/alloc/src/boxed.rs:1328:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/alloc/src/boxed.rs:1328:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/sys/unix/thread.rs:71:17
  33: start_thread
  34: __clone', /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:2312:77
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/panicking.rs:92:14
   2: core::option::expect_none_failed
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/option.rs:1268:5
   3: core::result::Result<T,E>::expect
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/result.rs:933:23
   4: compiletest_rs::runtest::TestCx::run_ui_test
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:2312:30
   5: compiletest_rs::runtest::TestCx::run_revision
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:143:19
   6: compiletest_rs::runtest::run
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/runtest.rs:83:9
   7: compiletest_rs::make_test_closure::{{closure}}
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/lib.rs:296:9
   8: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
   9: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/alloc/src/boxed.rs:1328:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread 'compile_test' panicked at 'Some tests failed', /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/lib.rs:105:22
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/std/src/panicking.rs:519:12
   1: compiletest_rs::run_tests
             at /home/runner/.cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/compiletest_rs-0.5.0/src/lib.rs:105:22
   2: compile_test::run_mode
             at ./tests/compile-test.rs:99:5
   3: compile_test::compile_test
             at ./tests/compile-test.rs:265:5
   4: compile_test::compile_test::{{closure}}
             at ./tests/compile-test.rs:262:1
   5: core::ops::function::FnOnce::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/5e6e1e33a11d140a4d70f946730137f241224eb3/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

3. The implementation is currently an all or nothing lint. Should it maybe also support a setting to enable/disable specific method use validations?
   • There can be cases where the use of an method like a.bitxor(b) can be seen as more readable than a ^ b. Having a setting could enable the user to specify which method uses should be linted and which not.
   • This would also cover the case of Deref and DerefMut.
4. I could also add support for the std::cmp::* traits like PartialOrd with the current structure. Should I maybe add support for them under a different lint name in this PR as well? 🙃
5. I'm not sure if this trait is enabled by default and if it should be. Are there any suggestions how this should be handled?

---

It has been fun working on this project and I'm impressed by the awesome util functions that are available. Thank you for the nice structure of the crate! 🙃

---

changelog: Added a lint for the use of operator trait methods (use_of_operator_trait_method)

[rust-highfive]

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

[xFrednet]

Hey @flip1995, I have some questions about this implementation. I've noted them in the PR description. Could you maybe help me with them? 🙃

[xFrednet]

There is a Zupli comment about some people preferring the .not method over the not operator ! (comment). This is another good example.

It might be worth to change this lint or create a new one that let's the user define for which which operators should be used and which methods. They could for instance define that you should always use the add operator + over the method but the .not() method over the operator !. Just a thought

[xFrednet]

I think that this still needs some discussion as I noted in the PR and the description. I'll close the PR and write up an overview on the issue to have it all bundled in one place :)

[flip1995]

Sorry for taking so long with reviewing. I was busy during the week with work stuff and on the weekend with other Clippy stuff, like the roadmap. Having this discussion in the issue first SGTM.

[xFrednet]

No problem, I got the vibe that you've been quite busy from your other contributions. It was actually good with this as it gave me some time to rethink it and to learn a bit more about provided utility within clippy. The code here is not really optimal ^^. Let me know when I can help in some other way :)