Auto merge of #5185 - krishna-veerareddy:issue-5160-lossy-float-literal, r=flip1995

bors · bors · commit 74f5e348fa9c · 2020-02-17T09:18:25.000Z
Lint lossy whole number float literals changelog: Extend `excessive_precision` to lint lossy whole number float literals Fixes #5160
diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs
@@ -6,33 +6,31 @@ use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::symbol::Symbol;
-use std::f32;
-use std::f64;
-use std::fmt;
+use std::{f32, f64, fmt};
 use syntax::ast::*;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for float literals with a precision greater
-    /// than that supported by the underlying type
+    /// than that supported by the underlying type.
     ///
-    /// **Why is this bad?** Rust will truncate the literal silently.
+    /// **Why is this bad?** Rust will silently lose precision during conversion
+    /// to a float.
     ///
     /// **Known problems:** None.
     ///
     /// **Example:**
     ///
     /// ```rust
     /// // Bad
-    /// let v: f32 = 0.123_456_789_9;
-    /// println!("{}", v); //  0.123_456_789
+    /// let a: f32 = 0.123_456_789_9; // 0.123_456_789
+    /// let b: f32 = 16_777_217.0; // 16_777_216.0
     ///
     /// // Good
-    /// let v: f64 = 0.123_456_789_9;
-    /// println!("{}", v); //  0.123_456_789_9
+    /// let a: f64 = 0.123_456_789_9;
+    /// let b: f64 = 16_777_216.0;
     /// ```
     pub EXCESSIVE_PRECISION,
-    style,
+    correctness,
     "excessive precision for float literal"
 }
 
@@ -44,71 +42,62 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
             let ty = cx.tables.expr_ty(expr);
             if let ty::Float(fty) = ty.kind;
             if let hir::ExprKind::Lit(ref lit) = expr.kind;
-            if let LitKind::Float(sym, _) = lit.node;
-            if let Some(sugg) = Self::check(sym, fty);
+            if let LitKind::Float(sym, lit_float_ty) = lit.node;
             then {
-                span_lint_and_sugg(
-                    cx,
-                    EXCESSIVE_PRECISION,
-                    expr.span,
-                    "float has excessive precision",
-                    "consider changing the type or truncating it to",
-                    sugg,
-                    Applicability::MachineApplicable,
-                );
-            }
-        }
-    }
-}
-
-impl ExcessivePrecision {
-    // None if nothing to lint, Some(suggestion) if lint necessary
-    #[must_use]
-    fn check(sym: Symbol, fty: FloatTy) -> Option<String> {
-        let max = max_digits(fty);
-        let sym_str = sym.as_str();
-        if dot_zero_exclusion(&sym_str) {
-            return None;
-        }
-        // Try to bail out if the float is for sure fine.
-        // If its within the 2 decimal digits of being out of precision we
-        // check if the parsed representation is the same as the string
-        // since we'll need the truncated string anyway.
-        let digits = count_digits(&sym_str);
-        if digits > max as usize {
-            let formatter = FloatFormat::new(&sym_str);
-            let sr = match fty {
-                FloatTy::F32 => sym_str.parse::<f32>().map(|f| formatter.format(f)),
-                FloatTy::F64 => sym_str.parse::<f64>().map(|f| formatter.format(f)),
-            };
-            // We know this will parse since we are in LatePass
-            let s = sr.unwrap();
+                let sym_str = sym.as_str();
+                let formatter = FloatFormat::new(&sym_str);
+                // Try to bail out if the float is for sure fine.
+                // If its within the 2 decimal digits of being out of precision we
+                // check if the parsed representation is the same as the string
+                // since we'll need the truncated string anyway.
+                let digits = count_digits(&sym_str);
+                let max = max_digits(fty);
+                let float_str = match fty {
+                    FloatTy::F32 => sym_str.parse::<f32>().map(|f| formatter.format(f)),
+                    FloatTy::F64 => sym_str.parse::<f64>().map(|f| formatter.format(f)),
+                }.unwrap();
+                let type_suffix = match lit_float_ty {
+                    LitFloatType::Suffixed(FloatTy::F32) => Some("f32"),
+                    LitFloatType::Suffixed(FloatTy::F64) => Some("f64"),
+                    _ => None
+                };
 
-            if sym_str == s {
-                None
-            } else {
-                Some(format_numeric_literal(&s, None, true))
+                if is_whole_number(&sym_str, fty) {
+                    // Normalize the literal by stripping the fractional portion
+                    if sym_str.split('.').next().unwrap() != float_str {
+                        span_lint_and_sugg(
+                            cx,
+                            EXCESSIVE_PRECISION,
+                            expr.span,
+                            "literal cannot be represented as the underlying type without loss of precision",
+                            "consider changing the type or replacing it with",
+                            format_numeric_literal(format!("{}.0", float_str).as_str(), type_suffix, true),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                } else if digits > max as usize && sym_str != float_str {
+                    span_lint_and_sugg(
+                        cx,
+                        EXCESSIVE_PRECISION,
+                        expr.span,
+                        "float has excessive precision",
+                        "consider changing the type or truncating it to",
+                        format_numeric_literal(&float_str, type_suffix, true),
+                        Applicability::MachineApplicable,
+                    );
+                }
             }
-        } else {
-            None
         }
     }
 }
 
-/// Should we exclude the float because it has a `.0` or `.` suffix
-/// Ex `1_000_000_000.0`
-/// Ex `1_000_000_000.`
+// Checks whether a float literal is a whole number
 #[must_use]
-fn dot_zero_exclusion(s: &str) -> bool {
-    s.split('.').nth(1).map_or(false, |after_dec| {
-        let mut decpart = after_dec.chars().take_while(|c| *c != 'e' || *c != 'E');
-
-        match decpart.next() {
-            Some('0') => decpart.count() == 0,
-            Some(_) => false,
-            None => true,
-        }
-    })
+fn is_whole_number(sym_str: &str, fty: FloatTy) -> bool {
+    match fty {
+        FloatTy::F32 => sym_str.parse::<f32>().unwrap().fract() == 0.0,
+        FloatTy::F64 => sym_str.parse::<f64>().unwrap().fract() == 0.0,
+    }
 }
 
 #[must_use]
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -1386,7 +1386,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&enum_variants::MODULE_INCEPTION),
         LintId::of(&eq_op::OP_REF),
         LintId::of(&eta_reduction::REDUNDANT_CLOSURE),
-        LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
         LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
@@ -1567,6 +1566,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
         LintId::of(&eq_op::EQ_OP),
         LintId::of(&erasing_op::ERASING_OP),
+        LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
         LintId::of(&formatting::POSSIBLE_MISSING_COMMA),
         LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
         LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -492,7 +492,7 @@ pub const ALL_LINTS: [Lint; 355] = [
     },
     Lint {
         name: "excessive_precision",
-        group: "style",
+        group: "correctness",
         desc: "excessive precision for float literal",
         deprecation: None,
         module: "excessive_precision",
diff --git a/tests/ui/excessive_precision.fixed b/tests/ui/excessive_precision.fixed
@@ -12,12 +12,12 @@ fn main() {
     const GOOD64_SM: f32 = 0.000_000_000_000_000_1;
     const GOOD64_DOT: f32 = 10_000_000_000_000_000.0;
 
-    const BAD32_1: f32 = 0.123_456_79;
+    const BAD32_1: f32 = 0.123_456_79_f32;
     const BAD32_2: f32 = 0.123_456_79;
     const BAD32_3: f32 = 0.1;
     const BAD32_EDGE: f32 = 1.000_001;
 
-    const BAD64_1: f64 = 0.123_456_789_012_345_66;
+    const BAD64_1: f64 = 0.123_456_789_012_345_66_f64;
     const BAD64_2: f64 = 0.123_456_789_012_345_66;
     const BAD64_3: f64 = 0.1;
 
@@ -34,11 +34,11 @@ fn main() {
     let good64_inf = 0.123_456_789_012;
 
     let bad32: f32 = 1.123_456_8;
-    let bad32_suf: f32 = 1.123_456_8;
-    let bad32_inf = 1.123_456_8;
+    let bad32_suf: f32 = 1.123_456_8_f32;
+    let bad32_inf = 1.123_456_8_f32;
 
     let bad64: f64 = 0.123_456_789_012_345_66;
-    let bad64_suf: f64 = 0.123_456_789_012_345_66;
+    let bad64_suf: f64 = 0.123_456_789_012_345_66_f64;
     let bad64_inf = 0.123_456_789_012_345_66;
 
     // Vectors
@@ -60,4 +60,26 @@ fn main() {
 
     // issue #2840
     let num = 0.000_000_000_01e-10f64;
+
+    // Lossy whole-number float literals
+    let _: f32 = 16_777_216.0;
+    let _: f32 = 16_777_220.0;
+    let _: f32 = 16_777_220.0;
+    let _: f32 = 16_777_220.0;
+    let _ = 16_777_220.0_f32;
+    let _: f32 = -16_777_220.0;
+    let _: f64 = 9_007_199_254_740_992.0;
+    let _: f64 = 9_007_199_254_740_992.0;
+    let _: f64 = 9_007_199_254_740_992.0;
+    let _ = 9_007_199_254_740_992.0_f64;
+    let _: f64 = -9_007_199_254_740_992.0;
+
+    // Lossless whole number float literals
+    let _: f32 = 16_777_216.0;
+    let _: f32 = 16_777_218.0;
+    let _: f32 = 16_777_220.0;
+    let _: f32 = -16_777_216.0;
+    let _: f32 = -16_777_220.0;
+    let _: f64 = 9_007_199_254_740_992.0;
+    let _: f64 = -9_007_199_254_740_992.0;
 }
diff --git a/tests/ui/excessive_precision.rs b/tests/ui/excessive_precision.rs
@@ -60,4 +60,26 @@ fn main() {
 
     // issue #2840
     let num = 0.000_000_000_01e-10f64;
+
+    // Lossy whole-number float literals
+    let _: f32 = 16_777_217.0;
+    let _: f32 = 16_777_219.0;
+    let _: f32 = 16_777_219.;
+    let _: f32 = 16_777_219.000;
+    let _ = 16_777_219f32;
+    let _: f32 = -16_777_219.0;
+    let _: f64 = 9_007_199_254_740_993.0;
+    let _: f64 = 9_007_199_254_740_993.;
+    let _: f64 = 9_007_199_254_740_993.000;
+    let _ = 9_007_199_254_740_993f64;
+    let _: f64 = -9_007_199_254_740_993.0;
+
+    // Lossless whole number float literals
+    let _: f32 = 16_777_216.0;
+    let _: f32 = 16_777_218.0;
+    let _: f32 = 16_777_220.0;
+    let _: f32 = -16_777_216.0;
+    let _: f32 = -16_777_220.0;
+    let _: f64 = 9_007_199_254_740_992.0;
+    let _: f64 = -9_007_199_254_740_992.0;
 }
diff --git a/tests/ui/excessive_precision.stderr b/tests/ui/excessive_precision.stderr
@@ -2,7 +2,7 @@ error: float has excessive precision
   --> $DIR/excessive_precision.rs:15:26
    |
 LL |     const BAD32_1: f32 = 0.123_456_789_f32;
-   |                          ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79`
+   |                          ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_79_f32`
    |
    = note: `-D clippy::excessive-precision` implied by `-D warnings`
 
@@ -28,7 +28,7 @@ error: float has excessive precision
   --> $DIR/excessive_precision.rs:20:26
    |
 LL |     const BAD64_1: f64 = 0.123_456_789_012_345_67f64;
-   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66`
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64`
 
 error: float has excessive precision
   --> $DIR/excessive_precision.rs:21:26
@@ -58,13 +58,13 @@ error: float has excessive precision
   --> $DIR/excessive_precision.rs:37:26
    |
 LL |     let bad32_suf: f32 = 1.123_456_789_f32;
-   |                          ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8`
+   |                          ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8_f32`
 
 error: float has excessive precision
   --> $DIR/excessive_precision.rs:38:21
    |
 LL |     let bad32_inf = 1.123_456_789_f32;
-   |                     ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8`
+   |                     ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8_f32`
 
 error: float has excessive precision
   --> $DIR/excessive_precision.rs:40:22
@@ -76,7 +76,7 @@ error: float has excessive precision
   --> $DIR/excessive_precision.rs:41:26
    |
 LL |     let bad64_suf: f64 = 0.123_456_789_012_345_67f64;
-   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66`
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64`
 
 error: float has excessive precision
   --> $DIR/excessive_precision.rs:42:21
@@ -108,5 +108,71 @@ error: float has excessive precision
 LL |     let bad_bige32: f32 = 1.123_456_788_888E-10;
    |                           ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8E-10`
 
-error: aborting due to 18 previous errors
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:65:18
+   |
+LL |     let _: f32 = 16_777_217.0;
+   |                  ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_216.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:66:18
+   |
+LL |     let _: f32 = 16_777_219.0;
+   |                  ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:67:18
+   |
+LL |     let _: f32 = 16_777_219.;
+   |                  ^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:68:18
+   |
+LL |     let _: f32 = 16_777_219.000;
+   |                  ^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:69:13
+   |
+LL |     let _ = 16_777_219f32;
+   |             ^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0_f32`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:70:19
+   |
+LL |     let _: f32 = -16_777_219.0;
+   |                   ^^^^^^^^^^^^ help: consider changing the type or replacing it with: `16_777_220.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:71:18
+   |
+LL |     let _: f64 = 9_007_199_254_740_993.0;
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:72:18
+   |
+LL |     let _: f64 = 9_007_199_254_740_993.;
+   |                  ^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:73:18
+   |
+LL |     let _: f64 = 9_007_199_254_740_993.000;
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:74:13
+   |
+LL |     let _ = 9_007_199_254_740_993f64;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0_f64`
+
+error: literal cannot be represented as the underlying type without loss of precision
+  --> $DIR/excessive_precision.rs:75:19
+   |
+LL |     let _: f64 = -9_007_199_254_740_993.0;
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or replacing it with: `9_007_199_254_740_992.0`
+
+error: aborting due to 29 previous errors
 
