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

Add lint for misstyped literal casting #3129

Merged
merged 3 commits into from
Sep 7, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ All notable changes to this project will be documented in this file.
[`cmp_owned`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cmp_owned
[`collapsible_if`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#collapsible_if
[`const_static_lifetime`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#const_static_lifetime
[`copy_iterator`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#copy_iterator
[`crosspointer_transmute`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#crosspointer_transmute
[`cyclomatic_complexity`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cyclomatic_complexity
[`decimal_literal_representation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#decimal_literal_representation
Expand Down Expand Up @@ -748,6 +749,7 @@ All notable changes to this project will be documented in this file.
[`misrefactored_assign_op`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#misrefactored_assign_op
[`missing_docs_in_private_items`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
[`missing_inline_in_public_items`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
[`mistyped_literal_suffixes`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`module_inception`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#module_inception
[`modulo_one`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#modulo_one
Expand Down Expand Up @@ -800,6 +802,7 @@ All notable changes to this project will be documented in this file.
[`print_with_newline`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#print_with_newline
[`println_empty_string`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#println_empty_string
[`ptr_arg`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#ptr_arg
[`ptr_offset_with_cast`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
[`pub_enum_variant_names`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#pub_enum_variant_names
[`question_mark`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#question_mark
[`range_minus_one`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#range_minus_one
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 273 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
[There are 275 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
lifetimes::NEEDLESS_LIFETIMES,
literal_representation::INCONSISTENT_DIGIT_GROUPING,
literal_representation::LARGE_DIGIT_GROUPS,
literal_representation::MISTYPED_LITERAL_SUFFIXES,
literal_representation::UNREADABLE_LITERAL,
loops::EMPTY_LOOP,
loops::EXPLICIT_COUNTER_LOOP,
Expand Down Expand Up @@ -869,6 +870,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
infinite_iter::INFINITE_ITER,
inline_fn_without_body::INLINE_FN_WITHOUT_BODY,
invalid_ref::INVALID_REF,
literal_representation::MISTYPED_LITERAL_SUFFIXES,
loops::FOR_LOOP_OVER_OPTION,
loops::FOR_LOOP_OVER_RESULT,
loops::ITER_NEXT_LOOP,
Expand Down
90 changes: 74 additions & 16 deletions clippy_lints/src/literal_representation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@ declare_clippy_lint! {
"long integer literal without underscores"
}

/// **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
/// ```
declare_clippy_lint! {
pub MISTYPED_LITERAL_SUFFIXES,
correctness,
"mistyped literal suffix"
}

/// **What it does:** Warns if an integral or floating-point constant is
/// grouped inconsistently with underscores.
///
Expand Down Expand Up @@ -135,18 +155,24 @@ 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 (digits, suffix) = sans_prefix.split_at(suffix_start);
return Self {
digits,
radix,
prefix,
suffix: Some(suffix),
float,
};
let suffix_start = if last_d == '_' {
d_idx - 1
} else {
d_idx
};
if float && (d == 'f' || d == 'e' || d == 'E') ||
!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,
radix,
prefix,
suffix: Some(suffix),
float,
};
}
last_d = d
}
Expand All @@ -161,7 +187,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('.') {
Expand Down Expand Up @@ -211,11 +237,18 @@ 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..]);
}
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,
self.suffix.unwrap_or("")
suffix_hint
)
}
}
Expand All @@ -226,11 +259,22 @@ 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()
)
},
WarningType::UnreadableLiteral => span_lint_and_sugg(
cx,
UNREADABLE_LITERAL,
Expand Down Expand Up @@ -303,7 +347,7 @@ impl LiteralDigitGrouping {
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)
});
}
Expand All @@ -325,12 +369,12 @@ impl LiteralDigitGrouping {

// 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], None)
.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,
Expand Down Expand Up @@ -373,7 +417,12 @@ impl LiteralDigitGrouping {

/// 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()
Expand Down Expand Up @@ -504,3 +553,12 @@ impl LiteralRepresentation {
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)
}
11 changes: 11 additions & 0 deletions tests/ui/literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,15 @@ fn main() {
let fail11 = 0xabcdeff;
let fail12 = 0xabcabcabcabcabcabc;
let fail13 = 0x1_23456_78901_usize;

let fail14 = 2_32;
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
let fail15 = 4_64;
let fail16 = 7_8;
let fail17 = 23_16;
let ok18 = 23_128;
let fail19 = 12_3456_21;
let fail20 = 2__8;
let fail21 = 4___16;
let fail22 = 3__4___23;
let fail23 = 3__16___23;
}
60 changes: 59 additions & 1 deletion tests/ui/literals.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,63 @@ error: digit groups should be smaller
|
= note: `-D clippy::large-digit-groups` implied by `-D warnings`

error: aborting due to 16 previous errors
error: mistyped literal suffix
--> $DIR/literals.rs:47:18
|
47 | let fail14 = 2_32;
| ^^^^ help: did you mean to write: `2_i32`
|
= note: #[deny(clippy::mistyped_literal_suffixes)] on by default

error: mistyped literal suffix
--> $DIR/literals.rs:48:18
|
48 | let fail15 = 4_64;
| ^^^^ help: did you mean to write: `4_i64`

error: mistyped literal suffix
--> $DIR/literals.rs:49:18
|
49 | let fail16 = 7_8;
| ^^^ help: did you mean to write: `7_i8`

error: mistyped literal suffix
--> $DIR/literals.rs:50:18
|
50 | let fail17 = 23_16;
| ^^^^^ help: did you mean to write: `23_i16`

error: digits grouped inconsistently by underscores
--> $DIR/literals.rs:52:18
|
52 | let fail19 = 12_3456_21;
| ^^^^^^^^^^ help: consider: `12_345_621`
|
= note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings`

error: mistyped literal suffix
--> $DIR/literals.rs:53:18
|
53 | let fail20 = 2__8;
| ^^^^ help: did you mean to write: `2_i8`

error: mistyped literal suffix
--> $DIR/literals.rs:54:18
|
54 | let fail21 = 4___16;
| ^^^^^^ help: did you mean to write: `4_i16`

error: digits grouped inconsistently by underscores
--> $DIR/literals.rs:55:18
|
55 | let fail22 = 3__4___23;
| ^^^^^^^^^ help: consider: `3_423`

error: digits grouped inconsistently by underscores
--> $DIR/literals.rs:56:18
|
56 | let fail23 = 3__16___23;
| ^^^^^^^^^^ help: consider: `31_623`

error: aborting due to 25 previous errors