Use only check_expr with parent expr and precedence

This commit is contained in:
ThibsG 2020-03-07 15:33:27 +01:00
parent b6d4330550
commit 72b9ae2a10
8 changed files with 79 additions and 100 deletions

View File

@ -1256,7 +1256,7 @@ Released 2018-09-13
[`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call
[`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
[`explicit_deref_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_method
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
[`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
[`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
[`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write

View File

@ -1,10 +1,8 @@
use crate::utils::{
get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg,
};
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::{Expr, ExprKind, QPath, StmtKind};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
@ -31,100 +29,52 @@ declare_clippy_lint! {
/// ```rust,ignore
/// let _ = d.unwrap().deref();
/// ```
pub EXPLICIT_DEREF_METHOD,
pub EXPLICIT_DEREF_METHODS,
pedantic,
"Explicit use of deref or deref_mut method while not in a method chain."
}
declare_lint_pass!(Dereferencing => [
EXPLICIT_DEREF_METHOD
EXPLICIT_DEREF_METHODS
]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) {
if_chain! {
if let StmtKind::Local(ref local) = stmt.kind;
if let Some(ref init) = local.init;
then {
match init.kind {
ExprKind::Call(ref _method, args) => {
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 (ie: &*a.len())
let (method_names, arg_list, _) = method_calls(arg, 2);
if method_names.len() == 1;
// Caller must be a variable
let variables = arg_list[0];
if variables.len() == 1;
if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
then {
let name = method_names[0].as_str();
lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span);
}
}
}
}
ExprKind::MethodCall(ref method_name, _, ref args) => {
if init.span.from_expansion() {
return;
}
if_chain! {
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 (ie: &*a.len())
let (method_names, arg_list, _) = method_calls(init, 2);
if method_names.len() == 1;
// Caller must be a variable
let variables = arg_list[0];
if variables.len() == 1;
if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
then {
let name = method_name.ident.as_str();
lint_deref(cx, &*name, init, args[0].span, init.span);
}
}
}
_ => ()
}
}
}
}
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if !expr.span.from_expansion();
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 {
match parent.kind {
// Already linted using statements
ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (),
_ => {
if let Some(parent_expr) = get_parent_expr(cx, expr) {
// Check if we have the whole call chain here
if let ExprKind::MethodCall(..) = parent_expr.kind {
return;
}
// Check for unary precedence
if let ExprPrecedence::Unary = parent_expr.precedence() {
return;
}
}
let name = method_name.ident.as_str();
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
}
}
}
}
}
}
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
match fn_name {
fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
match method_name {
"deref" => {
if get_trait_def_id(cx, &paths::DEREF_TRAIT)
if cx
.tcx
.lang_items()
.deref_trait()
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
{
span_lint_and_sugg(
cx,
EXPLICIT_DEREF_METHOD,
EXPLICIT_DEREF_METHODS,
expr_span,
"explicit deref method call",
"try this",
@ -134,12 +84,15 @@ fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var
}
},
"deref_mut" => {
if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT)
if cx
.tcx
.lang_items()
.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,
EXPLICIT_DEREF_METHODS,
expr_span,
"explicit deref_mut method call",
"try this",

View File

@ -513,7 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&copy_iterator::COPY_ITERATOR,
&dbg_macro::DBG_MACRO,
&default_trait_access::DEFAULT_TRAIT_ACCESS,
&dereference::EXPLICIT_DEREF_METHOD,
&dereference::EXPLICIT_DEREF_METHODS,
&derive::DERIVE_HASH_XOR_EQ,
&derive::EXPL_IMPL_CLONE_ON_COPY,
&doc::DOC_MARKDOWN,
@ -1091,7 +1091,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
LintId::of(&copy_iterator::COPY_ITERATOR),
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
LintId::of(&dereference::EXPLICIT_DEREF_METHOD),
LintId::of(&dereference::EXPLICIT_DEREF_METHODS),
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
LintId::of(&doc::DOC_MARKDOWN),
LintId::of(&doc::MISSING_ERRORS_DOC),

View File

@ -25,9 +25,7 @@ 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"];

View File

@ -529,7 +529,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
module: "loops",
},
Lint {
name: "explicit_deref_method",
name: "explicit_deref_methods",
group: "pedantic",
desc: "Explicit use of deref or deref_mut method while not in a method chain.",
deprecation: None,

View File

@ -1,7 +1,7 @@
// run-rustfix
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
#![warn(clippy::explicit_deref_method)]
#![warn(clippy::explicit_deref_methods)]
use std::ops::{Deref, DerefMut};
@ -34,11 +34,18 @@ fn main() {
let b: String = concat(&*a);
let b = &*just_return(a);
let b: String = concat(&*just_return(a));
let b: &str = &*a.deref();
let opt_a = Some(a.clone());
let b = &*opt_a.unwrap();
// following should not require linting
let b = just_return(a).deref();
let b: String = concat(just_return(a).deref());
let b: &str = &*a.deref();
let b: String = a.deref().clone();
@ -46,8 +53,6 @@ fn main() {
let b: &usize = &a.deref().len();
let b: &str = a.deref().deref();
let b: &str = &*a;
let b: &mut str = &mut *a;
@ -59,9 +64,6 @@ fn main() {
}
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);

View File

@ -1,7 +1,7 @@
// run-rustfix
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
#![warn(clippy::explicit_deref_method)]
#![warn(clippy::explicit_deref_methods)]
use std::ops::{Deref, DerefMut};
@ -34,20 +34,25 @@ fn main() {
let b: String = concat(a.deref());
// following should not require linting
let b = just_return(a).deref();
let b: String = concat(just_return(a).deref());
let b: &str = a.deref().deref();
let opt_a = Some(a.clone());
let b = opt_a.unwrap().deref();
// following should not require linting
let b: &str = &*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;
@ -59,9 +64,6 @@ fn main() {
}
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);

View File

@ -4,7 +4,7 @@ error: explicit deref method call
LL | let b: &str = a.deref();
| ^^^^^^^^^ help: try this: `&*a`
|
= note: `-D clippy::explicit-deref-method` implied by `-D warnings`
= note: `-D clippy::explicit-deref-methods` implied by `-D warnings`
error: explicit deref_mut method call
--> $DIR/dereference.rs:23:23
@ -42,5 +42,29 @@ error: explicit deref method call
LL | let b: String = concat(a.deref());
| ^^^^^^^^^ help: try this: `&*a`
error: aborting due to 7 previous errors
error: explicit deref method call
--> $DIR/dereference.rs:37:13
|
LL | let b = just_return(a).deref();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)`
error: explicit deref method call
--> $DIR/dereference.rs:39:28
|
LL | let b: String = concat(just_return(a).deref());
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)`
error: explicit deref method call
--> $DIR/dereference.rs:41:19
|
LL | let b: &str = a.deref().deref();
| ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()`
error: explicit deref method call
--> $DIR/dereference.rs:44:13
|
LL | let b = opt_a.unwrap().deref();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`
error: aborting due to 11 previous errors