Introduce function for comparing expression values

tmiasko · tmiasko · commit 6afa4ef60f97 · 2020-08-16T23:30:33.000+02:00
Introduce `eq_expr_value(cx, a, b)` as a shortcut for
`SpanlessEq::new(cx).ignore_fn().eq_expr(cx, a, b)`.

No functional changes intended.
diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
@@ -1,5 +1,5 @@
 use crate::utils::{
-    get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq,
+    eq_expr_value, get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method,
 };
 use crate::utils::{higher, sugg};
 use if_chain::if_chain;
@@ -70,11 +70,11 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {
                         return;
                     }
                     // lhs op= l op r
-                    if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) {
+                    if eq_expr_value(cx, lhs, l) {
                         lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, r);
                     }
                     // lhs op= l commutative_op r
-                    if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) {
+                    if is_commutative(op.node) && eq_expr_value(cx, lhs, r) {
                         lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, l);
                     }
                 }
@@ -161,14 +161,12 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {
 
                     if visitor.counter == 1 {
                         // a = a op b
-                        if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, l) {
+                        if eq_expr_value(cx, assignee, l) {
                             lint(assignee, r);
                         }
                         // a = b commutative_op a
                         // Limited to primitive type as these ops are know to be commutative
-                        if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r)
-                            && cx.typeck_results().expr_ty(assignee).is_primitive_ty()
-                        {
+                        if eq_expr_value(cx, assignee, r) && cx.typeck_results().expr_ty(assignee).is_primitive_ty() {
                             match op.node {
                                 hir::BinOpKind::Add
                                 | hir::BinOpKind::Mul
@@ -253,7 +251,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> {
     type Map = Map<'tcx>;
 
     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
-        if SpanlessEq::new(self.cx).ignore_fn().eq_expr(self.assignee, expr) {
+        if eq_expr_value(self.cx, self.assignee, expr) {
             self.counter += 1;
         }
 
diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs
@@ -1,6 +1,6 @@
 use crate::utils::{
-    get_trait_def_id, implements_trait, in_macro, is_type_diagnostic_item, paths, snippet_opt, span_lint_and_sugg,
-    span_lint_and_then, SpanlessEq,
+    eq_expr_value, get_trait_def_id, implements_trait, in_macro, is_type_diagnostic_item, paths, snippet_opt,
+    span_lint_and_sugg, span_lint_and_then,
 };
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
@@ -128,7 +128,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
             }
         }
         for (n, expr) in self.terminals.iter().enumerate() {
-            if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e, expr) {
+            if eq_expr_value(self.cx, e, expr) {
                 #[allow(clippy::cast_possible_truncation)]
                 return Ok(Bool::Term(n as u8));
             }
@@ -138,8 +138,8 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
                 if implements_ord(self.cx, e_lhs);
                 if let ExprKind::Binary(expr_binop, expr_lhs, expr_rhs) = &expr.kind;
                 if negate(e_binop.node) == Some(expr_binop.node);
-                if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_lhs, expr_lhs);
-                if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_rhs, expr_rhs);
+                if eq_expr_value(self.cx, e_lhs, expr_lhs);
+                if eq_expr_value(self.cx, e_rhs, expr_rhs);
                 then {
                     #[allow(clippy::cast_possible_truncation)]
                     return Ok(Bool::Not(Box::new(Bool::Term(n as u8))));
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
@@ -1,5 +1,5 @@
+use crate::utils::{eq_expr_value, SpanlessEq, SpanlessHash};
 use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then};
-use crate::utils::{SpanlessEq, SpanlessHash};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind};
 use rustc_lint::{LateContext, LateLintPass};
@@ -197,8 +197,7 @@ fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
         h.finish()
     };
 
-    let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool =
-        &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
+    let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { eq_expr_value(cx, lhs, rhs) };
 
     for (i, j) in search_same(conds, hash, eq) {
         span_lint_and_note(
@@ -222,7 +221,7 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
 
     let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
         // Do not spawn warning if `IFS_SAME_COND` already produced it.
-        if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) {
+        if eq_expr_value(cx, lhs, rhs) {
             return false;
         }
         SpanlessEq::new(cx).eq_expr(lhs, rhs)
diff --git a/clippy_lints/src/double_comparison.rs b/clippy_lints/src/double_comparison.rs
@@ -6,7 +6,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 
-use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
+use crate::utils::{eq_expr_value, snippet_with_applicability, span_lint_and_sugg};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for double comparisons that could be simplified to a single expression.
@@ -46,8 +46,7 @@ impl<'tcx> DoubleComparisons {
             },
             _ => return,
         };
-        let mut spanless_eq = SpanlessEq::new(cx).ignore_fn();
-        if !(spanless_eq.eq_expr(&llhs, &rlhs) && spanless_eq.eq_expr(&lrhs, &rrhs)) {
+        if !(eq_expr_value(cx, &llhs, &rlhs) && eq_expr_value(cx, &lrhs, &rrhs)) {
             return;
         }
         macro_rules! lint_double_comparison {
diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs
@@ -1,5 +1,5 @@
 use crate::utils::{
-    implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, SpanlessEq,
+    eq_expr_value, implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then,
 };
 use rustc_errors::Applicability;
 use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind};
@@ -69,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
             if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
                 return;
             }
-            if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
+            if is_valid_operator(op) && eq_expr_value(cx, left, right) {
                 span_lint(
                     cx,
                     EQ_OP,
diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs
@@ -2,7 +2,7 @@ use crate::consts::{
     constant, constant_simple, Constant,
     Constant::{Int, F32, F64},
 };
-use crate::utils::{get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq};
+use crate::utils::{eq_expr_value, get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
@@ -363,8 +363,8 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option<String> {
         if_chain! {
             if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lmul_lhs, ref lmul_rhs) = add_lhs.kind;
             if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref rmul_lhs, ref rmul_rhs) = add_rhs.kind;
-            if are_exprs_equal(cx, lmul_lhs, lmul_rhs);
-            if are_exprs_equal(cx, rmul_lhs, rmul_rhs);
+            if eq_expr_value(cx, lmul_lhs, lmul_rhs);
+            if eq_expr_value(cx, rmul_lhs, rmul_rhs);
             then {
                 return Some(format!("{}.hypot({})", Sugg::hir(cx, &lmul_lhs, ".."), Sugg::hir(cx, &rmul_lhs, "..")));
             }
@@ -502,8 +502,8 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
 fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
     if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
         match op {
-            BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && are_exprs_equal(cx, left, test),
-            BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && are_exprs_equal(cx, right, test),
+            BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && eq_expr_value(cx, left, test),
+            BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && eq_expr_value(cx, right, test),
             _ => false,
         }
     } else {
@@ -515,19 +515,15 @@ fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -
 fn is_testing_negative(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
     if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
         match op {
-            BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && are_exprs_equal(cx, right, test),
-            BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && are_exprs_equal(cx, left, test),
+            BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && eq_expr_value(cx, right, test),
+            BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && eq_expr_value(cx, left, test),
             _ => false,
         }
     } else {
         false
     }
 }
 
-fn are_exprs_equal(cx: &LateContext<'_>, expr1: &Expr<'_>, expr2: &Expr<'_>) -> bool {
-    SpanlessEq::new(cx).ignore_fn().eq_expr(expr1, expr2)
-}
-
 /// Returns true iff expr is some zero literal
 fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     match constant_simple(cx, cx.typeck_results(), expr) {
@@ -546,12 +542,12 @@ fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 /// returns None.
 fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a>) -> Option<(bool, &'a Expr<'a>)> {
     if let ExprKind::Unary(UnOp::UnNeg, expr1_negated) = &expr1.kind {
-        if are_exprs_equal(cx, expr1_negated, expr2) {
+        if eq_expr_value(cx, expr1_negated, expr2) {
             return Some((false, expr2));
         }
     }
     if let ExprKind::Unary(UnOp::UnNeg, expr2_negated) = &expr2.kind {
-        if are_exprs_equal(cx, expr1, expr2_negated) {
+        if eq_expr_value(cx, expr1, expr2_negated) {
             return Some((true, expr1));
         }
     }
@@ -614,7 +610,7 @@ fn are_same_base_logs(cx: &LateContext<'_>, expr_a: &Expr<'_>, expr_b: &Expr<'_>
                 args_a.len() == args_b.len() &&
                 (
                     ["ln", "log2", "log10"].contains(&&*method_name_a.as_str()) ||
-                    method_name_a.as_str() == "log" && args_a.len() == 2 && are_exprs_equal(cx, &args_a[1], &args_b[1])
+                    method_name_a.as_str() == "log" && args_a.len() == 2 && eq_expr_value(cx, &args_a[1], &args_b[1])
                 );
         }
     }
diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
@@ -7,8 +7,8 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 use crate::utils::sugg::Sugg;
 use crate::utils::{
-    higher, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet_with_applicability,
-    span_lint_and_sugg, SpanlessEq,
+    eq_expr_value, higher, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet_with_applicability,
+    span_lint_and_sugg,
 };
 
 declare_clippy_lint! {
@@ -65,7 +65,7 @@ impl QuestionMark {
                         if let ExprKind::Block(block, None) = &else_.kind;
                         if block.stmts.is_empty();
                         if let Some(block_expr) = &block.expr;
-                        if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr);
+                        if eq_expr_value(cx, subject, block_expr);
                         then {
                             replacement = Some(format!("Some({}?)", receiver_str));
                         }
diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs
@@ -1,7 +1,7 @@
 use crate::utils::sugg::Sugg;
 use crate::utils::{
-    differing_macro_contexts, is_type_diagnostic_item, snippet_with_applicability, span_lint_and_then, walk_ptrs_ty,
-    SpanlessEq,
+    differing_macro_contexts, eq_expr_value, is_type_diagnostic_item, snippet_with_applicability, span_lint_and_then,
+    walk_ptrs_ty,
 };
 use if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -92,8 +92,8 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) {
             if rhs2.segments.len() == 1;
 
             if ident.as_str() == rhs2.segments[0].ident.as_str();
-            if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1);
-            if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2);
+            if eq_expr_value(cx, tmp_init, lhs1);
+            if eq_expr_value(cx, rhs1, lhs2);
             then {
                 if let ExprKind::Field(ref lhs1, _) = lhs1.kind {
                     if let ExprKind::Field(ref lhs2, _) = lhs2.kind {
@@ -193,7 +193,7 @@ enum Slice<'a> {
 fn check_for_slice<'a>(cx: &LateContext<'_>, lhs1: &'a Expr<'_>, lhs2: &'a Expr<'_>) -> Slice<'a> {
     if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind {
         if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind {
-            if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
+            if eq_expr_value(cx, lhs1, lhs2) {
                 let ty = walk_ptrs_ty(cx.typeck_results().expr_ty(lhs1));
 
                 if matches!(ty.kind, ty::Slice(_))
@@ -221,8 +221,8 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) {
             if !differing_macro_contexts(first.span, second.span);
             if let ExprKind::Assign(ref lhs0, ref rhs0, _) = first.kind;
             if let ExprKind::Assign(ref lhs1, ref rhs1, _) = second.kind;
-            if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1);
-            if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0);
+            if eq_expr_value(cx, lhs0, rhs1);
+            if eq_expr_value(cx, lhs1, rhs0);
             then {
                 let lhs0 = Sugg::hir_opt(cx, lhs0);
                 let rhs0 = Sugg::hir_opt(cx, rhs0);
diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs
@@ -340,6 +340,11 @@ pub fn over<X>(left: &[X], right: &[X], mut eq_fn: impl FnMut(&X, &X) -> bool) -
     left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y))
 }
 
+/// Checks if two expressions evaluate to the same value, and don't contain any side effects.
+pub fn eq_expr_value(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>) -> bool {
+    SpanlessEq::new(cx).ignore_fn().eq_expr(left, right)
+}
+
 /// Type used to hash an ast element. This is different from the `Hash` trait
 /// on ast types as this
 /// trait would consider IDs and spans.
diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs
@@ -1,7 +1,6 @@
-use crate::utils::SpanlessEq;
 use crate::utils::{
     is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint,
-    span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty,
+    span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
 };
 use if_chain::if_chain;
 use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId};
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
@@ -21,7 +21,7 @@ pub mod sugg;
 pub mod usage;
 pub use self::attrs::*;
 pub use self::diagnostics::*;
-pub use self::hir_utils::{both, over, SpanlessEq, SpanlessHash};
+pub use self::hir_utils::{both, eq_expr_value, over, SpanlessEq, SpanlessHash};
 
 use std::borrow::Cow;
 use std::mem;
