From 6b4ab827469529f4eda5f1e9492abcb9ad9d209a Mon Sep 17 00:00:00 2001 From: ThibsG Date: Thu, 23 Jan 2020 16:28:01 +0100 Subject: [PATCH] Global rework + fix imports --- CHANGELOG.md | 1 + clippy_lints/src/dereference.rs | 60 ++++++++++++++++++--------------- clippy_lints/src/lib.rs | 7 ++-- src/lintlist/mod.rs | 7 ++++ tests/ui/dereference.rs | 59 ++++++++++++-------------------- tests/ui/dereference.stderr | 48 +++++++++++++------------- 6 files changed, 89 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac3cace20..eb5a6af97d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1256,6 +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_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 diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index c29c0d466d1..dea00e5aa3b 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,47 +1,51 @@ -use crate::rustc::hir::{Expr, ExprKind, QPath}; -use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use crate::rustc::{declare_tool_lint, lint_array}; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_errors::Applicability; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, declare_lint_pass}; use crate::utils::{in_macro, span_lint_and_sugg}; use if_chain::if_chain; -/// **What it does:** Checks for explicit deref() or deref_mut() method calls. -/// -/// **Why is this bad?** Derefencing by &*x or &mut *x is clearer and more concise, -/// when not part of a method chain. -/// -/// **Example:** -/// ```rust -/// let b = a.deref(); -/// let c = a.deref_mut(); -/// -/// // excludes -/// let e = d.unwrap().deref(); -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls. + /// + /// **Why is this bad?** Derefencing by `&*x` or `&mut *x` is clearer and more concise, + /// when not part of a method chain. + /// + /// **Example:** + /// ```rust + /// let b = a.deref(); + /// let c = a.deref_mut(); + /// ``` + /// Could be written as: + /// ```rust + /// let b = &*a; + /// let c = &mut *a; + /// ``` + /// + /// This lint excludes + /// ```rust + /// let e = d.unwrap().deref(); + /// ``` pub EXPLICIT_DEREF_METHOD, pedantic, "Explicit use of deref or deref_mut method while not in a method chain." } -pub struct Pass; +declare_lint_pass!(Dereferencing => [ + EXPLICIT_DEREF_METHOD +]); -impl LintPass for Pass { - fn get_lints(&self) -> LintArray { - lint_array!(EXPLICIT_DEREF_METHOD) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { - fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if in_macro(expr.span) { return; } if_chain! { // if this is a method call - if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.node; + if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind; // on a Path (i.e. a variable/name, not another method) - if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node; + if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; then { let name = method_name.ident.as_str(); // alter help slightly to account for _mut @@ -54,6 +58,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { "explicit deref method call", "try this", format!("&*{}", path), + Applicability::MachineApplicable ); }, "deref_mut" => { @@ -64,6 +69,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { "explicit deref_mut method call", "try this", format!("&mut *{}", path), + Applicability::MachineApplicable ); }, _ => () diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a6ddd6573a8..6443caa89d6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -391,7 +391,7 @@ pub fn read_conf(args: &[rustc_ast::ast::NestedMetaItem], sess: &Session) -> Con } conf - } + }, Err((err, span)) => { sess.struct_span_err(span, err) .span_note(span, "Clippy will use default configuration") @@ -513,7 +513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ©_iterator::COPY_ITERATOR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, - &dereference::DEREF_METHOD_EXPLICIT, + &dereference::EXPLICIT_DEREF_METHOD, &derive::DERIVE_HASH_XOR_EQ, &derive::EXPL_IMPL_CLONE_ON_COPY, &doc::DOC_MARKDOWN, @@ -1040,7 +1040,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); - store.register_late_pass(|| box dereference::DerefMethodExplicit); + store.register_late_pass(|| box dereference::Dereferencing); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1181,7 +1181,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), - LintId::of(&dereference::EXPLICIT_DEREF_METHOD), LintId::of(&derive::DERIVE_HASH_XOR_EQ), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 935ea180ebe..3f1d31f0302 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -528,6 +528,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "loops", }, + Lint { + name: "explicit_deref_method", + group: "pedantic", + desc: "Explicit use of deref or deref_mut method while not in a method chain.", + deprecation: None, + module: "dereference", + }, Lint { name: "explicit_into_iter_loop", group: "pedantic", diff --git a/tests/ui/dereference.rs b/tests/ui/dereference.rs index 7800cd84c24..07421eb715d 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/dereference.rs @@ -1,55 +1,38 @@ -#![feature(tool_lints)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![warn(clippy::explicit_deref_method)] use std::ops::{Deref, DerefMut}; -#[allow(clippy::many_single_char_names, clippy::clone_double_ref)] -#[allow(unused_variables)] -#[warn(clippy::explicit_deref_method)] fn main() { let a: &mut String = &mut String::from("foo"); // these should require linting - { - let b: &str = a.deref(); - } + let b: &str = a.deref(); - { - let b: &mut str = a.deref_mut(); - } + let b: &mut str = a.deref_mut(); - { - let b: String = a.deref().clone(); - } - - { - let b: usize = a.deref_mut().len(); - } - - { - let b: &usize = &a.deref().len(); - } + let b: String = a.deref().clone(); - { - // only first deref should get linted here - let b: &str = a.deref().deref(); - } + let b: usize = a.deref_mut().len(); - { - // both derefs should get linted here - let b: String = format!("{}, {}", a.deref(), a.deref()); - } + let b: &usize = &a.deref().len(); + + // only first deref should get linted here + let b: &str = a.deref().deref(); + + // both derefs should get linted here + let b: String = format!("{}, {}", a.deref(), a.deref()); // these should not require linting - { - let b: &str = &*a; - } - { - let b: &mut str = &mut *a; - } + let b: &str = &*a; - { - macro_rules! expr_deref { ($body:expr) => { $body.deref() } } - let b: &str = expr_deref!(a); + let b: &mut str = &mut *a; + + macro_rules! expr_deref { + ($body:expr) => { + $body.deref() + }; } + let b: &str = expr_deref!(a); } diff --git a/tests/ui/dereference.stderr b/tests/ui/dereference.stderr index a4c2487d06b..7169b689a86 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/dereference.stderr @@ -1,52 +1,52 @@ error: explicit deref method call - --> $DIR/dereference.rs:13:23 + --> $DIR/dereference.rs:10:19 | -13 | let b: &str = a.deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &str = a.deref(); + | ^^^^^^^^^ help: try this: `&*a` | = note: `-D clippy::explicit-deref-method` implied by `-D warnings` error: explicit deref_mut method call - --> $DIR/dereference.rs:17:27 + --> $DIR/dereference.rs:12:23 | -17 | let b: &mut str = a.deref_mut(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: &mut str = a.deref_mut(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:21:25 + --> $DIR/dereference.rs:14:21 | -21 | let b: String = a.deref().clone(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = a.deref().clone(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref_mut method call - --> $DIR/dereference.rs:25:24 + --> $DIR/dereference.rs:16:20 | -25 | let b: usize = a.deref_mut().len(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` +LL | let b: usize = a.deref_mut().len(); + | ^^^^^^^^^^^^^ help: try this: `&mut *a` error: explicit deref method call - --> $DIR/dereference.rs:29:26 + --> $DIR/dereference.rs:18:22 | -29 | let b: &usize = &a.deref().len(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &usize = &a.deref().len(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:34:23 + --> $DIR/dereference.rs:21:19 | -34 | let b: &str = a.deref().deref(); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: &str = a.deref().deref(); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:39:43 + --> $DIR/dereference.rs:24:39 | -39 | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: explicit deref method call - --> $DIR/dereference.rs:39:54 + --> $DIR/dereference.rs:24:50 | -39 | let b: String = format!("{}, {}", a.deref(), a.deref()); - | ^^^^^^^^^ help: try this: `&*a` +LL | let b: String = format!("{}, {}", a.deref(), a.deref()); + | ^^^^^^^^^ help: try this: `&*a` error: aborting due to 8 previous errors