]> git.lizzy.rs Git - rust.git/blobdiff - src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs
Rollup merge of #103305 - c410-f3r:moar-errors, r=petrochenkov
[rust.git] / src / tools / clippy / clippy_lints / src / operators / arithmetic_side_effects.rs
index d24c57c0a4b8a5ccdcb342a3b89850f9a42d70a0..8827daaa3ee7c37760d54ad43ada2dc2d369c83d 100644 (file)
@@ -1,8 +1,3 @@
-#![allow(
-    // False positive
-    clippy::match_same_arms
-)]
-
 use super::ARITHMETIC_SIDE_EFFECTS;
 use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
 use rustc_ast as ast;
 use rustc_span::source_map::{Span, Spanned};
 
 const HARD_CODED_ALLOWED: &[&str] = &[
+    "&str",
     "f32",
     "f64",
     "std::num::Saturating",
-    "std::string::String",
     "std::num::Wrapping",
+    "std::string::String",
 ];
 
 #[derive(Debug)]
@@ -45,61 +41,59 @@ pub fn new(mut allowed: FxHashSet<String>) -> Self {
     /// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a
     /// non-constant environment that won't overflow.
     fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
-        if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node
-            && let hir::ExprKind::Lit(ref lit) = expr.kind
-            && let ast::LitKind::Int(0, _) = lit.node
-        {
-            return true;
-        }
-        if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node
-            && let hir::ExprKind::Lit(ref lit) = expr.kind
-            && !matches!(lit.node, ast::LitKind::Int(0, _))
+        if let hir::ExprKind::Lit(ref lit) = expr.kind &&
+            let ast::LitKind::Int(value, _) = lit.node
         {
-            return true;
-        }
-        if let hir::BinOpKind::Mul = op.node
-            && let hir::ExprKind::Lit(ref lit) = expr.kind
-            && let ast::LitKind::Int(0 | 1, _) = lit.node
-        {
-            return true;
+            match (&op.node, value) {
+                (hir::BinOpKind::Div | hir::BinOpKind::Rem, 0) => false,
+                (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0)
+                    | (hir::BinOpKind::Div | hir::BinOpKind::Rem, _)
+                    | (hir::BinOpKind::Mul, 0 | 1) => true,
+                _ => false,
+            }
+        } else {
+            false
         }
-        false
     }
 
     /// Checks if the given `expr` has any of the inner `allowed` elements.
-    fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
-        self.allowed.contains(
-            cx.typeck_results()
-                .expr_ty(expr)
-                .to_string()
-                .split('<')
-                .next()
-                .unwrap_or_default(),
-        )
+    fn is_allowed_ty(&self, ty: Ty<'_>) -> bool {
+        self.allowed
+            .contains(ty.to_string().split('<').next().unwrap_or_default())
     }
 
-    /// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references.
-    fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool {
-        let is_integral = expr_refs.is_integral();
-        let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_));
-        is_integral && is_literal
+    // For example, 8i32 or &i64::MAX.
+    fn is_integral(ty: Ty<'_>) -> bool {
+        ty.peel_refs().is_integral()
     }
 
+    // Common entry-point to avoid code duplication.
     fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
         let msg = "arithmetic operation that can potentially result in unexpected side-effects";
         span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
         self.expr_span = Some(expr.span);
     }
 
+    /// If `expr` does not match any variant of `LiteralIntegerTy`, returns `None`.
+    fn literal_integer<'expr, 'tcx>(expr: &'expr hir::Expr<'tcx>) -> Option<LiteralIntegerTy<'expr, 'tcx>> {
+        if matches!(expr.kind, hir::ExprKind::Lit(_)) {
+            return Some(LiteralIntegerTy::Value(expr));
+        }
+        if let hir::ExprKind::AddrOf(.., inn) = expr.kind && let hir::ExprKind::Lit(_) = inn.kind {
+            return Some(LiteralIntegerTy::Ref(inn));
+        }
+        None
+    }
+
     /// Manages when the lint should be triggered. Operations in constant environments, hard coded
     /// types, custom allowed types and non-constant operations that won't overflow are ignored.
-    fn manage_bin_ops(
+    fn manage_bin_ops<'tcx>(
         &mut self,
-        cx: &LateContext<'_>,
-        expr: &hir::Expr<'_>,
+        cx: &LateContext<'tcx>,
+        expr: &hir::Expr<'tcx>,
         op: &Spanned<hir::BinOpKind>,
-        lhs: &hir::Expr<'_>,
-        rhs: &hir::Expr<'_>,
+        lhs: &hir::Expr<'tcx>,
+        rhs: &hir::Expr<'tcx>,
     ) {
         if constant_simple(cx, cx.typeck_results(), expr).is_some() {
             return;
@@ -116,17 +110,20 @@ fn manage_bin_ops(
         ) {
             return;
         };
-        if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
+        let lhs_ty = cx.typeck_results().expr_ty(lhs);
+        let rhs_ty = cx.typeck_results().expr_ty(rhs);
+        let lhs_and_rhs_have_the_same_ty = lhs_ty == rhs_ty;
+        if lhs_and_rhs_have_the_same_ty && self.is_allowed_ty(lhs_ty) && self.is_allowed_ty(rhs_ty) {
             return;
         }
-        let has_valid_op = match (
-            Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(lhs).peel_refs()),
-            Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
-        ) {
-            (true, true) => true,
-            (true, false) => Self::has_valid_op(op, lhs),
-            (false, true) => Self::has_valid_op(op, rhs),
-            (false, false) => false,
+        let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
+            match (Self::literal_integer(lhs), Self::literal_integer(rhs)) {
+                (None, Some(lit_int_ty)) | (Some(lit_int_ty), None) => Self::has_valid_op(op, lit_int_ty.into()),
+                (Some(LiteralIntegerTy::Value(_)), Some(LiteralIntegerTy::Value(_))) => true,
+                (None, None) | (Some(_), Some(_)) => false,
+            }
+        } else {
+            false
         };
         if !has_valid_op {
             self.issue_lint(cx, expr);
@@ -135,7 +132,7 @@ fn manage_bin_ops(
 }
 
 impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) {
         if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) {
             return;
         }
@@ -180,3 +177,22 @@ fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>)
         }
     }
 }
+
+/// Tells if an expression is a integer declared by value or by reference.
+///
+/// If `LiteralIntegerTy::Ref`, then the contained value will be `hir::ExprKind::Lit` rather
+/// than `hirExprKind::Addr`.
+enum LiteralIntegerTy<'expr, 'tcx> {
+    /// For example, `&199`
+    Ref(&'expr hir::Expr<'tcx>),
+    /// For example, `1` or `i32::MAX`
+    Value(&'expr hir::Expr<'tcx>),
+}
+
+impl<'expr, 'tcx> From<LiteralIntegerTy<'expr, 'tcx>> for &'expr hir::Expr<'tcx> {
+    fn from(from: LiteralIntegerTy<'expr, 'tcx>) -> Self {
+        match from {
+            LiteralIntegerTy::Ref(elem) | LiteralIntegerTy::Value(elem) => elem,
+        }
+    }
+}