diff options
| -rw-r--r-- | clippy_lints/src/operators/identity_op.rs | 65 | ||||
| -rw-r--r-- | tests/ui/identity_op.fixed | 17 | ||||
| -rw-r--r-- | tests/ui/identity_op.rs | 17 | ||||
| -rw-r--r-- | tests/ui/identity_op.stderr | 20 |
4 files changed, 114 insertions, 5 deletions
diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs index e1fd09549a4..6460bf6d672 100644 --- a/clippy_lints/src/operators/identity_op.rs +++ b/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 +} diff --git a/tests/ui/identity_op.fixed b/tests/ui/identity_op.fixed index a1b55602998..2cc4a5d0c23 100644 --- a/tests/ui/identity_op.fixed +++ b/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 +} diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index f603e1078e4..da0597c7abe 100644 --- a/tests/ui/identity_op.rs +++ b/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 +} diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index 8f9c2b603c4..9c774a313ff 100644 --- a/tests/ui/identity_op.stderr +++ b/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 |