feat(lint): deref and deref_mut method explicit calls

[tommilligan]

Instead of calling x.deref() or x.deref_mut(), &*x and &mut *x should be recommended instead.

Probably should not lint if it's part of a method chain

This PR adds a lint deref_method_explicit (all, complexity), which fulfils the above criteria. Matching ui test and output have also been added.

I'm not certain how best to handle macro expansions as part of the lint - if a macro contains a deref() call for instance. If someone could provide a failing testcase for me to work against that would be great.

Closes #1566

[ghost]

Maybe this should be a pedantic lint to be consistent with explicit_iter_loop and explicit_into_iter_loop. See #3063 and #3119.

[tommilligan]

@mikerite thanks for the feedback. I'll also rename to explicit_deref_method for consistency.

Could you advise on why travis-ci and appveyor disagree, and which to trust? The issue appears to be a fix for my local build failing when I first cloned the repo:

858a902#diff-5b47b8bbc8833f6268e936ea8682644dL93

edit: updated my local nightly version

[flip1995]

Travis and appveyor check if clippy compiles with rustc master, not with rustc nightly. You can check this locally with

rustup-toolchain-install-master -f -n master
cargo +master test

See https://github.com/rust-lang-nursery/rust-clippy/blob/master/CONTRIBUTING.md#fixing-build-failures-caused-by-rust

Currently Travis fails because of

+++./util/update_lints.py -c
Please run util/update_lints.py to regenerate lints lists.

(This only gets checked in travis)

Also could you leave the requested comment in #3230, after this gets merged?

[flip1995]

Does your lint exclude these cases? Since _.deref().len() gets linted, I have a feeling, that also _.unwrap() will get linted.

[tommilligan]

Quite correct - should be the other way round. d.unwrap().deref() would not be linted.

[flip1995]

Could you also add a test for this?

[tommilligan]

https://github.com/rust-lang-nursery/rust-clippy/pull/3258/files#diff-280822c8730d5eac6da4c4b86a9a0fd6R34

https://github.com/rust-lang-nursery/rust-clippy/pull/3258/files#diff-21b45ffb0b35b75693c82b44153ebd60R36

In a method chain, deref will only be linted if it is the first method in the chain.

[flip1995]

Oh yeah, I missed that, thanks!

[flip1995]

This lint produces suggestions, that lead to erroneous code: Playground

[tommilligan]

Hmm, thanks for catching. Technically, we could suggest (&mut *a).len(); but that looks awful.

I need to alter the linting to only pick up on deref when it's the last method, rather than the first in the chain. I think this will require a Visitor implementation, so I'll look into that unless there are other suggestions.

[flip1995]

ping @tommilligan

If you don't have the motivation to fix the suggestion (this can take a while to do), you can replace the span_lint_and_sugg functions with
https://github.com/rust-lang-nursery/rust-clippy/blob/319b75c75b65985b7d3389d65a49f993ef3345fa/clippy_lints/src/utils/mod.rs#L522-L528
and this lint would be good to go. Having a lint without a suggestion is way better than not having the lint at all!

span_help_and_lint is not autofixable by rustfix, so having erroneous suggestions there does not really matter.

[tommilligan]

@flip1995 thanks for the ping. I believe my current implementation is actively unhelpful in some circumstances, so I'm not happy releasing it into the wild as is. I'll try to come back and revisit soon

[flip1995]

Ping from triage @tommilligan. Do you want to continue working on this?

[flip1995]

Ping from triage @tommilligan: It looks like this PR hasn't received any updates in a while, so I'm closing it per our new guidelines. Thank you for your contributions and please feel free to re-open in the future.

[jonboh]

@rustbot label -S-inactive-closed