Auto merge of #7596 - lengyijun:option_needless_deref, r=llogiq

New lint: option_needless_deref

changelog: [`option_needless_deref`]
fix #7571

Author: bors

CHANGELOG.md

@@ -2823,6 +2823,7 @@ Released 2018-09-13
 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
 [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each
 [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
+[`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref
 [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
 [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
 [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop

clippy_lints/src/lib.rs

@@ -290,6 +290,7 @@ mod needless_borrow;
 mod needless_borrowed_ref;
 mod needless_continue;
 mod needless_for_each;
+mod needless_option_as_deref;
 mod needless_pass_by_value;
 mod needless_question_mark;
 mod needless_update;
@@ -849,6 +850,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
         needless_continue::NEEDLESS_CONTINUE,
         needless_for_each::NEEDLESS_FOR_EACH,
+        needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF,
         needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
         needless_question_mark::NEEDLESS_QUESTION_MARK,
         needless_update::NEEDLESS_UPDATE,
@@ -1377,6 +1379,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(needless_bool::NEEDLESS_BOOL),
         LintId::of(needless_borrow::NEEDLESS_BORROW),
         LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
+        LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
         LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
         LintId::of(needless_update::NEEDLESS_UPDATE),
         LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
@@ -1640,6 +1643,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(needless_bool::BOOL_COMPARISON),
         LintId::of(needless_bool::NEEDLESS_BOOL),
         LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
+        LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
         LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
         LintId::of(needless_update::NEEDLESS_UPDATE),
         LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
@@ -1867,6 +1871,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(ptr::Ptr));
     store.register_late_pass(|| Box::new(ptr_eq::PtrEq));
     store.register_late_pass(|| Box::new(needless_bool::NeedlessBool));
+    store.register_late_pass(|| Box::new(needless_option_as_deref::OptionNeedlessDeref));
     store.register_late_pass(|| Box::new(needless_bool::BoolComparison));
     store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach));
     store.register_late_pass(|| Box::new(misc::MiscLints));

clippy_lints/src/loops/never_loop.rs

@@ -87,7 +87,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
 
 fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
     let stmts = block.stmts.iter().map(stmt_to_expr);
-    let expr = once(block.expr.as_deref());
+    let expr = once(block.expr);
     let mut iter = stmts.chain(expr).flatten();
     never_loop_expr_seq(&mut iter, main_loop_id)
 }
@@ -100,7 +100,7 @@ fn never_loop_expr_seq<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_lo
 fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
     match stmt.kind {
         StmtKind::Semi(e, ..) | StmtKind::Expr(e, ..) => Some(e),
-        StmtKind::Local(local) => local.init.as_deref(),
+        StmtKind::Local(local) => local.init,
         StmtKind::Item(..) => None,
     }
 }

clippy_lints/src/needless_option_as_deref.rs

@@ -0,0 +1,66 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::in_macro;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TyS;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for no-op uses of Option::{as_deref,as_deref_mut},
+    /// for example, `Option<&T>::as_deref()` returns the same type.
+    ///
+    /// ### Why is this bad?
+    /// Redundant code and improving readability.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let a = Some(&1);
+    /// let b = a.as_deref(); // goes from Option<&i32> to Option<&i32>
+    /// ```
+    /// Could be written as:
+    /// ```rust
+    /// let a = Some(&1);
+    /// let b = a;
+    /// ```
+    pub NEEDLESS_OPTION_AS_DEREF,
+    complexity,
+    "no-op use of `deref` or `deref_mut` method to `Option`."
+}
+
+declare_lint_pass!(OptionNeedlessDeref=> [
+    NEEDLESS_OPTION_AS_DEREF,
+]);
+
+impl<'tcx> LateLintPass<'tcx> for OptionNeedlessDeref {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if expr.span.from_expansion() || in_macro(expr.span) {
+            return;
+        }
+        let typeck = cx.typeck_results();
+        let outer_ty = typeck.expr_ty(expr);
+
+        if_chain! {
+            if is_type_diagnostic_item(cx,outer_ty,sym::option_type);
+            if let ExprKind::MethodCall(path, _, [sub_expr], _) = expr.kind;
+            let symbol = path.ident.as_str();
+            if symbol=="as_deref" || symbol=="as_deref_mut";
+            if TyS::same_type( outer_ty, typeck.expr_ty(sub_expr) );
+            then{
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_OPTION_AS_DEREF,
+                    expr.span,
+                    "derefed type is same as origin",
+                    "try this",
+                    snippet_opt(cx,sub_expr.span).unwrap(),
+                    Applicability::MachineApplicable
+                );
+            }
+        }
+    }
+}

tests/ui/needless_option_as_deref.fixed

@@ -0,0 +1,13 @@
+// run-rustfix
+
+#[warn(clippy::needless_option_as_deref)]
+
+fn main() {
+    // should lint
+    let _: Option<&usize> = Some(&1);
+    let _: Option<&mut usize> = Some(&mut 1);
+
+    // should not lint
+    let _ = Some(Box::new(1)).as_deref();
+    let _ = Some(Box::new(1)).as_deref_mut();
+}

tests/ui/needless_option_as_deref.rs

@@ -0,0 +1,13 @@
+// run-rustfix
+
+#[warn(clippy::needless_option_as_deref)]
+
+fn main() {
+    // should lint
+    let _: Option<&usize> = Some(&1).as_deref();
+    let _: Option<&mut usize> = Some(&mut 1).as_deref_mut();
+
+    // should not lint
+    let _ = Some(Box::new(1)).as_deref();
+    let _ = Some(Box::new(1)).as_deref_mut();
+}

tests/ui/needless_option_as_deref.stderr

@@ -0,0 +1,16 @@
+error: derefed type is same as origin
+  --> $DIR/needless_option_as_deref.rs:7:29
+   |
+LL |     let _: Option<&usize> = Some(&1).as_deref();
+   |                             ^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&1)`
+   |
+   = note: `-D clippy::needless-option-as-deref` implied by `-D warnings`
+
+error: derefed type is same as origin
+  --> $DIR/needless_option_as_deref.rs:8:33
+   |
+LL |     let _: Option<&mut usize> = Some(&mut 1).as_deref_mut();
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&mut 1)`
+
+error: aborting due to 2 previous errors
+