]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/misc.rs
ast/hir: Rename field-related structures
[rust.git] / clippy_lints / src / misc.rs
index 308e92057b751cecf7f8e2c5f13fb54904c43ec2..acdc245456b5a1a508ce480e972b861bc44bf985 100644 (file)
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::hygiene::DesugaringKind;
 use rustc_span::source_map::{ExpnKind, Span};
+use rustc_span::symbol::sym;
 
 use crate::consts::{constant, Constant};
 use crate::utils::sugg::Sugg;
 use crate::utils::{
-    get_item_name, get_parent_expr, higher, implements_trait, in_constant, is_integer_const, iter_input_pats,
-    last_path_segment, match_qpath, match_trait_method, paths, snippet, snippet_opt, span_lint, span_lint_and_sugg,
-    span_lint_and_then, span_lint_hir_and_then, SpanlessEq,
+    get_item_name, get_parent_expr, higher, implements_trait, in_constant, is_diagnostic_assoc_item, is_integer_const,
+    iter_input_pats, last_path_segment, match_qpath, snippet, snippet_opt, span_lint, span_lint_and_sugg,
+    span_lint_and_then, span_lint_hir_and_then, unsext, SpanlessEq,
 };
 
 declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for getting the remainder of a division by one.
+    /// **What it does:** Checks for getting the remainder of a division by one or minus
+    /// one.
     ///
-    /// **Why is this bad?** The result can only ever be zero. No one will write
-    /// such code deliberately, unless trying to win an Underhanded Rust
-    /// Contest. Even for that contest, it's probably a bad idea. Use something more
-    /// underhanded.
+    /// **Why is this bad?** The result for a divisor of one can only ever be zero; for
+    /// minus one it can cause panic/overflow (if the left operand is the minimal value of
+    /// the respective integer type) or results in zero. No one will write such code
+    /// deliberately, unless trying to win an Underhanded Rust Contest. Even for that
+    /// contest, it's probably a bad idea. Use something more underhanded.
     ///
     /// **Known problems:** None.
     ///
     /// ```rust
     /// # let x = 1;
     /// let a = x % 1;
+    /// let a = x % -1;
     /// ```
     pub MODULO_ONE,
     correctness,
-    "taking a number modulo 1, which always returns 0"
+    "taking a number modulo +/-1, which can either panic/overflow or always returns 0"
 }
 
 declare_clippy_lint! {
@@ -275,7 +279,7 @@ fn check_fn(
         span: Span,
         _: HirId,
     ) {
-        if let FnKind::Closure(_) = k {
+        if let FnKind::Closure = k {
             // Does not apply to closures
             return;
         }
@@ -289,7 +293,7 @@ fn check_fn(
                     TOPLEVEL_REF_ARG,
                     arg.pat.span,
                     "`ref` directly on a function argument is ignored. \
-                    Consider using a reference type instead.",
+                    Consider using a reference type instead",
                 );
             }
         }
@@ -378,60 +382,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 return;
             },
             ExprKind::Binary(ref cmp, ref left, ref right) => {
-                let op = cmp.node;
-                if op.is_comparison() {
-                    check_nan(cx, left, expr);
-                    check_nan(cx, right, expr);
-                    check_to_owned(cx, left, right, true);
-                    check_to_owned(cx, right, left, false);
-                }
-                if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) {
-                    if is_allowed(cx, left) || is_allowed(cx, right) {
-                        return;
-                    }
-
-                    // Allow comparing the results of signum()
-                    if is_signum(cx, left) && is_signum(cx, right) {
-                        return;
-                    }
-
-                    if let Some(name) = get_item_name(cx, expr) {
-                        let name = name.as_str();
-                        if name == "eq"
-                            || name == "ne"
-                            || name == "is_nan"
-                            || name.starts_with("eq_")
-                            || name.ends_with("_eq")
-                        {
-                            return;
-                        }
-                    }
-                    let is_comparing_arrays = is_array(cx, left) || is_array(cx, right);
-                    let (lint, msg) = get_lint_and_message(
-                        is_named_constant(cx, left) || is_named_constant(cx, right),
-                        is_comparing_arrays,
-                    );
-                    span_lint_and_then(cx, lint, expr.span, msg, |diag| {
-                        let lhs = Sugg::hir(cx, left, "..");
-                        let rhs = Sugg::hir(cx, right, "..");
-
-                        if !is_comparing_arrays {
-                            diag.span_suggestion(
-                                expr.span,
-                                "consider comparing them within some margin of error",
-                                format!(
-                                    "({}).abs() {} error_margin",
-                                    lhs - rhs,
-                                    if op == BinOpKind::Eq { '<' } else { '>' }
-                                ),
-                                Applicability::HasPlaceholders, // snippet
-                            );
-                        }
-                        diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
-                    });
-                } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
-                    span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
-                }
+                check_binary(cx, expr, cmp, left, right);
+                return;
             },
             _ => {},
         }
@@ -471,7 +423,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 expr.span,
                 &format!(
                     "used binding `{}` which is prefixed with an underscore. A leading \
-                     underscore signals that a binding will not be used.",
+                     underscore signals that a binding will not be used",
                     binding
                 ),
             );
@@ -551,7 +503,7 @@ fn is_allowed<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
 // Return true if `expr` is the result of `signum()` invoked on a float value.
 fn is_signum(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     // The negation of a signum is still a signum
-    if let ExprKind::Unary(UnOp::UnNeg, ref child_expr) = expr.kind {
+    if let ExprKind::Unary(UnOp::Neg, ref child_expr) = expr.kind {
         return is_signum(cx, &child_expr);
     }
 
@@ -603,11 +555,16 @@ fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'t
 
     let (arg_ty, snip) = match expr.kind {
         ExprKind::MethodCall(.., ref args, _) if args.len() == 1 => {
-            if match_trait_method(cx, expr, &paths::TO_STRING) || match_trait_method(cx, expr, &paths::TO_OWNED) {
-                (cx.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, ".."))
-            } else {
-                return;
-            }
+            if_chain!(
+                if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+                if is_diagnostic_assoc_item(cx, expr_def_id, sym::ToString)
+                    || is_diagnostic_assoc_item(cx, expr_def_id, sym::ToOwned);
+                then {
+                    (cx.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, ".."))
+                } else {
+                    return;
+                }
+            )
         },
         ExprKind::Call(ref path, ref v) if v.len() == 1 => {
             if let ExprKind::Path(ref path) = path.kind {
@@ -635,7 +592,7 @@ fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'t
         return;
     }
 
-    let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::UnDeref, _));
+    let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::Deref, _));
 
     let lint_span = if other_gets_derefed {
         expr.span.to(other.span)
@@ -744,3 +701,74 @@ fn check_cast(cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>)
         }
     }
 }
+
+fn check_binary(
+    cx: &LateContext<'a>,
+    expr: &Expr<'_>,
+    cmp: &rustc_span::source_map::Spanned<rustc_hir::BinOpKind>,
+    left: &'a Expr<'_>,
+    right: &'a Expr<'_>,
+) {
+    let op = cmp.node;
+    if op.is_comparison() {
+        check_nan(cx, left, expr);
+        check_nan(cx, right, expr);
+        check_to_owned(cx, left, right, true);
+        check_to_owned(cx, right, left, false);
+    }
+    if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) {
+        if is_allowed(cx, left) || is_allowed(cx, right) {
+            return;
+        }
+
+        // Allow comparing the results of signum()
+        if is_signum(cx, left) && is_signum(cx, right) {
+            return;
+        }
+
+        if let Some(name) = get_item_name(cx, expr) {
+            let name = name.as_str();
+            if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") || name.ends_with("_eq") {
+                return;
+            }
+        }
+        let is_comparing_arrays = is_array(cx, left) || is_array(cx, right);
+        let (lint, msg) = get_lint_and_message(
+            is_named_constant(cx, left) || is_named_constant(cx, right),
+            is_comparing_arrays,
+        );
+        span_lint_and_then(cx, lint, expr.span, msg, |diag| {
+            let lhs = Sugg::hir(cx, left, "..");
+            let rhs = Sugg::hir(cx, right, "..");
+
+            if !is_comparing_arrays {
+                diag.span_suggestion(
+                    expr.span,
+                    "consider comparing them within some margin of error",
+                    format!(
+                        "({}).abs() {} error_margin",
+                        lhs - rhs,
+                        if op == BinOpKind::Eq { '<' } else { '>' }
+                    ),
+                    Applicability::HasPlaceholders, // snippet
+                );
+            }
+            diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
+        });
+    } else if op == BinOpKind::Rem {
+        if is_integer_const(cx, right, 1) {
+            span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
+        }
+
+        if let ty::Int(ity) = cx.typeck_results().expr_ty(right).kind() {
+            if is_integer_const(cx, right, unsext(cx.tcx, -1, *ity)) {
+                span_lint(
+                    cx,
+                    MODULO_ONE,
+                    expr.span,
+                    "any number modulo -1 will panic/overflow or result in 0",
+                );
+            }
+        };
+    }
+}