ignore Foo::deref altogether

This commit is contained in:
Centri3 2023-06-01 14:29:56 -05:00
parent 8188da3614
commit eed466281c
4 changed files with 30 additions and 75 deletions

View File

@ -56,9 +56,11 @@
/// let b = &*a;
/// ```
///
/// This lint excludes:
/// This lint excludes all of:
/// ```rust,ignore
/// let _ = d.unwrap().deref();
/// let _ = Foo::deref(&foo);
/// let _ = <Foo as Deref>::deref(&foo);
/// ```
#[clippy::version = "1.44.0"]
pub EXPLICIT_DEREF_METHODS,
@ -155,7 +157,7 @@
#[derive(Default)]
pub struct Dereferencing<'tcx> {
state: Option<(State<'tcx>, StateData)>,
state: Option<(State, StateData)>,
// 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
@ -210,13 +212,12 @@ struct DerefedBorrow {
}
#[derive(Debug)]
enum State<'tcx> {
enum State {
// Any number of deref method calls.
DerefMethod {
// The number of calls in a sequence which changed the referenced type
ty_changed_count: usize,
is_final_ufcs: bool,
call_args: Option<&'tcx [Expr<'tcx>]>,
/// The required mutability
target_mut: Mutability,
},
@ -313,16 +314,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
&& position.lint_explicit_deref() =>
{
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((
State::DerefMethod {
ty_changed_count,
is_final_ufcs,
call_args,
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
target_mut,
},
StateData {
@ -445,7 +440,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
ty_changed_count + 1
},
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
call_args: None,
target_mut,
},
data,
@ -1480,16 +1474,15 @@ fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool {
}
#[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 {
State::DerefMethod {
ty_changed_count,
is_final_ufcs,
call_args,
target_mut,
} => {
let mut app = Applicability::MachineApplicable;
let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
let (expr_str, _expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
let ty = cx.typeck_results().expr_ty(expr);
let (_, ref_count) = peel_mid_ty_refs(ty);
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<'_>,
"&"
};
let mut expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX {
format!("({expr_str})")
// expr_str (the suggestion) is never shown if is_final_ufcs is true, since it's
// `expr.kind == ExprKind::Call`. Therefore, this is, afaik, always unnecessary.
/*
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX {
Cow::Owned(format!("({expr_str})"))
} else {
expr_str.into_owned()
expr_str
};
*/
// 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();
// Fix #10850, do not lint if it's `Foo::deref` instead of `foo.deref()`.
if is_final_ufcs {
return;
}
span_lint_and_sugg(

View File

@ -75,15 +75,16 @@ fn main() {
let opt_a = Some(a.clone());
let b = &*opt_a.unwrap();
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
// make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified
// syntax
&*Aaa;
&mut *Aaa;
&*Aaa;
&mut *Aaa;
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;
&mut *aaa;
Aaa::deref(&aaa);
Aaa::deref_mut(&mut aaa);
// following should not require linting

View File

@ -75,7 +75,8 @@ fn main() {
let opt_a = Some(a.clone());
let b = opt_a.unwrap().deref();
// make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax
// make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified
// syntax
Aaa::deref(&Aaa);
Aaa::deref_mut(&mut Aaa);

View File

@ -67,46 +67,10 @@ LL | let b = opt_a.unwrap().deref();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`
error: explicit `deref` method call
--> $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
--> $DIR/explicit_deref_methods.rs:113:31
|
LL | let b: &str = expr_deref!(a.deref());
| ^^^^^^^^^ help: try this: `&*a`
error: aborting due to 18 previous errors
error: aborting due to 12 previous errors