Auto merge of #4421 - lzutao:unsep_literal, r=flip1995

bors · bors · commit b8e5e6f3c204 · 2019-08-26T10:32:24.000Z
Cleaner code for unsep literals Continuing discussion in #4401 (comment) changelog: none r? @flip1995
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -1,46 +1,61 @@
 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`
    |
    = 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
 

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ LL \| let fail_multi_zero = 000_123usize;`
`25`	`25`	`\| ^^^^^^^^^^^^`
`26`	`26`	`\|`
`27`	`27`	= note: `-D clippy::zero-prefixed-literal` implied by `-D warnings`
`28`		-help: if you mean to use a decimal constant, remove the `0` to remove confusion
	`28`	+help: if you mean to use a decimal constant, remove the `0` to avoid confusion
`29`	`29`	`\|`
`30`	`30`	`LL \| let fail_multi_zero = 123usize;`
`31`	`31`	`\| ^^^^^^^^`
`@@ -39,7 +39,7 @@ error: this is a decimal constant`
`39`	`39`	`\|`
`40`	`40`	`LL \| let fail8 = 0123;`
`41`	`41`	`\| ^^^^`
`42`		-help: if you mean to use a decimal constant, remove the `0` to remove confusion
	`42`	+help: if you mean to use a decimal constant, remove the `0` to avoid confusion
`43`	`43`	`\|`
`44`	`44`	`LL \| let fail8 = 123;`
`45`	`45`	`\| ^^^`