mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix needless_borrow suggestion when calling a trait method taking self

Browse Source
This commit is contained in:
Jason Newcomb 2022-01-29 20:29:43 -05:00
parent 0b4ba734cb
commit 6d21b79be9
4 changed files with 117 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
clippy_lints/src/dereference.rs
View File

@ -11,11 +11,13 @@
ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem,
TraitItemKind, TyKind, UnOp,
};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeckResults};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{symbol::sym, Span, Symbol};
use rustc_trait_selection::infer::InferCtxtExt;
declare_clippy_lint! {
/// ### What it does
@ -165,7 +167,6 @@ struct StateData {
struct DerefedBorrow {
count: usize,
required_precedence: i8,
msg: &'static str,
position: Position,
}
@ -329,19 +330,19 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
"this expression creates a reference which is immediately dereferenced by the compiler";
let borrow_msg = "this expression borrows a value the compiler would automatically borrow";
let (required_refs, required_precedence, msg) = if position.can_auto_borrow() {
(1, PREC_POSTFIX, if deref_count == 1 { borrow_msg } else { deref_msg })
let (required_refs, msg) = if position.can_auto_borrow() {
(1, if deref_count == 1 { borrow_msg } else { deref_msg })
} else if let Some(&Adjust::Borrow(AutoBorrow::Ref(_, mutability))) =
next_adjust.map(|a| &a.kind)
{
if matches!(mutability, AutoBorrowMutability::Mut { .. }) && !position.is_reborrow_stable()
{
(3, 0, deref_msg)
(3, deref_msg)
} else {
(2, 0, deref_msg)
(2, deref_msg)
}
} else {
(2, 0, deref_msg)
(2, deref_msg)
};
if deref_count >= required_refs {
@ -350,7 +351,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// One of the required refs is for the current borrow expression, the remaining ones
// can't be removed without breaking the code. See earlier comment.
count: deref_count - required_refs,
required_precedence,
msg,
position,
}),
@ -601,6 +601,8 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool {
#[derive(Clone, Copy)]
enum Position {
MethodReceiver,
/// The method is defined on a reference type. e.g. `impl Foo for &T`
MethodReceiverRefImpl,
Callee,
FieldAccess(Symbol),
Postfix,
@ -627,6 +629,13 @@ fn can_auto_borrow(self) -> bool {
fn lint_explicit_deref(self) -> bool {
matches!(self, Self::Other | Self::DerefStable | Self::ReborrowStable)
}
fn needs_parens(self, precedence: i8) -> bool {
matches!(
self,
Self::MethodReceiver | Self::MethodReceiverRefImpl | Self::Callee | Self::FieldAccess(_) | Self::Postfix
) && precedence < PREC_POSTFIX
}
}
/// Walks up the parent expressions attempting to determine both how stable the auto-deref result
@ -730,10 +739,34 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap();
args.iter().position(|arg| arg.hir_id == child_id).map(|i| {
if i == 0 {
if e.hir_id == child_id {
Position::MethodReceiver
} else {
// Check for calls to trait methods where the trait is implemented on a reference.
// Two cases need to be handled:
// * `self` methods on `&T` will never have auto-borrow
// * `&self` methods on `&T` can have auto-borrow, but `&self` methods on `T` will take
// priority.
if e.hir_id != child_id {
Position::ReborrowStable
} else if let Some(trait_id) = cx.tcx.trait_of_item(id)
&& let arg_ty = cx.tcx.erase_regions(cx.typeck_results().expr_ty_adjusted(e))
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
&& let subs = cx.typeck_results().node_substs_opt(child_id).unwrap_or_else(
|| cx.tcx.mk_substs([].iter())
) && let impl_ty = if cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref() {
// Trait methods taking `&self`
sub_ty
} else {
// Trait methods taking `self`
arg_ty
} && impl_ty.is_ref()
&& cx.tcx.infer_ctxt().enter(|infcx|
infcx
.type_implements_trait(trait_id, impl_ty, subs, cx.param_env)
.must_apply_modulo_regions()
)
{
Position::MethodReceiverRefImpl
} else {
Position::MethodReceiver
}
} else {
param_auto_deref_stability(cx.tcx.fn_sig(id).skip_binder().inputs()[i])
@ -964,7 +997,7 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0;
span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, state.msg, |diag| {
let sugg = if state.required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) {
let sugg = if state.position.needs_parens(expr.precedence().order()) && !has_enclosing_paren(&snip) {
format!("({})", snip)
} else {
snip.into()

30
tests/ui/needless_borrow.fixed
View File

@ -85,6 +85,36 @@ fn main() {
let _ = x.0;
let x = &x as *const (i32, i32);
let _ = unsafe { (*x).0 };
// Issue #8367
trait Foo {
fn foo(self);
}
impl Foo for &'_ () {
fn foo(self) {}
}
(&()).foo(); // Don't lint. `()` doesn't implement `Foo`
(&()).foo();
impl Foo for i32 {
fn foo(self) {}
}
impl Foo for &'_ i32 {
fn foo(self) {}
}
(&5).foo(); // Don't lint. `5` will call `<i32 as Foo>::foo`
(&5).foo();
trait FooRef {
fn foo_ref(&self);
}
impl FooRef for () {
fn foo_ref(&self) {}
}
impl FooRef for &'_ () {
fn foo_ref(&self) {}
}
(&&()).foo_ref(); // Don't lint. `&()` will call `<() as FooRef>::foo_ref`
}
#[allow(clippy::needless_borrowed_reference)]

30
tests/ui/needless_borrow.rs
View File

@ -85,6 +85,36 @@ fn ref_mut_i32(_: &mut i32) {}
let _ = (&x).0;
let x = &x as *const (i32, i32);
let _ = unsafe { (&*x).0 };
// Issue #8367
trait Foo {
fn foo(self);
}
impl Foo for &'_ () {
fn foo(self) {}
}
(&()).foo(); // Don't lint. `()` doesn't implement `Foo`
(&&()).foo();
impl Foo for i32 {
fn foo(self) {}
}
impl Foo for &'_ i32 {
fn foo(self) {}
}
(&5).foo(); // Don't lint. `5` will call `<i32 as Foo>::foo`
(&&5).foo();
trait FooRef {
fn foo_ref(&self);
}
impl FooRef for () {
fn foo_ref(&self) {}
}
impl FooRef for &'_ () {
fn foo_ref(&self) {}
}
(&&()).foo_ref(); // Don't lint. `&()` will call `<() as FooRef>::foo_ref`
}
#[allow(clippy::needless_borrowed_reference)]

14
tests/ui/needless_borrow.stderr
View File

@ -108,5 +108,17 @@ error: this expression borrows a value the compiler would automatically borrow
LL | let _ = unsafe { (&*x).0 };
| ^^^^^ help: change this to: `(*x)`
error: aborting due to 18 previous errors
error: this expression creates a reference which is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:97:5
|
LL | (&&()).foo();
| ^^^^^^ help: change this to: `(&())`
error: this expression creates a reference which is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:106:5
|
LL | (&&5).foo();
| ^^^^^ help: change this to: `(&5)`
error: aborting due to 20 previous errors