-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add lint explicit_auto_deref
take 2
#8355
Conversation
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
☔ The latest upstream changes (presumably #8359) made this pull request unmergeable. Please resolve the merge conflicts. |
ba3112f
to
a31eaad
Compare
☔ The latest upstream changes (presumably #8289) made this pull request unmergeable. Please resolve the merge conflicts. |
Currently it is not directly possible to configure lints for the entire workspace via TOML, which forced us to repeat `#![allow]` blocks in each crate. embark pointed out this workaround to configure lints at the workspace level via RUSTFLAGS: EmbarkStudios/rust-ecosystem#22 (comment) Remove the common `#![allow]` blocks and switch to this method for global lint config. Temporarily allow `needless_borrow` lint, buggy pending this fix: rust-lang/rust-clippy#8355
Currently it is not directly possible to configure lints for the entire workspace via TOML, which forced us to repeat `#![allow]` blocks in each crate. embark pointed out this workaround to configure lints at the workspace level via RUSTFLAGS: EmbarkStudios/rust-ecosystem#22 (comment) Remove the common `#![allow]` blocks and switch to this method for global lint config. Temporarily allow `needless_borrow` lint, buggy pending this fix: rust-lang/rust-clippy#8355
3d37661
to
5a6a6d6
Compare
Does this also fix this case? pub fn put_u16(v: u16, out: &mut [u8]) {
let out: &mut [u8; 2] = (&mut out[..2]).try_into().unwrap();
*out = u16::to_be_bytes(v);
} Warning: --> rustls/src/msgs/codec.rs:91:29
|
91 | let out: &mut [u8; 2] = (&mut out[..2]).try_into().unwrap();
| ^^^^^^^^^^^^^^^ help: change this to: `out[..2]`
|
= note: `#[warn(clippy::needless_borrow)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. r=me after rebase
☔ The latest upstream changes (presumably #9007) made this pull request unmergeable. Please resolve the merge conflicts. |
1d935dd
to
dd3d747
Compare
☔ The latest upstream changes (presumably #8639) made this pull request unmergeable. Please resolve the merge conflicts. |
Oh no, another instance, where GitHub didn't show me a notification for a force push. @Jarcho please approve this PR yourself with |
`needless_borrow` will now walk further to find the target type.
Merge `Position` and `AutoDerefStability`
dd3d747
to
68596aa
Compare
68596aa
to
5e2a2d3
Compare
@bors r=flip1995 |
📌 Commit 5e2a2d3 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Though rust-lang/rust-clippy#8355 has been merged, it seems to still report false-positives on nightly channel. For now just fix the instances reported by stable clippy, and keep `needless_borrow` allowed.
Though rust-lang/rust-clippy#8355 has been merged, it seems to still report false-positives on nightly channel. For now just fix the instances reported by stable clippy, and keep `needless_borrow` allowed.
fixes: #234
fixes: #8367
fixes: #8380
Still things to do:
This currently only lints&*<expr>
when it doesn't triggerneedless_borrow
.This requires a borrow after a deref to trigger. So*<expr>
changing&&T
to&T
won't be caught.deref
andderef_mut
trait methods aren't linted.field accesses, nor method receivers are linted.This probably shouldn't lint reborrowing.&vec[..]
when just&vec
would dochangelog: new lint
explicit_auto_deref