From ab58331f220a77538eb02ad1bef828ca7230e7ac Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 5 Aug 2016 18:30:07 +0200 Subject: [PATCH 1/2] Lint inconsistent casing in hex literals (closes #703) --- CHANGELOG.md | 1 + README.md | 3 ++- clippy_lints/src/lib.rs | 1 + clippy_lints/src/misc_early.rs | 43 +++++++++++++++++++++++++++++++--- tests/compile-fail/literals.rs | 15 ++++++++++++ 5 files changed, 59 insertions(+), 4 deletions(-) create mode 100755 tests/compile-fail/literals.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 10de2fc4b23..4081d3d25c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/README.md b/README.md index 584db6a68b1..7edb751fb44 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 160 lints included in this crate: +There are 161 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7308a656590..eaac42c5c17 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -377,6 +377,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, diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index a8127ef88af..2d6b0a357da 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; 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. @@ -64,13 +64,29 @@ 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" +} + #[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) } } @@ -174,7 +190,28 @@ 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), + 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"); + } + }} } _ => () } diff --git a/tests/compile-fail/literals.rs b/tests/compile-fail/literals.rs new file mode 100755 index 00000000000..fb031ea7c96 --- /dev/null +++ b/tests/compile-fail/literals.rs @@ -0,0 +1,15 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(mixed_case_hex_literals)] +#![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 +} From 2f8247ada5bc1d88653605373e953e8e8dd814fd Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 5 Aug 2016 18:50:02 +0200 Subject: [PATCH 2/2] Lint literal suffixes not separated by underscores (see #703) --- CHANGELOG.md | 1 + README.md | 3 +- clippy_lints/src/lib.rs | 1 + clippy_lints/src/misc_early.rs | 70 +++++++++++++++++++++++----- tests/compile-fail/doc.rs | 0 tests/compile-fail/entry.rs | 0 tests/compile-fail/filter_methods.rs | 6 +-- tests/compile-fail/if_not_else.rs | 0 tests/compile-fail/literals.rs | 10 ++++ tests/compile-fail/shadow.rs | 2 +- 10 files changed, 77 insertions(+), 16 deletions(-) mode change 100755 => 100644 tests/compile-fail/doc.rs mode change 100755 => 100644 tests/compile-fail/entry.rs mode change 100755 => 100644 tests/compile-fail/if_not_else.rs mode change 100755 => 100644 tests/compile-fail/literals.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4081d3d25c0..d83611f1fd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -281,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 diff --git a/README.md b/README.md index 7edb751fb44..0c8e3911219 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 161 lints included in this crate: +There are 162 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -166,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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index eaac42c5c17..08f5e6b242d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 2d6b0a357da..2c058f3bcc2 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -1,5 +1,6 @@ use rustc::lint::*; use std::collections::HashMap; +use std::char; use syntax::ast::*; use syntax::codemap::Span; use syntax::visit::FnKind; @@ -79,6 +80,21 @@ declare_lint! { "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; @@ -86,7 +102,7 @@ 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, MIXED_CASE_HEX_LITERALS) + DOUBLE_NEG, MIXED_CASE_HEX_LITERALS, UNSEPARATED_LITERAL_SUFFIX) } } @@ -196,20 +212,52 @@ impl EarlyLintPass for MiscEarly { if_let_chain! {[ let LitKind::Int(..) = lit.node, let Some(src) = snippet_opt(cx, lit.span), - src.starts_with("0x") + let Some(firstch) = src.chars().next(), + char::to_digit(firstch, 10).is_some() ], { - let mut seen = (false, false); + let mut prev = '\0'; 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 ch == 'i' || ch == 'u' { + 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 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; } }} } diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/entry.rs b/tests/compile-fail/entry.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/filter_methods.rs b/tests/compile-fail/filter_methods.rs index 743c3c15aeb..bee9688f581 100644 --- a/tests/compile-fail/filter_methods.rs +++ b/tests/compile-fail/filter_methods.rs @@ -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(); diff --git a/tests/compile-fail/if_not_else.rs b/tests/compile-fail/if_not_else.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/literals.rs b/tests/compile-fail/literals.rs old mode 100755 new mode 100644 index fb031ea7c96..7645fb56e20 --- a/tests/compile-fail/literals.rs +++ b/tests/compile-fail/literals.rs @@ -1,6 +1,7 @@ #![feature(plugin)] #![plugin(clippy)] #![deny(mixed_case_hex_literals)] +#![deny(unseparated_literal_suffix)] #![allow(dead_code)] fn main() { @@ -12,4 +13,13 @@ fn main() { 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 } diff --git a/tests/compile-fail/shadow.rs b/tests/compile-fail/shadow.rs index 1cfcff74a44..1200e25cdbc 100644 --- a/tests/compile-fail/shadow.rs +++ b/tests/compile-fail/shadow.rs @@ -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 {