mirror of
https://github.com/rust-lang/rust-clippy
synced 2025-02-17 06:28:42 +00:00
Check for Deref trait impl + add fixed version
This commit is contained in:
parent
c1132434a7
commit
b6d4330550
5 changed files with 149 additions and 40 deletions
|
@ -1,4 +1,6 @@
|
|||
use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg};
|
||||
use crate::utils::{
|
||||
get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg,
|
||||
};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
|
@ -15,18 +17,19 @@ declare_clippy_lint! {
|
|||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// let b = a.deref();
|
||||
/// let c = a.deref_mut();
|
||||
/// use std::ops::Deref;
|
||||
/// let a: &mut String = &mut String::from("foo");
|
||||
/// let b: &str = a.deref();
|
||||
/// ```
|
||||
/// Could be written as:
|
||||
/// ```rust
|
||||
/// let a: &mut String = &mut String::from("foo");
|
||||
/// let b = &*a;
|
||||
/// let c = &mut *a;
|
||||
/// ```
|
||||
///
|
||||
/// This lint excludes
|
||||
/// ```rust
|
||||
/// let e = d.unwrap().deref();
|
||||
/// ```rust,ignore
|
||||
/// let _ = d.unwrap().deref();
|
||||
/// ```
|
||||
pub EXPLICIT_DEREF_METHOD,
|
||||
pedantic,
|
||||
|
@ -49,7 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
for arg in args {
|
||||
if_chain! {
|
||||
// Caller must call only one other function (deref or deref_mut)
|
||||
// otherwise it can lead to error prone suggestions (ex: &*a.len())
|
||||
// otherwise it can lead to error prone suggestions (ie: &*a.len())
|
||||
let (method_names, arg_list, _) = method_calls(arg, 2);
|
||||
if method_names.len() == 1;
|
||||
// Caller must be a variable
|
||||
|
@ -59,7 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
|
||||
then {
|
||||
let name = method_names[0].as_str();
|
||||
lint_deref(cx, &*name, variables[0].span, arg.span);
|
||||
lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -72,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
if args.len() == 1;
|
||||
if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
|
||||
// Caller must call only one other function (deref or deref_mut)
|
||||
// otherwise it can lead to error prone suggestions (ex: &*a.len())
|
||||
// otherwise it can lead to error prone suggestions (ie: &*a.len())
|
||||
let (method_names, arg_list, _) = method_calls(init, 2);
|
||||
if method_names.len() == 1;
|
||||
// Caller must be a variable
|
||||
|
@ -82,7 +85,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
|
||||
then {
|
||||
let name = method_name.ident.as_str();
|
||||
lint_deref(cx, &*name, args[0].span, init.span);
|
||||
lint_deref(cx, &*name, init, args[0].span, init.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -96,16 +99,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
if_chain! {
|
||||
if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
|
||||
if args.len() == 1;
|
||||
if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
|
||||
if let Some(parent) = get_parent_expr(cx, &expr);
|
||||
|
||||
then {
|
||||
// Call and MethodCall exprs are better reported using statements
|
||||
match parent.kind {
|
||||
ExprKind::Call(_, _) => return,
|
||||
ExprKind::MethodCall(_, _, _) => return,
|
||||
// Already linted using statements
|
||||
ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (),
|
||||
_ => {
|
||||
let name = method_name.ident.as_str();
|
||||
lint_deref(cx, &*name, args[0].span, expr.span);
|
||||
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -113,29 +116,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
|
|||
}
|
||||
}
|
||||
|
||||
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) {
|
||||
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
|
||||
match fn_name {
|
||||
"deref" => {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
EXPLICIT_DEREF_METHOD,
|
||||
expr_span,
|
||||
"explicit deref method call",
|
||||
"try this",
|
||||
format!("&*{}", &snippet(cx, var_span, "..")),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
if get_trait_def_id(cx, &paths::DEREF_TRAIT)
|
||||
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
EXPLICIT_DEREF_METHOD,
|
||||
expr_span,
|
||||
"explicit deref method call",
|
||||
"try this",
|
||||
format!("&*{}", &snippet(cx, var_span, "..")),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
},
|
||||
"deref_mut" => {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
EXPLICIT_DEREF_METHOD,
|
||||
expr_span,
|
||||
"explicit deref_mut method call",
|
||||
"try this",
|
||||
format!("&mut *{}", &snippet(cx, var_span, "..")),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT)
|
||||
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
EXPLICIT_DEREF_METHOD,
|
||||
expr_span,
|
||||
"explicit deref_mut method call",
|
||||
"try this",
|
||||
format!("&mut *{}", &snippet(cx, var_span, "..")),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
|
|
|
@ -25,7 +25,9 @@ pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"];
|
|||
pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"];
|
||||
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
|
||||
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
|
||||
pub const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
|
||||
pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
|
||||
pub const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
|
||||
pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
|
||||
pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
|
||||
pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
|
||||
|
|
79
tests/ui/dereference.fixed
Normal file
79
tests/ui/dereference.fixed
Normal file
|
@ -0,0 +1,79 @@
|
|||
// run-rustfix
|
||||
|
||||
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
|
||||
#![warn(clippy::explicit_deref_method)]
|
||||
|
||||
use std::ops::{Deref, DerefMut};
|
||||
|
||||
fn concat(deref_str: &str) -> String {
|
||||
format!("{}bar", deref_str)
|
||||
}
|
||||
|
||||
fn just_return(deref_str: &str) -> &str {
|
||||
deref_str
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let a: &mut String = &mut String::from("foo");
|
||||
|
||||
// these should require linting
|
||||
|
||||
let b: &str = &*a;
|
||||
|
||||
let b: &mut str = &mut *a;
|
||||
|
||||
// both derefs should get linted here
|
||||
let b: String = format!("{}, {}", &*a, &*a);
|
||||
|
||||
println!("{}", &*a);
|
||||
|
||||
#[allow(clippy::match_single_binding)]
|
||||
match &*a {
|
||||
_ => (),
|
||||
}
|
||||
|
||||
let b: String = concat(&*a);
|
||||
|
||||
// following should not require linting
|
||||
|
||||
let b = just_return(a).deref();
|
||||
|
||||
let b: String = concat(just_return(a).deref());
|
||||
|
||||
let b: String = a.deref().clone();
|
||||
|
||||
let b: usize = a.deref_mut().len();
|
||||
|
||||
let b: &usize = &a.deref().len();
|
||||
|
||||
let b: &str = a.deref().deref();
|
||||
|
||||
let b: &str = &*a;
|
||||
|
||||
let b: &mut str = &mut *a;
|
||||
|
||||
macro_rules! expr_deref {
|
||||
($body:expr) => {
|
||||
$body.deref()
|
||||
};
|
||||
}
|
||||
let b: &str = expr_deref!(a);
|
||||
|
||||
let opt_a = Some(a);
|
||||
let b = opt_a.unwrap().deref();
|
||||
|
||||
// The struct does not implement Deref trait
|
||||
#[derive(Copy, Clone)]
|
||||
struct NoLint(u32);
|
||||
impl NoLint {
|
||||
pub fn deref(self) -> u32 {
|
||||
self.0
|
||||
}
|
||||
pub fn deref_mut(self) -> u32 {
|
||||
self.0
|
||||
}
|
||||
}
|
||||
let no_lint = NoLint(42);
|
||||
let b = no_lint.deref();
|
||||
let b = no_lint.deref_mut();
|
||||
}
|
|
@ -1,3 +1,5 @@
|
|||
// run-rustfix
|
||||
|
||||
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
|
||||
#![warn(clippy::explicit_deref_method)]
|
||||
|
||||
|
@ -59,4 +61,19 @@ fn main() {
|
|||
|
||||
let opt_a = Some(a);
|
||||
let b = opt_a.unwrap().deref();
|
||||
|
||||
// The struct does not implement Deref trait
|
||||
#[derive(Copy, Clone)]
|
||||
struct NoLint(u32);
|
||||
impl NoLint {
|
||||
pub fn deref(self) -> u32 {
|
||||
self.0
|
||||
}
|
||||
pub fn deref_mut(self) -> u32 {
|
||||
self.0
|
||||
}
|
||||
}
|
||||
let no_lint = NoLint(42);
|
||||
let b = no_lint.deref();
|
||||
let b = no_lint.deref_mut();
|
||||
}
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:19:19
|
||||
--> $DIR/dereference.rs:21:19
|
||||
|
|
||||
LL | let b: &str = a.deref();
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
@ -7,37 +7,37 @@ LL | let b: &str = a.deref();
|
|||
= note: `-D clippy::explicit-deref-method` implied by `-D warnings`
|
||||
|
||||
error: explicit deref_mut method call
|
||||
--> $DIR/dereference.rs:21:23
|
||||
--> $DIR/dereference.rs:23:23
|
||||
|
|
||||
LL | let b: &mut str = a.deref_mut();
|
||||
| ^^^^^^^^^^^^^ help: try this: `&mut *a`
|
||||
|
||||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:24:39
|
||||
--> $DIR/dereference.rs:26:39
|
||||
|
|
||||
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
||||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:24:50
|
||||
--> $DIR/dereference.rs:26:50
|
||||
|
|
||||
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
||||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:26:20
|
||||
--> $DIR/dereference.rs:28:20
|
||||
|
|
||||
LL | println!("{}", a.deref());
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
||||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:29:11
|
||||
--> $DIR/dereference.rs:31:11
|
||||
|
|
||||
LL | match a.deref() {
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
||||
error: explicit deref method call
|
||||
--> $DIR/dereference.rs:33:28
|
||||
--> $DIR/dereference.rs:35:28
|
||||
|
|
||||
LL | let b: String = concat(a.deref());
|
||||
| ^^^^^^^^^ help: try this: `&*a`
|
||||
|
|
Loading…
Add table
Reference in a new issue