]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/literal_representation.rs
Auto merge of #3946 - rchaser53:issue-3920, r=flip1995
[rust.git] / clippy_lints / src / literal_representation.rs
index 383bba2d4bded65b651b6fbebe11c673a17d2dfc..e97845314f9d4b075465c42c273b4af3c5196065 100644 (file)
 //! Lints concerned with the grouping of digits with underscores in integral or
 //! floating-point literal expressions.
 
-use rustc::lint::*;
-use rustc::{declare_lint, lint_array};
+use crate::utils::{snippet_opt, span_lint_and_sugg};
 use if_chain::if_chain;
+use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_errors::Applicability;
 use syntax::ast::*;
 use syntax_pos;
-use crate::utils::{in_external_macro, snippet_opt, span_lint_and_sugg};
-
-/// **What it does:** Warns if a long integral or floating-point constant does
-/// not contain underscores.
-///
-/// **Why is this bad?** Reading long numbers is difficult without separators.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// 61864918973511
-/// ```
+
 declare_clippy_lint! {
+    /// **What it does:** Warns if a long integral or floating-point constant does
+    /// not contain underscores.
+    ///
+    /// **Why is this bad?** Reading long numbers is difficult without separators.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let x: u64 = 61864918973511;
+    /// ```
     pub UNREADABLE_LITERAL,
     style,
     "long integer literal without underscores"
 }
 
-/// **What it does:** Warns if an integral or floating-point constant is
-/// grouped inconsistently with underscores.
-///
-/// **Why is this bad?** Readers may incorrectly interpret inconsistently
-/// grouped digits.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// 618_64_9189_73_511
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Warns for mistyped suffix in literals
+    ///
+    /// **Why is this bad?** This is most probably a typo
+    ///
+    /// **Known problems:**
+    ///                - Recommends a signed suffix, even though the number might be too big and an unsigned
+    ///                suffix is required
+    ///                - Does not match on `_128` since that is a valid grouping for decimal and octal numbers
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// 2_32;
+    /// ```
+    pub MISTYPED_LITERAL_SUFFIXES,
+    correctness,
+    "mistyped literal suffix"
+}
+
+declare_clippy_lint! {
+    /// **What it does:** Warns if an integral or floating-point constant is
+    /// grouped inconsistently with underscores.
+    ///
+    /// **Why is this bad?** Readers may incorrectly interpret inconsistently
+    /// grouped digits.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let x: u64 = 618_64_9189_73_511;
+    /// ```
     pub INCONSISTENT_DIGIT_GROUPING,
     style,
     "integer literals with digits grouped inconsistently"
 }
 
-/// **What it does:** Warns if the digits of an integral or floating-point
-/// constant are grouped into groups that
-/// are too large.
-///
-/// **Why is this bad?** Negatively impacts readability.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// 6186491_8973511
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Warns if the digits of an integral or floating-point
+    /// constant are grouped into groups that
+    /// are too large.
+    ///
+    /// **Why is this bad?** Negatively impacts readability.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let x: u64 = 6186491_8973511;
+    /// ```
     pub LARGE_DIGIT_GROUPS,
-    style,
+    pedantic,
     "grouping digits into groups that are too large"
 }
 
-/// **What it does:** Warns if there is a better representation for a numeric literal.
-///
-/// **Why is this bad?** Especially for big powers of 2 a hexadecimal representation is more
-/// readable than a decimal representation.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// `255` => `0xFF`
-/// `65_535` => `0xFFFF`
-/// `4_042_322_160` => `0xF0F0_F0F0`
 declare_clippy_lint! {
+    /// **What it does:** Warns if there is a better representation for a numeric literal.
+    ///
+    /// **Why is this bad?** Especially for big powers of 2 a hexadecimal representation is more
+    /// readable than a decimal representation.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// `255` => `0xFF`
+    /// `65_535` => `0xFFFF`
+    /// `4_042_322_160` => `0xF0F0_F0F0`
     pub DECIMAL_LITERAL_REPRESENTATION,
     restriction,
     "using decimal representation when hexadecimal would be better"
@@ -91,7 +112,7 @@ pub(super) enum Radix {
 }
 
 impl Radix {
-    /// Return a reasonable digit group size for this radix.
+    /// Returns a reasonable digit group size for this radix.
     crate fn suggest_grouping(&self) -> usize {
         match *self {
             Radix::Binary | Radix::Hexadecimal => 4,
@@ -135,10 +156,16 @@ impl<'a> DigitInfo<'a> {
             (Some(p), s)
         };
 
+        let len = sans_prefix.len();
         let mut last_d = '\0';
         for (d_idx, d) in sans_prefix.char_indices() {
-            if !float && (d == 'i' || d == 'u') || float && (d == 'f' || d == 'e' || d == 'E') {
-                let suffix_start = if last_d == '_' { d_idx - 1 } else { d_idx };
+            let suffix_start = if last_d == '_' { d_idx - 1 } else { d_idx };
+            if float
+                && (d == 'f'
+                    || is_possible_float_suffix_index(&sans_prefix, suffix_start, len)
+                    || ((d == 'E' || d == 'e') && !has_possible_float_suffix(&sans_prefix)))
+                || !float && (d == 'i' || d == 'u' || is_possible_suffix_index(&sans_prefix, suffix_start, len))
+            {
                 let (digits, suffix) = sans_prefix.split_at(suffix_start);
                 return Self {
                     digits,
@@ -161,7 +188,7 @@ impl<'a> DigitInfo<'a> {
         }
     }
 
-    /// Returns digits grouped in a sensible way.
+    /// Returns literal formatted in a sensible way.
     crate fn grouping_hint(&self) -> String {
         let group_size = self.radix.suggest_grouping();
         if self.digits.contains('.') {
@@ -174,7 +201,7 @@ impl<'a> DigitInfo<'a> {
                 .filter(|&c| c != '_')
                 .collect::<Vec<_>>()
                 .chunks(group_size)
-                .map(|chunk| chunk.into_iter().rev().collect())
+                .map(|chunk| chunk.iter().rev().collect())
                 .rev()
                 .collect::<Vec<String>>()
                 .join("_");
@@ -185,24 +212,50 @@ impl<'a> DigitInfo<'a> {
                 .filter(|&c| c != '_')
                 .collect::<Vec<_>>()
                 .chunks(group_size)
-                .map(|chunk| chunk.into_iter().collect())
+                .map(|chunk| chunk.iter().collect())
+                .collect::<Vec<String>>()
+                .join("_");
+            let suffix_hint = match self.suffix {
+                Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]),
+                Some(suffix) => suffix.to_string(),
+                None => String::new(),
+            };
+            format!("{}.{}{}", int_part_hint, frac_part_hint, suffix_hint)
+        } else if self.float && (self.digits.contains('E') || self.digits.contains('e')) {
+            let which_e = if self.digits.contains('E') { 'E' } else { 'e' };
+            let parts: Vec<&str> = self.digits.split(which_e).collect();
+            let filtered_digits_vec_0 = parts[0].chars().filter(|&c| c != '_').rev().collect::<Vec<_>>();
+            let filtered_digits_vec_1 = parts[1].chars().filter(|&c| c != '_').rev().collect::<Vec<_>>();
+            let before_e_hint = filtered_digits_vec_0
+                .chunks(group_size)
+                .map(|chunk| chunk.iter().rev().collect())
+                .rev()
+                .collect::<Vec<String>>()
+                .join("_");
+            let after_e_hint = filtered_digits_vec_1
+                .chunks(group_size)
+                .map(|chunk| chunk.iter().rev().collect())
+                .rev()
                 .collect::<Vec<String>>()
                 .join("_");
+            let suffix_hint = match self.suffix {
+                Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]),
+                Some(suffix) => suffix.to_string(),
+                None => String::new(),
+            };
             format!(
-                "{}.{}{}",
-                int_part_hint,
-                frac_part_hint,
-                self.suffix.unwrap_or("")
+                "{}{}{}{}{}",
+                self.prefix.unwrap_or(""),
+                before_e_hint,
+                which_e,
+                after_e_hint,
+                suffix_hint
             )
         } else {
-            let filtered_digits_vec = self.digits
-                .chars()
-                .filter(|&c| c != '_')
-                .rev()
-                .collect::<Vec<_>>();
+            let filtered_digits_vec = self.digits.chars().filter(|&c| c != '_').rev().collect::<Vec<_>>();
             let mut hint = filtered_digits_vec
                 .chunks(group_size)
-                .map(|chunk| chunk.into_iter().rev().collect())
+                .map(|chunk| chunk.iter().rev().collect())
                 .rev()
                 .collect::<Vec<String>>()
                 .join("_");
@@ -211,12 +264,12 @@ impl<'a> DigitInfo<'a> {
             if self.radix == Radix::Hexadecimal && nb_digits_to_fill != 0 {
                 hint = format!("{:0>4}{}", &hint[..nb_digits_to_fill], &hint[nb_digits_to_fill..]);
             }
-            format!(
-                "{}{}{}",
-                self.prefix.unwrap_or(""),
-                hint,
-                self.suffix.unwrap_or("")
-            )
+            let suffix_hint = match self.suffix {
+                Some(suffix) if is_mistyped_suffix(suffix) => format!("_i{}", &suffix[1..]),
+                Some(suffix) => suffix.to_string(),
+                None => String::new(),
+            };
+            format!("{}{}{}", self.prefix.unwrap_or(""), hint, suffix_hint)
         }
     }
 }
@@ -226,11 +279,21 @@ enum WarningType {
     InconsistentDigitGrouping,
     LargeDigitGroups,
     DecimalRepresentation,
+    MistypedLiteralSuffix,
 }
 
 impl WarningType {
     crate fn display(&self, grouping_hint: &str, cx: &EarlyContext<'_>, span: syntax_pos::Span) {
         match self {
+            WarningType::MistypedLiteralSuffix => span_lint_and_sugg(
+                cx,
+                MISTYPED_LITERAL_SUFFIXES,
+                span,
+                "mistyped literal suffix",
+                "did you mean to write",
+                grouping_hint.to_string(),
+                Applicability::MaybeIncorrect,
+            ),
             WarningType::UnreadableLiteral => span_lint_and_sugg(
                 cx,
                 UNREADABLE_LITERAL,
@@ -238,6 +301,7 @@ impl WarningType {
                 "long literal lacking separators",
                 "consider",
                 grouping_hint.to_owned(),
+                Applicability::MachineApplicable,
             ),
             WarningType::LargeDigitGroups => span_lint_and_sugg(
                 cx,
@@ -246,6 +310,7 @@ impl WarningType {
                 "digit groups should be smaller",
                 "consider",
                 grouping_hint.to_owned(),
+                Applicability::MachineApplicable,
             ),
             WarningType::InconsistentDigitGrouping => span_lint_and_sugg(
                 cx,
@@ -254,6 +319,7 @@ impl WarningType {
                 "digits grouped inconsistently by underscores",
                 "consider",
                 grouping_hint.to_owned(),
+                Applicability::MachineApplicable,
             ),
             WarningType::DecimalRepresentation => span_lint_and_sugg(
                 cx,
@@ -262,6 +328,7 @@ impl WarningType {
                 "integer literal has a better hexadecimal representation",
                 "consider",
                 grouping_hint.to_owned(),
+                Applicability::MachineApplicable,
             ),
         };
     }
@@ -275,14 +342,19 @@ fn get_lints(&self) -> LintArray {
         lint_array!(
             UNREADABLE_LITERAL,
             INCONSISTENT_DIGIT_GROUPING,
-            LARGE_DIGIT_GROUPS
+            LARGE_DIGIT_GROUPS,
+            MISTYPED_LITERAL_SUFFIXES,
         )
     }
+
+    fn name(&self) -> &'static str {
+        "LiteralDigitGrouping"
+    }
 }
 
 impl EarlyLintPass for LiteralDigitGrouping {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
-        if in_external_macro(cx, expr.span) {
+        if in_external_macro(cx.sess(), expr.span) {
             return;
         }
 
@@ -303,7 +375,7 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
                     if char::to_digit(firstch, 10).is_some();
                     then {
                         let digit_info = DigitInfo::new(&src, false);
-                        let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| {
+                        let _ = Self::do_lint(digit_info.digits, digit_info.suffix).map_err(|warning_type| {
                             warning_type.display(&digit_info.grouping_hint(), cx, lit.span)
                         });
                     }
@@ -325,22 +397,24 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
 
                         // Lint integral and fractional parts separately, and then check consistency of digit
                         // groups if both pass.
-                        let _ = Self::do_lint(parts[0])
+                        let _ = Self::do_lint(parts[0], digit_info.suffix)
                             .map(|integral_group_size| {
                                 if parts.len() > 1 {
                                     // Lint the fractional part of literal just like integral part, but reversed.
                                     let fractional_part = &parts[1].chars().rev().collect::<String>();
-                                    let _ = Self::do_lint(fractional_part)
+                                    let _ = Self::do_lint(fractional_part, None)
                                         .map(|fractional_group_size| {
                                             let consistent = Self::parts_consistent(integral_group_size,
                                                                                     fractional_group_size,
                                                                                     parts[0].len(),
                                                                                     parts[1].len());
-                                            if !consistent {
-                                                WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(),
-                                                cx,
-                                                lit.span);
-                                            }
+                                                if !consistent {
+                                                    WarningType::InconsistentDigitGrouping.display(
+                                                        &digit_info.grouping_hint(),
+                                                        cx,
+                                                        lit.span,
+                                                    );
+                                                }
                                         })
                                     .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(),
                                     cx,
@@ -373,7 +447,12 @@ fn parts_consistent(int_group_size: usize, frac_group_size: usize, int_size: usi
 
     /// Performs lint on `digits` (no decimal point) and returns the group
     /// size on success or `WarningType` when emitting a warning.
-    fn do_lint(digits: &str) -> Result<usize, WarningType> {
+    fn do_lint(digits: &str, suffix: Option<&str>) -> Result<usize, WarningType> {
+        if let Some(suffix) = suffix {
+            if is_mistyped_suffix(suffix) {
+                return Err(WarningType::MistypedLiteralSuffix);
+            }
+        }
         // Grab underscore indices with respect to the units digit.
         let underscore_positions: Vec<usize> = digits
             .chars()
@@ -418,11 +497,15 @@ impl LintPass for LiteralRepresentation {
     fn get_lints(&self) -> LintArray {
         lint_array!(DECIMAL_LITERAL_REPRESENTATION)
     }
+
+    fn name(&self) -> &'static str {
+        "DecimalLiteralRepresentation"
+    }
 }
 
 impl EarlyLintPass for LiteralRepresentation {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
-        if in_external_macro(cx, expr.span) {
+        if in_external_macro(cx.sess(), expr.span) {
             return;
         }
 
@@ -434,9 +517,7 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
 
 impl LiteralRepresentation {
     pub fn new(threshold: u64) -> Self {
-        Self {
-            threshold,
-        }
+        Self { threshold }
     }
     fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
         // Lint integral literals.
@@ -445,23 +526,20 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
             if let Some(src) = snippet_opt(cx, lit.span);
             if let Some(firstch) = src.chars().next();
             if char::to_digit(firstch, 10).is_some();
+            let digit_info = DigitInfo::new(&src, false);
+            if digit_info.radix == Radix::Decimal;
+            if let Ok(val) = digit_info.digits
+                .chars()
+                .filter(|&c| c != '_')
+                .collect::<String>()
+                .parse::<u128>();
+            if val >= u128::from(self.threshold);
             then {
-                let digit_info = DigitInfo::new(&src, false);
-                if digit_info.radix == Radix::Decimal {
-                    let val = digit_info.digits
-                        .chars()
-                        .filter(|&c| c != '_')
-                        .collect::<String>()
-                        .parse::<u128>().unwrap();
-                    if val < u128::from(self.threshold) {
-                        return
-                    }
-                    let hex = format!("{:#X}", val);
-                    let digit_info = DigitInfo::new(&hex[..], false);
-                    let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| {
-                        warning_type.display(&digit_info.grouping_hint(), cx, lit.span)
-                    });
-                }
+                let hex = format!("{:#X}", val);
+                let digit_info = DigitInfo::new(&hex, false);
+                let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| {
+                    warning_type.display(&digit_info.grouping_hint(), cx, lit.span)
+                });
             }
         }
     }
@@ -469,7 +547,12 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
     fn do_lint(digits: &str) -> Result<(), WarningType> {
         if digits.len() == 1 {
             // Lint for 1 digit literals, if someone really sets the threshold that low
-            if digits == "1" || digits == "2" || digits == "4" || digits == "8" || digits == "3" || digits == "7"
+            if digits == "1"
+                || digits == "2"
+                || digits == "4"
+                || digits == "8"
+                || digits == "3"
+                || digits == "7"
                 || digits == "F"
             {
                 return Err(WarningType::DecimalRepresentation);
@@ -478,6 +561,7 @@ fn do_lint(digits: &str) -> Result<(), WarningType> {
             // Lint for Literals with a hex-representation of 2 or 3 digits
             let f = &digits[0..1]; // first digit
             let s = &digits[1..]; // suffix
+
             // Powers of 2
             if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && s.chars().all(|c| c == '0'))
                 // Powers of 2 minus 1
@@ -490,6 +574,7 @@ fn do_lint(digits: &str) -> Result<(), WarningType> {
             let f = &digits[0..1]; // first digit
             let m = &digits[1..digits.len() - 1]; // middle digits, except last
             let s = &digits[1..]; // suffix
+
             // Powers of 2 with a margin of +15/-16
             if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && m.chars().all(|c| c == '0'))
                 || ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && m.chars().all(|c| c == 'F'))
@@ -504,3 +589,23 @@ fn do_lint(digits: &str) -> Result<(), WarningType> {
         Ok(())
     }
 }
+
+fn is_mistyped_suffix(suffix: &str) -> bool {
+    ["_8", "_16", "_32", "_64"].contains(&suffix)
+}
+
+fn is_possible_suffix_index(lit: &str, idx: usize, len: usize) -> bool {
+    ((len > 3 && idx == len - 3) || (len > 2 && idx == len - 2)) && is_mistyped_suffix(lit.split_at(idx).1)
+}
+
+fn is_mistyped_float_suffix(suffix: &str) -> bool {
+    ["_32", "_64"].contains(&suffix)
+}
+
+fn is_possible_float_suffix_index(lit: &str, idx: usize, len: usize) -> bool {
+    (len > 3 && idx == len - 3) && is_mistyped_float_suffix(lit.split_at(idx).1)
+}
+
+fn has_possible_float_suffix(lit: &str) -> bool {
+    lit.ends_with("_32") || lit.ends_with("_64")
+}