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

Lint literal suffixes not separated by underscores (idea also from #703) #1145

Merged
merged 2 commits into from
Aug 7, 2016
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ All notable changes to this project will be documented in this file.
[`mem_forget`]: https://github.com/Manishearth/rust-clippy/wiki#mem_forget
[`min_max`]: https://github.com/Manishearth/rust-clippy/wiki#min_max
[`misrefactored_assign_op`]: https://github.com/Manishearth/rust-clippy/wiki#misrefactored_assign_op
[`mixed_case_hex_literals`]: https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals
[`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one
[`mut_mut`]: https://github.com/Manishearth/rust-clippy/wiki#mut_mut
[`mutex_atomic`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic
Expand Down Expand Up @@ -280,6 +281,7 @@ All notable changes to this project will be documented in this file.
[`unnecessary_operation`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation
[`unneeded_field_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern
[`unsafe_removed_from_name`]: https://github.com/Manishearth/rust-clippy/wiki#unsafe_removed_from_name
[`unseparated_literal_suffix`]: https://github.com/Manishearth/rust-clippy/wiki#unseparated_literal_suffix
[`unstable_as_mut_slice`]: https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice
[`unstable_as_slice`]: https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice
[`unused_collect`]: https://github.com/Manishearth/rust-clippy/wiki#unused_collect
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Table of contents:

## Lints

There are 160 lints included in this crate:
There are 162 lints included in this crate:

name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -98,6 +98,7 @@ name
[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `mem::forget` usage on `Drop` types is likely to cause memory leaks
[min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant
[misrefactored_assign_op](https://github.com/Manishearth/rust-clippy/wiki#misrefactored_assign_op) | warn | having a variable on both sides of an assign op
[mixed_case_hex_literals](https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals) | warn | letter digits in hex literals should be either completely upper- or lowercased
[modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0
[mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...`
[mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic) | warn | using a mutex where an atomic value could be used instead
Expand Down Expand Up @@ -165,6 +166,7 @@ name
[unnecessary_operation](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation) | warn | outer expressions with no effect
[unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern) | warn | Struct fields are bound to a wildcard instead of using `..`
[unsafe_removed_from_name](https://github.com/Manishearth/rust-clippy/wiki#unsafe_removed_from_name) | warn | unsafe removed from name
[unseparated_literal_suffix](https://github.com/Manishearth/rust-clippy/wiki#unseparated_literal_suffix) | allow | literal suffixes should be separated with an underscore
[unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop
[unused_label](https://github.com/Manishearth/rust-clippy/wiki#unused_label) | warn | unused label
[unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions
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 @@ -277,6 +277,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
methods::RESULT_UNWRAP_USED,
methods::WRONG_PUB_SELF_CONVENTION,
misc::USED_UNDERSCORE_BINDING,
misc_early::UNSEPARATED_LITERAL_SUFFIX,
mut_mut::MUT_MUT,
mutex_atomic::MUTEX_INTEGER,
non_expressive_names::SIMILAR_NAMES,
Expand Down Expand Up @@ -377,6 +378,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
misc::TOPLEVEL_REF_ARG,
misc_early::DOUBLE_NEG,
misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
misc_early::MIXED_CASE_HEX_LITERALS,
misc_early::REDUNDANT_CLOSURE_CALL,
misc_early::UNNEEDED_FIELD_PATTERN,
mut_reference::UNNECESSARY_MUT_PASSED,
Expand Down
91 changes: 88 additions & 3 deletions clippy_lints/src/misc_early.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use rustc::lint::*;
use std::collections::HashMap;
use std::char;
use syntax::ast::*;
use syntax::codemap::Span;
use syntax::visit::FnKind;
use utils::{span_lint, span_help_and_lint, snippet, span_lint_and_then};
use utils::{span_lint, span_help_and_lint, snippet, snippet_opt, span_lint_and_then};
/// **What it does:** This lint checks for structure field patterns bound to wildcards.
///
/// **Why is this bad?** Using `..` instead is shorter and leaves the focus on the fields that are actually bound.
Expand Down Expand Up @@ -64,13 +65,44 @@ declare_lint! {
"`--x` is a double negation of `x` and not a pre-decrement as in C or C++"
}

/// **What it does:** Warns on hexadecimal literals with mixed-case letter digits.
///
/// **Why is this bad?** It looks confusing.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let y = 0x1a9BAcD;
/// ```
declare_lint! {
pub MIXED_CASE_HEX_LITERALS, Warn,
"letter digits in hex literals should be either completely upper- or lowercased"
}

/// **What it does:** Warns if literal suffixes are not separated by an underscore.
///
/// **Why is this bad?** It is much less readable.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let y = 123832i32;
/// ```
declare_lint! {
pub UNSEPARATED_LITERAL_SUFFIX, Allow,
"literal suffixes should be separated with an underscore"
}


#[derive(Copy, Clone)]
pub struct MiscEarly;

impl LintPass for MiscEarly {
fn get_lints(&self) -> LintArray {
lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL, DOUBLE_NEG)
lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL,
DOUBLE_NEG, MIXED_CASE_HEX_LITERALS, UNSEPARATED_LITERAL_SUFFIX)
}
}

Expand Down Expand Up @@ -174,7 +206,60 @@ impl EarlyLintPass for MiscEarly {
DOUBLE_NEG,
expr.span,
"`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op");
}
}
}
ExprKind::Lit(ref lit) => {
if_let_chain! {[
let LitKind::Int(..) = lit.node,
let Some(src) = snippet_opt(cx, lit.span),
let Some(firstch) = src.chars().next(),
char::to_digit(firstch, 10).is_some()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because of NaN and Infinity? If so, leave a comment :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, never mind, macros

], {
let mut prev = '\0';
for ch in src.chars() {
if ch == 'i' || ch == 'u' {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also f?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, never mind, you handle those separately.

Not yet fully awake I guess :p

if prev != '_' {
span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span,
"integer type suffix should be separated by an underscore");
}
break;
}
prev = ch;
}
if src.starts_with("0x") {
let mut seen = (false, false);
for ch in src.chars() {
match ch {
'a' ... 'f' => seen.0 = true,
'A' ... 'F' => seen.1 = true,
'i' | 'u' => break, // start of suffix already
_ => ()
}
}
if seen.0 && seen.1 {
span_lint(cx, MIXED_CASE_HEX_LITERALS, lit.span,
"inconsistent casing in hexadecimal literal");
}
}
}}
if_let_chain! {[
let LitKind::Float(..) = lit.node,
let Some(src) = snippet_opt(cx, lit.span),
let Some(firstch) = src.chars().next(),
char::to_digit(firstch, 10).is_some()
], {
let mut prev = '\0';
for ch in src.chars() {
if ch == 'f' {
if prev != '_' {
span_lint(cx, UNSEPARATED_LITERAL_SUFFIX, lit.span,
"float type suffix should be separated by an underscore");
}
break;
}
prev = ch;
}
}}
}
_ => ()
}
Expand Down
Empty file modified tests/compile-fail/doc.rs
100755 → 100644
Empty file.
Empty file modified tests/compile-fail/entry.rs
100755 → 100644
Empty file.
6 changes: 3 additions & 3 deletions tests/compile-fail/filter_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ fn main() {
.map(|x| x * 2)
.collect();

let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter(p).flat_map(q)` on an `Iterator`
let _: Vec<_> = vec![5_i8; 6].into_iter() //~ERROR called `filter(p).flat_map(q)` on an `Iterator`
.filter(|&x| x == 0)
.flat_map(|x| x.checked_mul(2))
.collect();

let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter_map(p).flat_map(q)` on an `Iterator`
let _: Vec<_> = vec![5_i8; 6].into_iter() //~ERROR called `filter_map(p).flat_map(q)` on an `Iterator`
.filter_map(|x| x.checked_mul(2))
.flat_map(|x| x.checked_mul(2))
.collect();

let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter_map(p).map(q)` on an `Iterator`
let _: Vec<_> = vec![5_i8; 6].into_iter() //~ERROR called `filter_map(p).map(q)` on an `Iterator`
.filter_map(|x| x.checked_mul(2))
.map(|x| x.checked_mul(2))
.collect();
Expand Down
Empty file modified tests/compile-fail/if_not_else.rs
100755 → 100644
Empty file.
25 changes: 25 additions & 0 deletions tests/compile-fail/literals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(mixed_case_hex_literals)]
#![deny(unseparated_literal_suffix)]
#![allow(dead_code)]

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 fail1 = 0xabCD; //~ERROR inconsistent casing in hexadecimal literal
let fail2 = 0xabCD_u32; //~ERROR inconsistent casing in hexadecimal literal
let fail2 = 0xabCD_isize; //~ERROR inconsistent casing in hexadecimal literal

let ok6 = 1234_i32;
let ok7 = 1234_f32;
let ok8 = 1234_isize;
let fail3 = 1234i32; //~ERROR integer type suffix should be separated
let fail4 = 1234u32; //~ERROR integer type suffix should be separated
let fail5 = 1234isize; //~ERROR integer type suffix should be separated
let fail6 = 1234usize; //~ERROR integer type suffix should be separated
let fail7 = 1.5f32; //~ERROR float type suffix should be separated
}
2 changes: 1 addition & 1 deletion tests/compile-fail/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn main() {
let y = 1;
let x = y; //~ERROR `x` is shadowed by `y`

let o = Some(1u8);
let o = Some(1_u8);

if let Some(p) = o { assert_eq!(1, p); }
match o {
Expand Down