Fix FP of identity_op when encountering Default::default()

Author: relaxcn

clippy_lints/src/operators/identity_op.rs

@@ -3,10 +3,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{clip, peel_hir_expr_refs, unsext};
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
+use rustc_hir::{BinOpKind, Expr, ExprKind, HirId, Item, ItemKind, Node, QPath};
 use rustc_lint::LateContext;
 use rustc_middle::ty;
-use rustc_span::Span;
+use rustc_span::{Span, sym};
 
 use super::IDENTITY_OP;
 
@@ -17,7 +17,7 @@ pub(crate) fn check<'tcx>(
     left: &'tcx Expr<'_>,
     right: &'tcx Expr<'_>,
 ) {
-    if !is_allowed(cx, op, left, right) {
+    if !is_allowed(cx, expr, op, left, right) {
         return;
     }
 
@@ -165,7 +165,14 @@ fn needs_parenthesis(cx: &LateContext<'_>, binary: &Expr<'_>, child: &Expr<'_>)
     Parens::Needed
 }
 
-fn is_allowed(cx: &LateContext<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Expr<'_>) -> bool {
+fn is_allowed(cx: &LateContext<'_>, expr: &Expr<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Expr<'_>) -> bool {
+    // Exclude case where the left or right side is a call to `Default::default()`
+    // and the expression is not a let binding's init expression and the let binding has a type
+    // annotation, or a function's return value.
+    if (is_default_call(cx, left) || is_default_call(cx, right)) && !is_expr_with_type_annotation(cx, expr.hir_id) {
+        return false;
+    }
+
     // This lint applies to integers and their references
     cx.typeck_results().expr_ty(left).peel_refs().is_integral()
         && cx.typeck_results().expr_ty(right).peel_refs().is_integral()
@@ -175,6 +182,20 @@ fn is_allowed(cx: &LateContext<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Exp
             && ConstEvalCtxt::new(cx).eval_simple(left) == Some(Constant::Int(1)))
 }
 
+/// Check if the expression is a call to `Default::default()`
+fn is_default_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    if let ExprKind::Call(func, []) = peel_hir_expr_refs(expr).0.kind
+        && let ExprKind::Path(qpath) = func.kind
+        // Detect and ignore <Foo as Default>::default() because these calls do explicitly name the type.
+        && let QPath::Resolved(None, _path) = qpath
+        && let Some(def_id) = cx.qpath_res(&qpath, func.hir_id).opt_def_id()
+        && cx.tcx.is_diagnostic_item(sym::default_fn, def_id)
+    {
+        return true;
+    }
+    false
+}
+
 fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span: Span, arg: Span) {
     let ecx = ConstEvalCtxt::new(cx);
     if match (ecx.eval_full_int(left), ecx.eval_full_int(right)) {
@@ -234,3 +255,39 @@ fn span_ineffective_operation(
         applicability,
     );
 }
+
+/// Check if the expression is a let binding's init expression and the let binding has a type
+/// annotation. Also check if the expression is a function's return value.
+fn is_expr_with_type_annotation(cx: &LateContext<'_>, hir_id: HirId) -> bool {
+    // Get the parent node of the expression
+    if let Some((_, parent)) = cx.tcx.hir_parent_iter(hir_id).next() {
+        match parent {
+            Node::LetStmt(local) => {
+                // Check if this expression is the init expression of the let binding
+                if let Some(init) = local.init
+                    && init.hir_id == hir_id
+                {
+                    // Check if the let binding has an explicit type annotation
+                    return local.ty.is_some();
+                }
+            },
+            Node::Block(block) => {
+                // If the parent node is a block, we can make sure the expression is the last expression in the
+                // block.
+                return is_expr_with_type_annotation(cx, block.hir_id);
+            },
+            Node::Expr(expr) => {
+                return is_expr_with_type_annotation(cx, expr.hir_id);
+            },
+            Node::Item(Item {
+                kind: ItemKind::Fn { .. },
+                ..
+            }) => {
+                // Every function has a return type, so we can return true.
+                return true;
+            },
+            _ => {},
+        }
+    }
+    false
+}

tests/ui/identity_op.fixed

@@ -312,3 +312,20 @@ fn issue_13470() {
     let _: u64 = 1u64 + ((x as i32 + y as i32) as u64);
     //~^ identity_op
 }
+
+fn issue_14932() {
+    let _ = 0usize + &Default::default(); // no error
+
+    0usize + &Default::default(); // no error
+
+    let _ = usize::default();
+    //~^ identity_op
+
+    let _n: usize = Default::default();
+    //~^ identity_op
+}
+
+fn issue_14932_2() -> usize {
+    Default::default()
+    //~^ identity_op
+}

tests/ui/identity_op.rs

@@ -312,3 +312,20 @@ fn issue_13470() {
     let _: u64 = 1u64 + ((x as i32 + y as i32) as u64 + 0u64);
     //~^ identity_op
 }
+
+fn issue_14932() {
+    let _ = 0usize + &Default::default(); // no error
+
+    0usize + &Default::default(); // no error
+
+    let _ = 0usize + &usize::default();
+    //~^ identity_op
+
+    let _n: usize = 0usize + &Default::default();
+    //~^ identity_op
+}
+
+fn issue_14932_2() -> usize {
+    0usize + &Default::default()
+    //~^ identity_op
+}

tests/ui/identity_op.stderr

@@ -379,5 +379,23 @@ error: this operation has no effect
 LL |     let _: u64 = 1u64 + ((x as i32 + y as i32) as u64 + 0u64);
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `((x as i32 + y as i32) as u64)`
 
-error: aborting due to 63 previous errors
+error: this operation has no effect
+  --> tests/ui/identity_op.rs:321:13
+   |
+LL |     let _ = 0usize + &usize::default();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `usize::default()`
+
+error: this operation has no effect
+  --> tests/ui/identity_op.rs:324:21
+   |
+LL |     let _n: usize = 0usize + &Default::default();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `Default::default()`
+
+error: this operation has no effect
+  --> tests/ui/identity_op.rs:329:5
+   |
+LL |     0usize + &Default::default()
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `Default::default()`
+
+error: aborting due to 66 previous errors