Skip to content

Commit

Permalink
literal representation restructure 4
Browse files Browse the repository at this point in the history
Simplify `grouping_hint` by splitting digits into parts and handling
one at a time.

Fixes #4762
  • Loading branch information
Michael Wright committed Nov 13, 2019
1 parent 2e8946a commit 2d244d3
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 66 deletions.
150 changes: 86 additions & 64 deletions clippy_lints/src/literal_representation.rs
Expand Up @@ -190,89 +190,111 @@ impl<'a> DigitInfo<'a> {
}
}

fn split_digit_parts(&self) -> (&str, Option<&str>, Option<(char, &str)>) {
let digits = self.digits;

let mut integer = digits;
let mut fraction = None;
let mut exponent = None;

if self.float {
for (i, c) in digits.char_indices() {
match c {
'.' => {
integer = &digits[..i];
fraction = Some(&digits[i + 1..]);
},
'e' | 'E' => {
if integer.len() > i {
integer = &digits[..i];
} else {
fraction = Some(&digits[integer.len() + 1..i]);
};
exponent = Some((c, &digits[i + 1..]));
break;
},
_ => {},
}
}
}

(integer, fraction, exponent)
}

/// Returns literal formatted in a sensible way.
crate fn grouping_hint(&self) -> String {
let mut output = String::new();

if let Some(prefix) = self.prefix {
output.push_str(prefix);
}

let group_size = self.radix.suggest_grouping();
if self.digits.contains('.') {
let mut parts = self.digits.split('.');
let int_part_hint = parts
.next()
.expect("split always returns at least one element")

let (integer, fraction, exponent) = &self.split_digit_parts();

let int_digits: Vec<_> = integer.chars().rev().filter(|&c| c != '_').collect();
let int_part_hint = int_digits
.chunks(group_size)
.map(|chunk| chunk.iter().rev().collect())
.rev()
.collect::<Vec<String>>()
.join("_");

// Pad leading hexidecimal group with zeros
if self.radix == Radix::Hexadecimal {
debug_assert!(group_size > 0);
let first_group_size = (int_digits.len() + group_size - 1) % group_size + 1;
for _ in 0..group_size - first_group_size {
output.push('0');
}
}

output.push_str(&int_part_hint);

if let Some(fraction) = fraction {
let frac_part_hint = fraction
.chars()
.rev()
.filter(|&c| c != '_')
.collect::<Vec<_>>()
.chunks(group_size)
.map(|chunk| chunk.iter().rev().collect())
.rev()
.map(|chunk| chunk.iter().collect())
.collect::<Vec<String>>()
.join("_");
let frac_part_hint = parts
.next()
.expect("already checked that there is a `.`")

output.push('.');
output.push_str(&frac_part_hint);
}

if let Some((separator, exponent)) = exponent {
let after_e_hint = exponent
.chars()
.rev()
.filter(|&c| c != '_')
.collect::<Vec<_>>()
.chunks(group_size)
.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!(
"{}{}{}{}{}",
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 mut hint = filtered_digits_vec
.chunks(group_size)
.map(|chunk| chunk.iter().rev().collect())
.rev()
.collect::<Vec<String>>()
.join("_");
// Forces hexadecimal values to be grouped by 4 being filled with zeroes (e.g 0x00ab_cdef)
let nb_digits_to_fill = filtered_digits_vec.len() % 4;
if self.radix == Radix::Hexadecimal && nb_digits_to_fill != 0 {
hint = format!("{:0>4}{}", &hint[..nb_digits_to_fill], &hint[nb_digits_to_fill..]);

output.push(*separator);
output.push_str(&after_e_hint);
}

if let Some(suffix) = self.suffix {
if self.float && is_mistyped_float_suffix(suffix) {
output.push_str("_f");
output.push_str(&suffix[1..]);
} else if is_mistyped_suffix(suffix) {
output.push_str("_i");
output.push_str(&suffix[1..]);
} else {
output.push_str(suffix);
}
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)
}

output
}
}

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/inconsistent_digit_grouping.fixed
Expand Up @@ -12,4 +12,10 @@ fn main() {
1.123_456_7_f32,
);
let bad = (123_456, 12_345_678, 1_234_567, 1_234.567_8_f32, 1.234_567_8_f32);

// Test padding
let _ = 0x0010_0000;
let _ = 0x0100_0000;
let _ = 0x1000_0000;
let _ = 0x0001_0000_0000_u64;
}
6 changes: 6 additions & 0 deletions tests/ui/inconsistent_digit_grouping.rs
Expand Up @@ -12,4 +12,10 @@ fn main() {
1.123_456_7_f32,
);
let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32);

// Test padding
let _ = 0x100000;
let _ = 0x1000000;
let _ = 0x10000000;
let _ = 0x100000000_u64;
}
28 changes: 27 additions & 1 deletion tests/ui/inconsistent_digit_grouping.stderr
Expand Up @@ -30,5 +30,31 @@ error: digits grouped inconsistently by underscores
LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32);
| ^^^^^^^^^^^^^^ help: consider: `1.234_567_8_f32`

error: aborting due to 5 previous errors
error: long literal lacking separators
--> $DIR/inconsistent_digit_grouping.rs:17:13
|
LL | let _ = 0x100000;
| ^^^^^^^^ help: consider: `0x0010_0000`
|
= note: `-D clippy::unreadable-literal` implied by `-D warnings`

error: long literal lacking separators
--> $DIR/inconsistent_digit_grouping.rs:18:13
|
LL | let _ = 0x1000000;
| ^^^^^^^^^ help: consider: `0x0100_0000`

error: long literal lacking separators
--> $DIR/inconsistent_digit_grouping.rs:19:13
|
LL | let _ = 0x10000000;
| ^^^^^^^^^^ help: consider: `0x1000_0000`

error: long literal lacking separators
--> $DIR/inconsistent_digit_grouping.rs:20:13
|
LL | let _ = 0x100000000_u64;
| ^^^^^^^^^^^^^^^ help: consider: `0x0001_0000_0000_u64`

error: aborting due to 9 previous errors

2 changes: 2 additions & 0 deletions tests/ui/mistyped_literal_suffix.fixed
Expand Up @@ -19,4 +19,6 @@ fn main() {
#[allow(overflowing_literals)]
let fail28 = 241_251_235E723_f64;
let fail29 = 42_279.911_f32;

let _ = 1.123_45E1_f32;
}
2 changes: 2 additions & 0 deletions tests/ui/mistyped_literal_suffix.rs
Expand Up @@ -19,4 +19,6 @@ fn main() {
#[allow(overflowing_literals)]
let fail28 = 241251235E723_64;
let fail29 = 42279.911_32;

let _ = 1.12345E1_32;
}
8 changes: 7 additions & 1 deletion tests/ui/mistyped_literal_suffix.stderr
Expand Up @@ -72,5 +72,11 @@ error: mistyped literal suffix
LL | let fail29 = 42279.911_32;
| ^^^^^^^^^^^^ help: did you mean to write: `42_279.911_f32`

error: aborting due to 12 previous errors
error: mistyped literal suffix
--> $DIR/mistyped_literal_suffix.rs:23:13
|
LL | let _ = 1.12345E1_32;
| ^^^^^^^^^^^^ help: did you mean to write: `1.123_45E1_f32`

error: aborting due to 13 previous errors

0 comments on commit 2d244d3

Please sign in to comment.