Fix suggestion on fully qualified syntax

This commit is contained in:
Centri3 2023-05-31 23:33:30 -05:00
parent 652b4c720d
commit 8188da3614
4 changed files with 132 additions and 20 deletions

View File

@ -155,7 +155,7 @@ impl_lint_pass!(Dereferencing<'_> => [
#[derive(Default)] #[derive(Default)]
pub struct Dereferencing<'tcx> { pub struct Dereferencing<'tcx> {
state: Option<(State, StateData)>, state: Option<(State<'tcx>, StateData)>,
// While parsing a `deref` method call in ufcs form, the path to the function is itself an // While parsing a `deref` method call in ufcs form, the path to the function is itself an
// expression. This is to store the id of that expression so it can be skipped when // expression. This is to store the id of that expression so it can be skipped when
@ -210,12 +210,13 @@ struct DerefedBorrow {
} }
#[derive(Debug)] #[derive(Debug)]
enum State { enum State<'tcx> {
// Any number of deref method calls. // Any number of deref method calls.
DerefMethod { DerefMethod {
// The number of calls in a sequence which changed the referenced type // The number of calls in a sequence which changed the referenced type
ty_changed_count: usize, ty_changed_count: usize,
is_final_ufcs: bool, is_final_ufcs: bool,
call_args: Option<&'tcx [Expr<'tcx>]>,
/// The required mutability /// The required mutability
target_mut: Mutability, target_mut: Mutability,
}, },
@ -312,10 +313,16 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
&& position.lint_explicit_deref() => && position.lint_explicit_deref() =>
{ {
let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr))); let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)));
let (is_final_ufcs, call_args) = if let ExprKind::Call(_, args) = expr.kind {
(true, Some(args))
} else {
(false, None)
};
self.state = Some(( self.state = Some((
State::DerefMethod { State::DerefMethod {
ty_changed_count, ty_changed_count,
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), is_final_ufcs,
call_args,
target_mut, target_mut,
}, },
StateData { StateData {
@ -438,6 +445,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
ty_changed_count + 1 ty_changed_count + 1
}, },
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
call_args: None,
target_mut, target_mut,
}, },
data, data,
@ -1472,11 +1480,12 @@ fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool {
} }
#[expect(clippy::needless_pass_by_value, clippy::too_many_lines)] #[expect(clippy::needless_pass_by_value, clippy::too_many_lines)]
fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State<'_>, data: StateData) {
match state { match state {
State::DerefMethod { State::DerefMethod {
ty_changed_count, ty_changed_count,
is_final_ufcs, is_final_ufcs,
call_args,
target_mut, target_mut,
} => { } => {
let mut app = Applicability::MachineApplicable; let mut app = Applicability::MachineApplicable;
@ -1503,12 +1512,25 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
"&" "&"
}; };
let expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { let mut expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX {
format!("({expr_str})") format!("({expr_str})")
} else { } else {
expr_str.into_owned() expr_str.into_owned()
}; };
// Fix #10850, changes suggestion if it's `Foo::deref` instead of `foo.deref`. Since `Foo::deref` is
// a `Call` instead of a `MethodCall` this should catch all instances of this, even if it's fully
// qualified or whatnot.
if is_final_ufcs && let Some(args) = call_args {
// Remove ref if it's there
let arg = if let ExprKind::AddrOf(.., arg) = args[0].kind {
arg
} else {
&args[0]
};
expr_str = snippet_with_applicability(cx, arg.span, "{ .. }", &mut app).to_string();
}
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
EXPLICIT_DEREF_METHODS, EXPLICIT_DEREF_METHODS,

View File

@ -1,11 +1,12 @@
//@run-rustfix //@run-rustfix
#![warn(clippy::explicit_deref_methods)] #![warn(clippy::explicit_deref_methods)]
#![allow(unused_variables)] #![allow(unused_variables, unused_must_use)]
#![allow( #![allow(
clippy::borrow_deref_ref, clippy::borrow_deref_ref,
suspicious_double_ref_op, suspicious_double_ref_op,
clippy::explicit_auto_deref, clippy::explicit_auto_deref,
clippy::needless_borrow, clippy::needless_borrow,
clippy::no_effect,
clippy::uninlined_format_args clippy::uninlined_format_args
)] )]
@ -28,6 +29,22 @@ impl Deref for CustomVec {
} }
} }
struct Aaa;
impl Deref for Aaa {
type Target = ();
fn deref(&self) -> &Self::Target {
todo!();
}
}
impl DerefMut for Aaa {
fn deref_mut(&mut self) -> &mut Self::Target {
todo!();
}
}
fn main() { fn main() {
let a: &mut String = &mut String::from("foo"); let a: &mut String = &mut String::from("foo");
@ -58,6 +75,16 @@ fn main() {
let opt_a = Some(a.clone()); let opt_a = Some(a.clone());
let b = &*opt_a.unwrap(); let b = &*opt_a.unwrap();
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
&*Aaa;
&mut *Aaa;
&*Aaa;
&mut *Aaa;
let mut aaa = Aaa;
&*aaa;
&mut *aaa;
// following should not require linting // following should not require linting
let cv = CustomVec(vec![0, 42]); let cv = CustomVec(vec![0, 42]);

View File

@ -1,11 +1,12 @@
//@run-rustfix //@run-rustfix
#![warn(clippy::explicit_deref_methods)] #![warn(clippy::explicit_deref_methods)]
#![allow(unused_variables)] #![allow(unused_variables, unused_must_use)]
#![allow( #![allow(
clippy::borrow_deref_ref, clippy::borrow_deref_ref,
suspicious_double_ref_op, suspicious_double_ref_op,
clippy::explicit_auto_deref, clippy::explicit_auto_deref,
clippy::needless_borrow, clippy::needless_borrow,
clippy::no_effect,
clippy::uninlined_format_args clippy::uninlined_format_args
)] )]
@ -28,6 +29,22 @@ impl Deref for CustomVec {
} }
} }
struct Aaa;
impl Deref for Aaa {
type Target = ();
fn deref(&self) -> &Self::Target {
todo!();
}
}
impl DerefMut for Aaa {
fn deref_mut(&mut self) -> &mut Self::Target {
todo!();
}
}
fn main() { fn main() {
let a: &mut String = &mut String::from("foo"); let a: &mut String = &mut String::from("foo");
@ -58,6 +75,16 @@ fn main() {
let opt_a = Some(a.clone()); let opt_a = Some(a.clone());
let b = opt_a.unwrap().deref(); let b = opt_a.unwrap().deref();
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
Aaa::deref(&Aaa);
Aaa::deref_mut(&mut Aaa);
<Aaa as Deref>::deref(&Aaa);
<Aaa as DerefMut>::deref_mut(&mut Aaa);
let mut aaa = Aaa;
Aaa::deref(&aaa);
Aaa::deref_mut(&mut aaa);
// following should not require linting // following should not require linting
let cv = CustomVec(vec![0, 42]); let cv = CustomVec(vec![0, 42]);

View File

@ -1,5 +1,5 @@
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:36:19 --> $DIR/explicit_deref_methods.rs:53:19
| |
LL | let b: &str = a.deref(); LL | let b: &str = a.deref();
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
@ -7,70 +7,106 @@ LL | let b: &str = a.deref();
= note: `-D clippy::explicit-deref-methods` implied by `-D warnings` = note: `-D clippy::explicit-deref-methods` implied by `-D warnings`
error: explicit `deref_mut` method call error: explicit `deref_mut` method call
--> $DIR/explicit_deref_methods.rs:38:23 --> $DIR/explicit_deref_methods.rs:55:23
| |
LL | let b: &mut str = a.deref_mut(); LL | let b: &mut str = a.deref_mut();
| ^^^^^^^^^^^^^ help: try this: `&mut **a` | ^^^^^^^^^^^^^ help: try this: `&mut **a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:41:39 --> $DIR/explicit_deref_methods.rs:58:39
| |
LL | let b: String = format!("{}, {}", a.deref(), a.deref()); LL | let b: String = format!("{}, {}", a.deref(), a.deref());
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:41:50 --> $DIR/explicit_deref_methods.rs:58:50
| |
LL | let b: String = format!("{}, {}", a.deref(), a.deref()); LL | let b: String = format!("{}, {}", a.deref(), a.deref());
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:43:20 --> $DIR/explicit_deref_methods.rs:60:20
| |
LL | println!("{}", a.deref()); LL | println!("{}", a.deref());
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:46:11 --> $DIR/explicit_deref_methods.rs:63:11
| |
LL | match a.deref() { LL | match a.deref() {
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:50:28 --> $DIR/explicit_deref_methods.rs:67:28
| |
LL | let b: String = concat(a.deref()); LL | let b: String = concat(a.deref());
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:52:13 --> $DIR/explicit_deref_methods.rs:69:13
| |
LL | let b = just_return(a).deref(); LL | let b = just_return(a).deref();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:54:28 --> $DIR/explicit_deref_methods.rs:71:28
| |
LL | let b: String = concat(just_return(a).deref()); LL | let b: String = concat(just_return(a).deref());
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:56:19 --> $DIR/explicit_deref_methods.rs:73:19
| |
LL | let b: &str = a.deref().deref(); LL | let b: &str = a.deref().deref();
| ^^^^^^^^^^^^^^^^^ help: try this: `&**a` | ^^^^^^^^^^^^^^^^^ help: try this: `&**a`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:59:13 --> $DIR/explicit_deref_methods.rs:76:13
| |
LL | let b = opt_a.unwrap().deref(); LL | let b = opt_a.unwrap().deref();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`
error: explicit `deref` method call error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:85:31 --> $DIR/explicit_deref_methods.rs:80:5
|
LL | Aaa::deref(&Aaa);
| ^^^^^^^^^^^^^^^^ help: try this: `&*Aaa`
error: explicit `deref_mut` method call
--> $DIR/explicit_deref_methods.rs:81:5
|
LL | Aaa::deref_mut(&mut Aaa);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa`
error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:82:5
|
LL | <Aaa as Deref>::deref(&Aaa);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*Aaa`
error: explicit `deref_mut` method call
--> $DIR/explicit_deref_methods.rs:83:5
|
LL | <Aaa as DerefMut>::deref_mut(&mut Aaa);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa`
error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:85:5
|
LL | Aaa::deref(&aaa);
| ^^^^^^^^^^^^^^^^ help: try this: `&*aaa`
error: explicit `deref_mut` method call
--> $DIR/explicit_deref_methods.rs:86:5
|
LL | Aaa::deref_mut(&mut aaa);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *aaa`
error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:112:31
| |
LL | let b: &str = expr_deref!(a.deref()); LL | let b: &str = expr_deref!(a.deref());
| ^^^^^^^^^ help: try this: `&*a` | ^^^^^^^^^ help: try this: `&*a`
error: aborting due to 12 previous errors error: aborting due to 18 previous errors