Skip to content

Commit

Permalink
literal representation restructure 9
Browse files Browse the repository at this point in the history 
Only store valid suffixes (and not mistyped suffixes) in DigitInfo.
Check for mistyped suffixes later and not when DigitInfo is created.
This opens the door to more sophisticated mistyped suffix checks later.
  • Loading branch information
Michael Wright committed Nov 13, 2019
1 parent a58b980 commit a9c5a59
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 77 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/excessive_precision.rs
View file
Expand Up @@ -86,7 +86,7 @@ impl ExcessivePrecision {
if sym_str == s {
None
} else {
let di = super::literal_representation::DigitInfo::new(&s, true);
let di = super::literal_representation::DigitInfo::new(&s, None, true);
Some(di.grouping_hint())
}
} else {
Expand Down
161 changes: 85 additions & 76 deletions clippy_lints/src/literal_representation.rs
View file
Expand Up @@ -138,13 +138,21 @@ pub(super) struct DigitInfo<'a> {

/// The type suffix, including preceding underscore if present.
crate suffix: Option<&'a str>,
/// True for floating-point literals.
crate float: bool,
}

impl<'a> DigitInfo<'a> {
fn from_lit(src: &'a str, lit: &Lit) -> Option<DigitInfo<'a>> {
if lit.kind.is_numeric() && src.chars().next().map_or(false, |c| c.is_digit(10)) {
let (unsuffixed, suffix) = split_suffix(&src, &lit.kind);
let float = if let LitKind::Float(..) = lit.kind { true } else { false };
Some(DigitInfo::new(unsuffixed, suffix, float))
} else {
None
}
}

#[must_use]
crate fn new(lit: &'a str, float: bool) -> Self {
crate fn new(lit: &'a str, suffix: Option<&'a str>, float: bool) -> Self {
// Determine delimiter for radix prefix, if present, and radix.
let radix = if lit.starts_with("0x") {
Radix::Hexadecimal
Expand All @@ -157,35 +165,19 @@ impl<'a> DigitInfo<'a> {
};

// Grab part of the literal after prefix, if present.
let (prefix, sans_prefix) = if let Radix::Decimal = radix {
let (prefix, mut sans_prefix) = if let Radix::Decimal = radix {
(None, lit)
} else {
let (p, s) = lit.split_at(2);
(Some(p), s)
};

let mut digits = sans_prefix;
let mut suffix = None;

let len = sans_prefix.len();
let mut last_d = '\0';
for (d_idx, d) in sans_prefix.char_indices() {
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 (d, s) = sans_prefix.split_at(suffix_start);
digits = d;
suffix = Some(s);
break;
}
last_d = d
if suffix.is_some() && sans_prefix.ends_with('_') {
// The '_' before the suffix isn't part of the digits
sans_prefix = &sans_prefix[..sans_prefix.len() - 1];
}

let (integer, fraction, exponent) = Self::split_digit_parts(digits, float);
let (integer, fraction, exponent) = Self::split_digit_parts(sans_prefix, float);

Self {
radix,
Expand All @@ -194,7 +186,6 @@ impl<'a> DigitInfo<'a> {
fraction,
exponent,
suffix,
float,
}
}

Expand Down Expand Up @@ -256,15 +247,8 @@ impl<'a> DigitInfo<'a> {
}

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);
}
output.push('_');
output.push_str(suffix);
}

output
Expand Down Expand Up @@ -303,6 +287,34 @@ impl<'a> DigitInfo<'a> {
}
}

fn split_suffix<'a>(src: &'a str, lit_kind: &LitKind) -> (&'a str, Option<&'a str>) {
debug_assert!(lit_kind.is_numeric());
if let Some(suffix_length) = lit_suffix_length(lit_kind) {
let (unsuffixed, suffix) = src.split_at(src.len() - suffix_length);
(unsuffixed, Some(suffix))
} else {
(src, None)
}
}

fn lit_suffix_length(lit_kind: &LitKind) -> Option<usize> {
debug_assert!(lit_kind.is_numeric());
let suffix = match lit_kind {
LitKind::Int(_, int_lit_kind) => match int_lit_kind {
LitIntType::Signed(int_ty) => Some(int_ty.name_str()),
LitIntType::Unsigned(uint_ty) => Some(uint_ty.name_str()),
LitIntType::Unsuffixed => None,
},
LitKind::Float(_, float_lit_kind) => match float_lit_kind {
LitFloatType::Suffixed(float_ty) => Some(float_ty.name_str()),
LitFloatType::Unsuffixed => None,
},
_ => None,
};

suffix.map(str::len)
}

enum WarningType {
UnreadableLiteral,
InconsistentDigitGrouping,
Expand Down Expand Up @@ -388,22 +400,13 @@ impl LiteralDigitGrouping {

if_chain! {
if let Some(src) = snippet_opt(cx, lit.span);
if let Some(firstch) = src.chars().next();
if char::is_digit(firstch, 10);
if let Some(mut digit_info) = DigitInfo::from_lit(&src, &lit);
then {

let digit_info = match lit.kind {
LitKind::Int(..) => DigitInfo::new(&src, false),
LitKind::Float(..) => DigitInfo::new(&src, true),
_ => return,
};
if !Self::check_for_mistyped_suffix(cx, lit.span, &mut digit_info) {
return;
}

let result = (|| {
if let Some(suffix) = digit_info.suffix {
if is_mistyped_suffix(suffix) {
return Err(WarningType::MistypedLiteralSuffix);
}
}

let integral_group_size = Self::get_group_size(digit_info.integer.split('_'), in_macro)?;
if let Some(fraction) = digit_info.fraction {
Expand All @@ -428,6 +431,39 @@ impl LiteralDigitGrouping {
}
}

// Returns `false` if the check fails
fn check_for_mistyped_suffix(
cx: &EarlyContext<'_>,
span: syntax_pos::Span,
digit_info: &mut DigitInfo<'_>,
) -> bool {
if digit_info.suffix.is_some() {
return true;
}

let (part, mistyped_suffixes, missing_char) = if let Some((_, exponent)) = &mut digit_info.exponent {
(exponent, &["32", "64"][..], 'f')
} else if let Some(fraction) = &mut digit_info.fraction {
(fraction, &["32", "64"][..], 'f')
} else {
(&mut digit_info.integer, &["8", "16", "32", "64"][..], 'i')
};

let mut split = part.rsplit('_');
let last_group = split.next().expect("At least one group");
if split.next().is_some() && mistyped_suffixes.contains(&last_group) {
*part = &part[..part.len() - last_group.len()];
let mut hint = digit_info.grouping_hint();
hint.push('_');
hint.push(missing_char);
hint.push_str(last_group);
WarningType::MistypedLiteralSuffix.display(&hint, cx, span);
false
} else {
true
}
}

/// Given the sizes of the digit groups of both integral and fractional
/// parts, and the length
/// of both parts, determine if the digits have been grouped consistently.
Expand Down Expand Up @@ -503,14 +539,12 @@ impl DecimalLiteralRepresentation {
if_chain! {
if let LitKind::Int(val, _) = lit.kind;
if let Some(src) = snippet_opt(cx, lit.span);
if let Some(firstch) = src.chars().next();
if char::is_digit(firstch, 10);
let digit_info = DigitInfo::new(&src, false);
if let Some(digit_info) = DigitInfo::from_lit(&src, &lit);
if digit_info.radix == Radix::Decimal;
if val >= u128::from(self.threshold);
then {
let hex = format!("{:#X}", val);
let digit_info = DigitInfo::new(&hex, false);
let digit_info = DigitInfo::new(&hex, None, false);
let _ = Self::do_lint(digit_info.integer).map_err(|warning_type| {
warning_type.display(&digit_info.grouping_hint(), cx, lit.span)
});
Expand Down Expand Up @@ -563,28 +597,3 @@ impl DecimalLiteralRepresentation {
Ok(())
}
}

#[must_use]
fn is_mistyped_suffix(suffix: &str) -> bool {
["_8", "_16", "_32", "_64"].contains(&suffix)
}

#[must_use]
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)
}

#[must_use]
fn is_mistyped_float_suffix(suffix: &str) -> bool {
["_32", "_64"].contains(&suffix)
}

#[must_use]
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)
}

#[must_use]
fn has_possible_float_suffix(lit: &str) -> bool {
lit.ends_with("_32") || lit.ends_with("_64")
}

0 comments on commit a9c5a59

Please sign in to comment.