Skip to content

Fix suggestion on fully qualified syntax #10855

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 4, 2023
Merged

Fix suggestion on fully qualified syntax #10855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8188da3
Fix suggestion on fully qualified syntax
Centri3 Jun 1, 2023
eed4662
ignore `Foo::deref` altogether
Centri3 Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions clippy_lints/src/dereference.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ declare_clippy_lint! {
/// 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,
Expand Down Expand Up @@ -1480,7 +1482,7 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
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 {
Expand All @@ -1503,11 +1505,20 @@ 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 {
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, do not lint if it's `Foo::deref` instead of `foo.deref()`.
if is_final_ufcs {
return;
}

span_lint_and_sugg(
cx,
Expand Down
30 changes: 29 additions & 1 deletion tests/ui/explicit_deref_methods.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//@run-rustfix
#![warn(clippy::explicit_deref_methods)]
#![allow(unused_variables)]
#![allow(unused_variables, unused_must_use)]
#![allow(
clippy::borrow_deref_ref,
suspicious_double_ref_op,
clippy::explicit_auto_deref,
clippy::needless_borrow,
clippy::no_effect,
clippy::uninlined_format_args
)]

Expand All @@ -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() {
let a: &mut String = &mut String::from("foo");

Expand Down Expand Up @@ -58,6 +75,17 @@ fn main() {
let opt_a = Some(a.clone());
let b = &*opt_a.unwrap();

// 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);
<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

let cv = CustomVec(vec![0, 42]);
Expand Down
30 changes: 29 additions & 1 deletion tests/ui/explicit_deref_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//@run-rustfix
#![warn(clippy::explicit_deref_methods)]
#![allow(unused_variables)]
#![allow(unused_variables, unused_must_use)]
#![allow(
clippy::borrow_deref_ref,
suspicious_double_ref_op,
clippy::explicit_auto_deref,
clippy::needless_borrow,
clippy::no_effect,
clippy::uninlined_format_args
)]

Expand All @@ -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() {
let a: &mut String = &mut String::from("foo");

Expand Down Expand Up @@ -58,6 +75,17 @@ fn main() {
let opt_a = Some(a.clone());
let b = opt_a.unwrap().deref();

// 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);
<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

let cv = CustomVec(vec![0, 42]);
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/explicit_deref_methods.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,73 +1,73 @@
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();
| ^^^^^^^^^ help: try this: `&*a`
|
= note: `-D clippy::explicit-deref-methods` implied by `-D warnings`

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();
| ^^^^^^^^^^^^^ help: try this: `&mut **a`

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());
| ^^^^^^^^^ help: try this: `&*a`

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());
| ^^^^^^^^^ help: try this: `&*a`

error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:43:20
--> $DIR/explicit_deref_methods.rs:60:20
|
LL | println!("{}", a.deref());
| ^^^^^^^^^ help: try this: `&*a`

error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:46:11
--> $DIR/explicit_deref_methods.rs:63:11
|
LL | match a.deref() {
| ^^^^^^^^^ help: try this: `&*a`

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());
| ^^^^^^^^^ help: try this: `&*a`

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();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)`

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());
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)`

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();
| ^^^^^^^^^^^^^^^^^ help: try this: `&**a`

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();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`

error: explicit `deref` method call
--> $DIR/explicit_deref_methods.rs:85:31
--> $DIR/explicit_deref_methods.rs:113:31
|
LL | let b: &str = expr_deref!(a.deref());
| ^^^^^^^^^ help: try this: `&*a`
Expand Down