Skip to content

Commit eed4662

Browse files
Centri3Centri3
committed
ignore Foo::deref altogether
1 parent 8188da3 commit eed4662

File tree

4 files changed

+30
-75
lines changed

4 files changed

+30
-75
lines changed

‎clippy_lints/src/dereference.rs

+18-29
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ declare_clippy_lint! {
5656
/// let b = &*a;
5757
/// ```
5858
///
59-
/// This lint excludes:
59+
/// This lint excludes all of:
6060
/// ```rust,ignore
6161
/// let _ = d.unwrap().deref();
62+
/// let _ = Foo::deref(&foo);
63+
/// let _ = <Foo as Deref>::deref(&foo);
6264
/// ```
6365
#[clippy::version = "1.44.0"]
6466
pub EXPLICIT_DEREF_METHODS,
@@ -155,7 +157,7 @@ impl_lint_pass!(Dereferencing<'_> => [
155157

156158
#[derive(Default)]
157159
pub struct Dereferencing<'tcx> {
158-
state: Option<(State<'tcx>, StateData)>,
160+
state: Option<(State, StateData)>,
159161

160162
// While parsing a `deref` method call in ufcs form, the path to the function is itself an
161163
// expression. This is to store the id of that expression so it can be skipped when
@@ -210,13 +212,12 @@ struct DerefedBorrow {
210212
}
211213

212214
#[derive(Debug)]
213-
enum State<'tcx> {
215+
enum State {
214216
// Any number of deref method calls.
215217
DerefMethod {
216218
// The number of calls in a sequence which changed the referenced type
217219
ty_changed_count: usize,
218220
is_final_ufcs: bool,
219-
call_args: Option<&'tcx [Expr<'tcx>]>,
220221
/// The required mutability
221222
target_mut: Mutability,
222223
},
@@ -313,16 +314,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
313314
&& position.lint_explicit_deref() =>
314315
{
315316
let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)));
316-
let (is_final_ufcs, call_args) = if let ExprKind::Call(_, args) = expr.kind {
317-
(true, Some(args))
318-
} else {
319-
(false, None)
320-
};
321317
self.state = Some((
322318
State::DerefMethod {
323319
ty_changed_count,
324-
is_final_ufcs,
325-
call_args,
320+
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
326321
target_mut,
327322
},
328323
StateData {
@@ -445,7 +440,6 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
445440
ty_changed_count + 1
446441
},
447442
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
448-
call_args: None,
449443
target_mut,
450444
},
451445
data,
@@ -1480,16 +1474,15 @@ fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool {
14801474
}
14811475

14821476
#[expect(clippy::needless_pass_by_value, clippy::too_many_lines)]
1483-
fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State<'_>, data: StateData) {
1477+
fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) {
14841478
match state {
14851479
State::DerefMethod {
14861480
ty_changed_count,
14871481
is_final_ufcs,
1488-
call_args,
14891482
target_mut,
14901483
} => {
14911484
let mut app = Applicability::MachineApplicable;
1492-
let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
1485+
let (expr_str, _expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
14931486
let ty = cx.typeck_results().expr_ty(expr);
14941487
let (_, ref_count) = peel_mid_ty_refs(ty);
14951488
let deref_str = if ty_changed_count >= ref_count && ref_count != 0 {
@@ -1512,23 +1505,19 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State<'_>,
15121505
"&"
15131506
};
15141507

1515-
let mut expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX {
1516-
format!("({expr_str})")
1508+
// expr_str (the suggestion) is never shown if is_final_ufcs is true, since it's
1509+
// `expr.kind == ExprKind::Call`. Therefore, this is, afaik, always unnecessary.
1510+
/*
1511+
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX {
1512+
Cow::Owned(format!("({expr_str})"))
15171513
} else {
1518-
expr_str.into_owned()
1514+
expr_str
15191515
};
1516+
*/
15201517

1521-
// Fix #10850, changes suggestion if it's `Foo::deref` instead of `foo.deref`. Since `Foo::deref` is
1522-
// a `Call` instead of a `MethodCall` this should catch all instances of this, even if it's fully
1523-
// qualified or whatnot.
1524-
if is_final_ufcs && let Some(args) = call_args {
1525-
// Remove ref if it's there
1526-
let arg = if let ExprKind::AddrOf(.., arg) = args[0].kind {
1527-
arg
1528-
} else {
1529-
&args[0]
1530-
};
1531-
expr_str = snippet_with_applicability(cx, arg.span, "{ .. }", &mut app).to_string();
1518+
// Fix #10850, do not lint if it's `Foo::deref` instead of `foo.deref()`.
1519+
if is_final_ufcs {
1520+
return;
15321521
}
15331522

15341523
span_lint_and_sugg(

‎tests/ui/explicit_deref_methods.fixed

+8-7
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,16 @@ fn main() {
7575
let opt_a = Some(a.clone());
7676
let b = &*opt_a.unwrap();
7777

78-
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
78+
// make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified
79+
// syntax
7980

80-
&*Aaa;
81-
&mut *Aaa;
82-
&*Aaa;
83-
&mut *Aaa;
81+
Aaa::deref(&Aaa);
82+
Aaa::deref_mut(&mut Aaa);
83+
<Aaa as Deref>::deref(&Aaa);
84+
<Aaa as DerefMut>::deref_mut(&mut Aaa);
8485
let mut aaa = Aaa;
85-
&*aaa;
86-
&mut *aaa;
86+
Aaa::deref(&aaa);
87+
Aaa::deref_mut(&mut aaa);
8788

8889
// following should not require linting
8990

‎tests/ui/explicit_deref_methods.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ fn main() {
7575
let opt_a = Some(a.clone());
7676
let b = opt_a.unwrap().deref();
7777

78-
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
78+
// make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified
79+
// syntax
7980

8081
Aaa::deref(&Aaa);
8182
Aaa::deref_mut(&mut Aaa);

‎tests/ui/explicit_deref_methods.stderr

+2-38
Original file line numberDiff line numberDiff line change
@@ -67,46 +67,10 @@ LL | let b = opt_a.unwrap().deref();
6767
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`
6868

6969
error: explicit `deref` method call
70-
--> $DIR/explicit_deref_methods.rs:80:5
71-
|
72-
LL | Aaa::deref(&Aaa);
73-
| ^^^^^^^^^^^^^^^^ help: try this: `&*Aaa`
74-
75-
error: explicit `deref_mut` method call
76-
--> $DIR/explicit_deref_methods.rs:81:5
77-
|
78-
LL | Aaa::deref_mut(&mut Aaa);
79-
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa`
80-
81-
error: explicit `deref` method call
82-
--> $DIR/explicit_deref_methods.rs:82:5
83-
|
84-
LL | <Aaa as Deref>::deref(&Aaa);
85-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*Aaa`
86-
87-
error: explicit `deref_mut` method call
88-
--> $DIR/explicit_deref_methods.rs:83:5
89-
|
90-
LL | <Aaa as DerefMut>::deref_mut(&mut Aaa);
91-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa`
92-
93-
error: explicit `deref` method call
94-
--> $DIR/explicit_deref_methods.rs:85:5
95-
|
96-
LL | Aaa::deref(&aaa);
97-
| ^^^^^^^^^^^^^^^^ help: try this: `&*aaa`
98-
99-
error: explicit `deref_mut` method call
100-
--> $DIR/explicit_deref_methods.rs:86:5
101-
|
102-
LL | Aaa::deref_mut(&mut aaa);
103-
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *aaa`
104-
105-
error: explicit `deref` method call
106-
--> $DIR/explicit_deref_methods.rs:112:31
70+
--> $DIR/explicit_deref_methods.rs:113:31
10771
|
10872
LL | let b: &str = expr_deref!(a.deref());
10973
| ^^^^^^^^^ help: try this: `&*a`
11074

111-
error: aborting due to 18 previous errors
75+
error: aborting due to 12 previous errors
11276

0 commit comments

Comments
 (0)