Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hex bin digit grouping #6183

Merged
merged 5 commits into from
Oct 25, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_grouping
[`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
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_GROUPING,
&loops::EMPTY_LOOP,
&loops::EXPLICIT_COUNTER_LOOP,
&loops::EXPLICIT_INTO_ITER_LOOP,
Expand Down Expand Up @@ -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_GROUPING),
LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
LintId::of(&loops::FOR_KV_MAP),
Expand Down Expand Up @@ -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_GROUPING),
LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::FOR_KV_MAP),
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
Expand Down
42 changes: 39 additions & 3 deletions clippy_lints/src/literal_representation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_GROUPING,
cgm616 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -125,6 +144,7 @@ enum WarningType {
LargeDigitGroups,
DecimalRepresentation,
MistypedLiteralSuffix,
UnusualByteGrouping,
}

impl WarningType {
Expand Down Expand Up @@ -175,6 +195,15 @@ impl WarningType {
suggested_format,
Applicability::MachineApplicable,
),
Self::UnusualByteGrouping => span_lint_and_sugg(
cx,
UNUSUAL_BYTE_GROUPING,
span,
"digits of hex or binary literal not grouped by four",
"consider",
suggested_format,
Applicability::MachineApplicable,
),
};
}
}
Expand All @@ -184,6 +213,7 @@ declare_lint_pass!(LiteralDigitGrouping => [
INCONSISTENT_DIGIT_GROUPING,
LARGE_DIGIT_GROUPS,
MISTYPED_LITERAL_SUFFIXES,
UNUSUAL_BYTE_GROUPING,
]);

impl EarlyLintPass for LiteralDigitGrouping {
Expand Down Expand Up @@ -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,
Expand All @@ -229,6 +259,7 @@ impl LiteralDigitGrouping {
return Err(WarningType::InconsistentDigitGrouping);
};
}

Ok(())
})();

Expand All @@ -237,6 +268,7 @@ impl LiteralDigitGrouping {
let should_warn = match warning_type {
| WarningType::UnreadableLiteral
| WarningType::InconsistentDigitGrouping
| WarningType::UnusualByteGrouping
| WarningType::LargeDigitGroups => {
!in_macro(lit.span)
}
Expand Down Expand Up @@ -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::UnusualByteGrouping);
}

if let Some(second) = groups.next() {
if !groups.all(|x| x == second) || first > second {
Err(WarningType::InconsistentDigitGrouping)
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/utils/numeric_literal.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
}
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2643,6 +2643,13 @@ vec![
deprecation: None,
module: "unused_unit",
},
Lint {
name: "unusual_byte_grouping",
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",
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/large_digit_groups.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 13 additions & 7 deletions tests/ui/large_digit_groups.stderr
Original file line number Diff line number Diff line change
@@ -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-grouping` 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
Expand All @@ -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

13 changes: 9 additions & 4 deletions tests/ui/literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

fn main() {
let ok1 = 0xABCD;
let ok3 = 0xab_cd;
let ok4 = 0xab_cd_i32;
let ok5 = 0xAB_CD_u32;
let ok5 = 0xAB_CD_isize;
let ok3 = 0xabcd;
let ok4 = 0xabcd_i32;
let ok5 = 0xABCD_u32;
let ok5 = 0xABCD_isize;
cgm616 marked this conversation as resolved.
Show resolved Hide resolved
cgm616 marked this conversation as resolved.
Show resolved Hide resolved
let fail1 = 0xabCD;
let fail2 = 0xabCD_u32;
let fail2 = 0xabCD_isize;
Expand All @@ -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;
}
16 changes: 15 additions & 1 deletion tests/ui/literals.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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-grouping` 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

2 changes: 1 addition & 1 deletion tests/ui/unreadable_literal.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() {
let _good = (
0b1011_i64,
0o1_234_u32,
0x1_234_567,
0x0123_4567,
65536,
1_2345_6789,
1234_f32,
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/unreadable_literal.stderr
Original file line number Diff line number Diff line change
@@ -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-grouping` implied by `-D warnings`

error: long literal lacking separators
--> $DIR/unreadable_literal.rs:25:17
|
Expand Down Expand Up @@ -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