Auto merge of rust-lang#6183 - cgm616:hex_bin_digit_grouping, r=flip1995

Hex bin digit grouping

This revives and updates an old pr (rust-lang#3391) for the current API.

Closes rust-lang#2538.

---

*Please keep the line below*
changelog: Add [`unusual_byte_groupings`] lint.

Author: bors

CHANGELOG.md

@@ -2017,6 +2017,7 @@ Released 2018-09-13
 [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
 [`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self
 [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
+[`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings
 [`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result
 [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
 [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug

clippy_lints/src/lib.rs

@@ -623,6 +623,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &literal_representation::LARGE_DIGIT_GROUPS,
         &literal_representation::MISTYPED_LITERAL_SUFFIXES,
         &literal_representation::UNREADABLE_LITERAL,
+        &literal_representation::UNUSUAL_BYTE_GROUPINGS,
         &loops::EMPTY_LOOP,
         &loops::EXPLICIT_COUNTER_LOOP,
         &loops::EXPLICIT_INTO_ITER_LOOP,
@@ -1365,6 +1366,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
         LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
         LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES),
+        LintId::of(&literal_representation::UNUSUAL_BYTE_GROUPINGS),
         LintId::of(&loops::EMPTY_LOOP),
         LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
         LintId::of(&loops::FOR_KV_MAP),
@@ -1587,6 +1589,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
         LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
+        LintId::of(&literal_representation::UNUSUAL_BYTE_GROUPINGS),
         LintId::of(&loops::EMPTY_LOOP),
         LintId::of(&loops::FOR_KV_MAP),
         LintId::of(&loops::NEEDLESS_RANGE_LOOP),

clippy_lints/src/literal_representation.rs

@@ -82,6 +82,25 @@ declare_clippy_lint! {
     "integer literals with digits grouped inconsistently"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Warns if hexadecimal or binary literals are not grouped
+    /// by nibble or byte.
+    ///
+    /// **Why is this bad?** Negatively impacts readability.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let x: u32 = 0xFFF_FFF;
+    /// let y: u8 = 0b01_011_101;
+    /// ```
+    pub UNUSUAL_BYTE_GROUPINGS,
+    style,
+    "binary or hex literals that aren't grouped by four"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Warns if the digits of an integral or floating-point
     /// constant are grouped into groups that
@@ -125,6 +144,7 @@ enum WarningType {
     LargeDigitGroups,
     DecimalRepresentation,
     MistypedLiteralSuffix,
+    UnusualByteGroupings,
 }
 
 impl WarningType {
@@ -175,6 +195,15 @@ impl WarningType {
                 suggested_format,
                 Applicability::MachineApplicable,
             ),
+            Self::UnusualByteGroupings => span_lint_and_sugg(
+                cx,
+                UNUSUAL_BYTE_GROUPINGS,
+                span,
+                "digits of hex or binary literal not grouped by four",
+                "consider",
+                suggested_format,
+                Applicability::MachineApplicable,
+            ),
         };
     }
 }
@@ -184,6 +213,7 @@ declare_lint_pass!(LiteralDigitGrouping => [
     INCONSISTENT_DIGIT_GROUPING,
     LARGE_DIGIT_GROUPS,
     MISTYPED_LITERAL_SUFFIXES,
+    UNUSUAL_BYTE_GROUPINGS,
 ]);
 
 impl EarlyLintPass for LiteralDigitGrouping {
@@ -217,9 +247,9 @@ impl LiteralDigitGrouping {
 
                 let result = (|| {
 
-                    let integral_group_size = Self::get_group_size(num_lit.integer.split('_'))?;
+                    let integral_group_size = Self::get_group_size(num_lit.integer.split('_'), num_lit.radix)?;
                     if let Some(fraction) = num_lit.fraction {
-                        let fractional_group_size = Self::get_group_size(fraction.rsplit('_'))?;
+                        let fractional_group_size = Self::get_group_size(fraction.rsplit('_'), num_lit.radix)?;
 
                         let consistent = Self::parts_consistent(integral_group_size,
                                                                 fractional_group_size,
@@ -229,6 +259,7 @@ impl LiteralDigitGrouping {
                             return Err(WarningType::InconsistentDigitGrouping);
                         };
                     }
+
                     Ok(())
                 })();
 
@@ -237,6 +268,7 @@ impl LiteralDigitGrouping {
                     let should_warn = match warning_type {
                         | WarningType::UnreadableLiteral
                         | WarningType::InconsistentDigitGrouping
+                        | WarningType::UnusualByteGroupings
                         | WarningType::LargeDigitGroups => {
                             !in_macro(lit.span)
                         }
@@ -331,11 +363,15 @@ impl LiteralDigitGrouping {
 
     /// Returns the size of the digit groups (or None if ungrouped) if successful,
     /// otherwise returns a `WarningType` for linting.
-    fn get_group_size<'a>(groups: impl Iterator<Item = &'a str>) -> Result<Option<usize>, WarningType> {
+    fn get_group_size<'a>(groups: impl Iterator<Item = &'a str>, radix: Radix) -> Result<Option<usize>, WarningType> {
         let mut groups = groups.map(str::len);
 
         let first = groups.next().expect("At least one group");
 
+        if (radix == Radix::Binary || radix == Radix::Hexadecimal) && groups.any(|i| i != 4 && i != 2) {
+            return Err(WarningType::UnusualByteGroupings);
+        }
+
         if let Some(second) = groups.next() {
             if !groups.all(|x| x == second) || first > second {
                 Err(WarningType::InconsistentDigitGrouping)

clippy_lints/src/utils/numeric_literal.rs

@@ -1,6 +1,6 @@
 use rustc_ast::ast::{Lit, LitFloatType, LitIntType, LitKind};
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Copy, Clone)]
 pub enum Radix {
     Binary,
     Octal,
@@ -11,8 +11,8 @@ pub enum Radix {
 impl Radix {
     /// Returns a reasonable digit group size for this radix.
     #[must_use]
-    fn suggest_grouping(&self) -> usize {
-        match *self {
+    fn suggest_grouping(self) -> usize {
+        match self {
             Self::Binary | Self::Hexadecimal => 4,
             Self::Octal | Self::Decimal => 3,
         }

src/lintlist/mod.rs

@@ -2643,6 +2643,13 @@ vec![
         deprecation: None,
         module: "unused_unit",
     },
+    Lint {
+        name: "unusual_byte_groupings",
+        group: "style",
+        desc: "binary or hex literals that aren\'t grouped by four",
+        deprecation: None,
+        module: "literal_representation",
+    },
     Lint {
         name: "unwrap_in_result",
         group: "restriction",

tests/ui/large_digit_groups.fixed

@@ -11,7 +11,7 @@ fn main() {
     let _good = (
         0b1011_i64,
         0o1_234_u32,
-        0x1_234_567,
+        0x0123_4567,
         1_2345_6789,
         1234_f32,
         1_234.12_f32,

tests/ui/large_digit_groups.stderr

@@ -1,24 +1,30 @@
-error: digit groups should be smaller
+error: digits of hex or binary literal not grouped by four
+  --> $DIR/large_digit_groups.rs:14:9
+   |
+LL |         0x1_234_567,
+   |         ^^^^^^^^^^^ help: consider: `0x0123_4567`
+   |
+   = note: `-D clippy::unusual-byte-groupings` implied by `-D warnings`
+
+error: digits of hex or binary literal not grouped by four
   --> $DIR/large_digit_groups.rs:22:9
    |
 LL |         0b1_10110_i64,
    |         ^^^^^^^^^^^^^ help: consider: `0b11_0110_i64`
-   |
-   = note: `-D clippy::large-digit-groups` implied by `-D warnings`
 
-error: digits grouped inconsistently by underscores
+error: digits of hex or binary literal not grouped by four
   --> $DIR/large_digit_groups.rs:23:9
    |
 LL |         0xd_e_adbee_f_usize,
    |         ^^^^^^^^^^^^^^^^^^^ help: consider: `0xdead_beef_usize`
-   |
-   = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings`
 
 error: digit groups should be smaller
   --> $DIR/large_digit_groups.rs:24:9
    |
 LL |         1_23456_f32,
    |         ^^^^^^^^^^^ help: consider: `123_456_f32`
+   |
+   = note: `-D clippy::large-digit-groups` implied by `-D warnings`
 
 error: digit groups should be smaller
   --> $DIR/large_digit_groups.rs:25:9
@@ -38,5 +44,5 @@ error: digit groups should be smaller
 LL |         1_23456.12345_6_f64,
    |         ^^^^^^^^^^^^^^^^^^^ help: consider: `123_456.123_456_f64`
 
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors
 

tests/ui/literals.rs

@@ -33,4 +33,9 @@ fn main() {
     let fail19 = 12_3456_21;
     let fail22 = 3__4___23;
     let fail23 = 3__16___23;
+
+    let fail24 = 0xAB_ABC_AB;
+    let fail25 = 0b01_100_101;
+    let ok26 = 0x6_A0_BF;
+    let ok27 = 0b1_0010_0101;
 }

tests/ui/literals.stderr

@@ -69,5 +69,19 @@ error: digits grouped inconsistently by underscores
 LL |     let fail23 = 3__16___23;
    |                  ^^^^^^^^^^ help: consider: `31_623`
 
-error: aborting due to 8 previous errors
+error: digits of hex or binary literal not grouped by four
+  --> $DIR/literals.rs:37:18
+   |
+LL |     let fail24 = 0xAB_ABC_AB;
+   |                  ^^^^^^^^^^^ help: consider: `0x0ABA_BCAB`
+   |
+   = note: `-D clippy::unusual-byte-groupings` implied by `-D warnings`
+
+error: digits of hex or binary literal not grouped by four
+  --> $DIR/literals.rs:38:18
+   |
+LL |     let fail25 = 0b01_100_101;
+   |                  ^^^^^^^^^^^^ help: consider: `0b0110_0101`
+
+error: aborting due to 10 previous errors
 

tests/ui/unreadable_literal.fixed

@@ -14,7 +14,7 @@ fn main() {
     let _good = (
         0b1011_i64,
         0o1_234_u32,
-        0x1_234_567,
+        0x0123_4567,
         65536,
         1_2345_6789,
         1234_f32,

tests/ui/unreadable_literal.stderr

@@ -1,3 +1,11 @@
+error: digits of hex or binary literal not grouped by four
+  --> $DIR/unreadable_literal.rs:17:9
+   |
+LL |         0x1_234_567,
+   |         ^^^^^^^^^^^ help: consider: `0x0123_4567`
+   |
+   = note: `-D clippy::unusual-byte-groupings` implied by `-D warnings`
+
 error: long literal lacking separators
   --> $DIR/unreadable_literal.rs:25:17
    |
@@ -54,5 +62,5 @@ error: long literal lacking separators
 LL |     let _fail12: i128 = 0xabcabcabcabcabcabc;
    |                         ^^^^^^^^^^^^^^^^^^^^ help: consider: `0x00ab_cabc_abca_bcab_cabc`
 
-error: aborting due to 9 previous errors
+error: aborting due to 10 previous errors