From c1a4b264064cecc9c979650416879670ec64a6cc Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Mon, 26 Aug 2019 16:11:43 +0700 Subject: [PATCH] Cleaner code for unsep literals --- clippy_lints/src/misc_early.rs | 146 ++++++++++---------- tests/ui/literals.stderr | 4 +- tests/ui/unseparated_prefix_literals.fixed | 14 ++ tests/ui/unseparated_prefix_literals.rs | 14 ++ tests/ui/unseparated_prefix_literals.stderr | 31 +++-- 5 files changed, 126 insertions(+), 83 deletions(-) diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index c8ac26044..6ff2a4045 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -6,7 +6,6 @@ use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, Lin use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; -use std::char; use syntax::ast::*; use syntax::source_map::Span; use syntax::visit::{walk_expr, FnKind, Visitor}; @@ -391,92 +390,93 @@ impl EarlyLintPass for MiscEarlyLints { impl MiscEarlyLints { fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { - if_chain! { - if let LitKind::Int(value, ..) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::to_digit(firstch, 10).is_some(); - then { - let mut prev = '\0'; - for (idx, ch) in src.chars().enumerate() { - if ch == 'i' || ch == 'u' { - if prev != '_' { - span_lint_and_sugg( - cx, - UNSEPARATED_LITERAL_SUFFIX, - lit.span, - "integer type suffix should be separated by an underscore", - "add an underscore", - format!("{}_{}", &src[0..idx], &src[idx..]), - Applicability::MachineApplicable, - ); - } - break; - } - prev = ch; + // The `line!()` macro is compiler built-in and a special case for these lints. + let lit_snip = match snippet_opt(cx, lit.span) { + Some(snip) => { + if snip.contains('!') { + return; } - 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 - _ => () - } + snip + }, + _ => return, + }; + + if let LitKind::Int(value, lit_int_type) = lit.node { + let suffix = match lit_int_type { + LitIntType::Signed(ty) => ty.ty_to_string(), + LitIntType::Unsigned(ty) => ty.ty_to_string(), + LitIntType::Unsuffixed => "", + }; + + let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1; + // Do not lint when literal is unsuffixed. + if !suffix.is_empty() && lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' { + span_lint_and_sugg( + cx, + UNSEPARATED_LITERAL_SUFFIX, + lit.span, + "integer type suffix should be separated by an underscore", + "add an underscore", + format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix), + Applicability::MachineApplicable, + ); + } + + if lit_snip.starts_with("0x") { + let mut seen = (false, false); + for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() { + match ch { + b'a'..=b'f' => seen.0 = true, + b'A'..=b'F' => seen.1 = true, + _ => {}, } if seen.0 && seen.1 { - span_lint(cx, MIXED_CASE_HEX_LITERALS, lit.span, - "inconsistent casing in hexadecimal literal"); + span_lint( + cx, + MIXED_CASE_HEX_LITERALS, + lit.span, + "inconsistent casing in hexadecimal literal", + ); + break; } - } else if src.starts_with("0b") || src.starts_with("0o") { - /* nothing to do */ - } else if value != 0 && src.starts_with('0') { - span_lint_and_then(cx, - ZERO_PREFIXED_LITERAL, - lit.span, - "this is a decimal constant", - |db| { + } + } else if lit_snip.starts_with("0b") || lit_snip.starts_with("0o") { + /* nothing to do */ + } else if value != 0 && lit_snip.starts_with('0') { + span_lint_and_then( + cx, + ZERO_PREFIXED_LITERAL, + lit.span, + "this is a decimal constant", + |db| { db.span_suggestion( lit.span, - "if you mean to use a decimal constant, remove the `0` to remove confusion", - src.trim_start_matches(|c| c == '_' || c == '0').to_string(), + "if you mean to use a decimal constant, remove the `0` to avoid confusion", + lit_snip.trim_start_matches(|c| c == '_' || c == '0').to_string(), Applicability::MaybeIncorrect, ); db.span_suggestion( lit.span, "if you mean to use an octal constant, use `0o`", - format!("0o{}", src.trim_start_matches(|c| c == '_' || c == '0')), + format!("0o{}", lit_snip.trim_start_matches(|c| c == '_' || c == '0')), Applicability::MaybeIncorrect, ); - }); - } + }, + ); } - } - if_chain! { - if let LitKind::Float(..) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::to_digit(firstch, 10).is_some(); - then { - let mut prev = '\0'; - for (idx, ch) in src.chars().enumerate() { - if ch == 'f' { - if prev != '_' { - span_lint_and_sugg( - cx, - UNSEPARATED_LITERAL_SUFFIX, - lit.span, - "float type suffix should be separated by an underscore", - "add an underscore", - format!("{}_{}", &src[0..idx], &src[idx..]), - Applicability::MachineApplicable, - ); - } - break; - } - prev = ch; - } + } else if let LitKind::Float(_, float_ty) = lit.node { + let suffix = float_ty.ty_to_string(); + let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1; + if lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' { + span_lint_and_sugg( + cx, + UNSEPARATED_LITERAL_SUFFIX, + lit.span, + "float type suffix should be separated by an underscore", + "add an underscore", + format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix), + Applicability::MachineApplicable, + ); } } } diff --git a/tests/ui/literals.stderr b/tests/ui/literals.stderr index 4f3d430c4..33321440d 100644 --- a/tests/ui/literals.stderr +++ b/tests/ui/literals.stderr @@ -25,7 +25,7 @@ LL | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ | = note: `-D clippy::zero-prefixed-literal` implied by `-D warnings` -help: if you mean to use a decimal constant, remove the `0` to remove confusion +help: if you mean to use a decimal constant, remove the `0` to avoid confusion | LL | let fail_multi_zero = 123usize; | ^^^^^^^^ @@ -39,7 +39,7 @@ error: this is a decimal constant | LL | let fail8 = 0123; | ^^^^ -help: if you mean to use a decimal constant, remove the `0` to remove confusion +help: if you mean to use a decimal constant, remove the `0` to avoid confusion | LL | let fail8 = 123; | ^^^ diff --git a/tests/ui/unseparated_prefix_literals.fixed b/tests/ui/unseparated_prefix_literals.fixed index 1948b18d1..26bc47257 100644 --- a/tests/ui/unseparated_prefix_literals.fixed +++ b/tests/ui/unseparated_prefix_literals.fixed @@ -3,6 +3,12 @@ #![warn(clippy::unseparated_literal_suffix)] #![allow(dead_code)] +macro_rules! lit_from_macro { + () => { + 42_usize + }; +} + fn main() { let _ok1 = 1234_i32; let _ok2 = 1234_isize; @@ -17,4 +23,12 @@ fn main() { let _okf2 = 1_f32; let _failf1 = 1.5_f32; let _failf2 = 1_f32; + + // Test for macro + let _ = lit_from_macro!(); + + // Counter example + let _ = line!(); + // Because `assert!` contains `line!()` macro. + assert_eq!(4897_u32, 32223); } diff --git a/tests/ui/unseparated_prefix_literals.rs b/tests/ui/unseparated_prefix_literals.rs index d70b1cf29..d710ccd1b 100644 --- a/tests/ui/unseparated_prefix_literals.rs +++ b/tests/ui/unseparated_prefix_literals.rs @@ -3,6 +3,12 @@ #![warn(clippy::unseparated_literal_suffix)] #![allow(dead_code)] +macro_rules! lit_from_macro { + () => { + 42usize + }; +} + fn main() { let _ok1 = 1234_i32; let _ok2 = 1234_isize; @@ -17,4 +23,12 @@ fn main() { let _okf2 = 1_f32; let _failf1 = 1.5f32; let _failf2 = 1f32; + + // Test for macro + let _ = lit_from_macro!(); + + // Counter example + let _ = line!(); + // Because `assert!` contains `line!()` macro. + assert_eq!(4897u32, 32223); } diff --git a/tests/ui/unseparated_prefix_literals.stderr b/tests/ui/unseparated_prefix_literals.stderr index 2b8121db3..85f188194 100644 --- a/tests/ui/unseparated_prefix_literals.stderr +++ b/tests/ui/unseparated_prefix_literals.stderr @@ -1,5 +1,5 @@ error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:10:18 + --> $DIR/unseparated_prefix_literals.rs:16:18 | LL | let _fail1 = 1234i32; | ^^^^^^^ help: add an underscore: `1234_i32` @@ -7,40 +7,55 @@ LL | let _fail1 = 1234i32; = note: `-D clippy::unseparated-literal-suffix` implied by `-D warnings` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:11:18 + --> $DIR/unseparated_prefix_literals.rs:17:18 | LL | let _fail2 = 1234u32; | ^^^^^^^ help: add an underscore: `1234_u32` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:12:18 + --> $DIR/unseparated_prefix_literals.rs:18:18 | LL | let _fail3 = 1234isize; | ^^^^^^^^^ help: add an underscore: `1234_isize` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:13:18 + --> $DIR/unseparated_prefix_literals.rs:19:18 | LL | let _fail4 = 1234usize; | ^^^^^^^^^ help: add an underscore: `1234_usize` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:14:18 + --> $DIR/unseparated_prefix_literals.rs:20:18 | LL | let _fail5 = 0x123isize; | ^^^^^^^^^^ help: add an underscore: `0x123_isize` error: float type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:18:19 + --> $DIR/unseparated_prefix_literals.rs:24:19 | LL | let _failf1 = 1.5f32; | ^^^^^^ help: add an underscore: `1.5_f32` error: float type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:19:19 + --> $DIR/unseparated_prefix_literals.rs:25:19 | LL | let _failf2 = 1f32; | ^^^^ help: add an underscore: `1_f32` -error: aborting due to 7 previous errors +error: integer type suffix should be separated by an underscore + --> $DIR/unseparated_prefix_literals.rs:8:9 + | +LL | 42usize + | ^^^^^^^ help: add an underscore: `42_usize` +... +LL | let _ = lit_from_macro!(); + | ----------------- in this macro invocation + +error: integer type suffix should be separated by an underscore + --> $DIR/unseparated_prefix_literals.rs:33:16 + | +LL | assert_eq!(4897u32, 32223); + | ^^^^^^^ help: add an underscore: `4897_u32` + +error: aborting due to 9 previous errors