Auto merge of #6421 - xFrednet:4176-unreadable-literal-lint-fractal-o…

…ption, r=Manishearth Added a lint-fraction-readability flag to the configuration This adds an option to disable the `unreadable_literal` lint for floats with a longer fraction. This allows users to write `0.100200300` without getting a warning. Fixes #4176 I have some open questions about this PR: 1. I've named the option `lint-fraction-readability` is this a good name or should I rename it to something else? 2. What should the default configuration value be? * The current default value is `true` as this was also the previous default. 3. Do I have to document this new option somewhere else or will it be extracted from the code comment? 4. The current fix option will also rewrite the fraction if the integer part violates the `unreadable_literal` lint it would otherwise also trigger the `inconsistent_digit_grouping` lint. Is this also okay? * `1.100200300` will be unaffected by the fix function * `100200300.100200300` will be effected and fixed to `100_200_300.100_200_300` --- The project needed some getting used to but I'm happy with the result. A big thank you to `@flip1995` for giving me some pointers for this implementation and to everyone for the great introduction documentation! --- changelog: Added the `unreadable-literal-lint-fractions` configuration to disable the `unreadable_literal` lint for fractions
rust-lang · Dec 6, 2020 · c1664c5 · c1664c5
2 parents 8d678bc + 898b7c5
commit c1664c5
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 37 deletions.
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -1138,7 +1138,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata);
     store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions);
     store.register_late_pass(|| box wildcard_dependencies::WildcardDependencies);
-    store.register_early_pass(|| box literal_representation::LiteralDigitGrouping);
+    let literal_representation_lint_fraction_readability = conf.unreadable_literal_lint_fractions;
+    store.register_early_pass(move || box literal_representation::LiteralDigitGrouping::new(literal_representation_lint_fraction_readability));
     let literal_representation_threshold = conf.literal_representation_threshold;
     store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
     let enum_variant_name_threshold = conf.enum_variant_name_threshold;

diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs
@@ -11,7 +11,7 @@ use rustc_ast::ast::{Expr, ExprKind, Lit, LitKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
-use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 
 declare_clippy_lint! {
     /// **What it does:** Warns if a long integral or floating-point constant does
@@ -32,7 +32,7 @@ declare_clippy_lint! {
     /// ```
     pub UNREADABLE_LITERAL,
     pedantic,
-    "long integer literal without underscores"
+    "long literal without underscores"
 }
 
 declare_clippy_lint! {
@@ -208,7 +208,13 @@ impl WarningType {
     }
 }
 
-declare_lint_pass!(LiteralDigitGrouping => [
+#[allow(clippy::module_name_repetitions)]
+#[derive(Copy, Clone)]
+pub struct LiteralDigitGrouping {
+    lint_fraction_readability: bool,
+}
+
+impl_lint_pass!(LiteralDigitGrouping => [
     UNREADABLE_LITERAL,
     INCONSISTENT_DIGIT_GROUPING,
     LARGE_DIGIT_GROUPS,
@@ -223,7 +229,7 @@ impl EarlyLintPass for LiteralDigitGrouping {
         }
 
         if let ExprKind::Lit(ref lit) = expr.kind {
-            Self::check_lit(cx, lit)
+            self.check_lit(cx, lit)
         }
     }
 }
@@ -232,7 +238,13 @@ impl EarlyLintPass for LiteralDigitGrouping {
 const UUID_GROUP_LENS: [usize; 5] = [8, 4, 4, 4, 12];
 
 impl LiteralDigitGrouping {
-    fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
+    pub fn new(lint_fraction_readability: bool) -> Self {
+        Self {
+            lint_fraction_readability,
+        }
+    }
+
+    fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
         if_chain! {
             if let Some(src) = snippet_opt(cx, lit.span);
             if let Some(mut num_lit) = NumericLiteral::from_lit(&src, &lit);
@@ -247,9 +259,12 @@ impl LiteralDigitGrouping {
 
                 let result = (|| {
 
-                    let integral_group_size = Self::get_group_size(num_lit.integer.split('_'), num_lit.radix)?;
+                    let integral_group_size = Self::get_group_size(num_lit.integer.split('_'), num_lit.radix, true)?;
                     if let Some(fraction) = num_lit.fraction {
-                        let fractional_group_size = Self::get_group_size(fraction.rsplit('_'), num_lit.radix)?;
+                        let fractional_group_size = Self::get_group_size(
+                            fraction.rsplit('_'),
+                            num_lit.radix,
+                            self.lint_fraction_readability)?;
 
                         let consistent = Self::parts_consistent(integral_group_size,
                                                                 fractional_group_size,
@@ -363,7 +378,11 @@ impl LiteralDigitGrouping {
 
     /// Returns the size of the digit groups (or None if ungrouped) if successful,
     /// otherwise returns a `WarningType` for linting.
-    fn get_group_size<'a>(groups: impl Iterator<Item = &'a str>, radix: Radix) -> Result<Option<usize>, WarningType> {
+    fn get_group_size<'a>(
+        groups: impl Iterator<Item = &'a str>,
+        radix: Radix,
+        lint_unreadable: bool,
+    ) -> Result<Option<usize>, WarningType> {
         let mut groups = groups.map(str::len);
 
         let first = groups.next().expect("At least one group");
@@ -380,7 +399,7 @@ impl LiteralDigitGrouping {
             } else {
                 Ok(Some(second))
             }
-        } else if first > 5 {
+        } else if first > 5 && lint_unreadable {
             Err(WarningType::UnreadableLiteral)
         } else {
             Ok(None)

diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -170,6 +170,8 @@ define_Conf! {
     (warn_on_all_wildcard_imports, "warn_on_all_wildcard_imports": bool, false),
     /// Lint: DISALLOWED_METHOD. The list of blacklisted methods to lint about. NB: `bar` is not here since it has legitimate uses
     (disallowed_methods, "disallowed_methods": Vec<String>, Vec::<String>::new()),
+    /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators.
+    (unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true),
 }
 
 impl Default for Conf {

diff --git a/tests/ui-toml/lint_decimal_readability/clippy.toml b/tests/ui-toml/lint_decimal_readability/clippy.toml
@@ -0,0 +1 @@
+unreadable-literal-lint-fractions = false
diff --git a/tests/ui-toml/lint_decimal_readability/test.rs b/tests/ui-toml/lint_decimal_readability/test.rs
@@ -0,0 +1,22 @@
+#[deny(clippy::unreadable_literal)]
+
+fn allow_inconsistent_digit_grouping() {
+    #![allow(clippy::inconsistent_digit_grouping)]
+    let _pass1 = 100_200_300.123456789;
+}
+
+fn main() {
+    allow_inconsistent_digit_grouping();
+
+    let _pass1 = 100_200_300.100_200_300;
+    let _pass2 = 1.123456789;
+    let _pass3 = 1.0;
+    let _pass4 = 10000.00001;
+    let _pass5 = 1.123456789e1;
+
+    // due to clippy::inconsistent-digit-grouping
+    let _fail1 = 100_200_300.123456789;
+
+    // fail due to the integer part
+    let _fail2 = 100200300.300200100;
+}
diff --git a/tests/ui-toml/lint_decimal_readability/test.stderr b/tests/ui-toml/lint_decimal_readability/test.stderr
@@ -0,0 +1,10 @@
+error: digits grouped inconsistently by underscores
+  --> $DIR/test.rs:18:18
+   |
+LL |     let _fail1 = 100_200_300.123456789;
+   |                  ^^^^^^^^^^^^^^^^^^^^^ help: consider: `100_200_300.123_456_789`
+   |
+   = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings`
+
+error: aborting due to previous error
+
diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -1,4 +1,4 @@
-error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `third-party` at line 5 column 1
+error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `third-party` at line 5 column 1
 
 error: aborting due to previous error
 
diff --git a/tests/ui/unreadable_literal.fixed b/tests/ui/unreadable_literal.fixed
@@ -10,6 +10,14 @@ macro_rules! foo {
     };
 }
 
+struct Bar(f32);
+
+macro_rules! bar {
+    () => {
+        Bar(100200300400.100200300400500)
+    };
+}
+
 fn main() {
     let _good = (
         0b1011_i64,
@@ -26,10 +34,12 @@ fn main() {
     let _good_sci = 1.1234e1;
     let _bad_sci = 1.123_456e1;
 
-    let _fail9 = 0x00ab_cdef;
-    let _fail10: u32 = 0xBAFE_BAFE;
-    let _fail11 = 0x0abc_deff;
-    let _fail12: i128 = 0x00ab_cabc_abca_bcab_cabc;
+    let _fail1 = 0x00ab_cdef;
+    let _fail2: u32 = 0xBAFE_BAFE;
+    let _fail3 = 0x0abc_deff;
+    let _fail4: i128 = 0x00ab_cabc_abca_bcab_cabc;
+    let _fail5 = 1.100_300_400;
 
     let _ = foo!();
+    let _ = bar!();
 }
diff --git a/tests/ui/unreadable_literal.rs b/tests/ui/unreadable_literal.rs
@@ -10,6 +10,14 @@ macro_rules! foo {
     };
 }
 
+struct Bar(f32);
+
+macro_rules! bar {
+    () => {
+        Bar(100200300400.100200300400500)
+    };
+}
+
 fn main() {
     let _good = (
         0b1011_i64,
@@ -26,10 +34,12 @@ fn main() {
     let _good_sci = 1.1234e1;
     let _bad_sci = 1.123456e1;
 
-    let _fail9 = 0xabcdef;
-    let _fail10: u32 = 0xBAFEBAFE;
-    let _fail11 = 0xabcdeff;
-    let _fail12: i128 = 0xabcabcabcabcabcabc;
+    let _fail1 = 0xabcdef;
+    let _fail2: u32 = 0xBAFEBAFE;
+    let _fail3 = 0xabcdeff;
+    let _fail4: i128 = 0xabcabcabcabcabcabc;
+    let _fail5 = 1.100300400;
 
     let _ = foo!();
+    let _ = bar!();
 }
diff --git a/tests/ui/unreadable_literal.stderr b/tests/ui/unreadable_literal.stderr
@@ -1,66 +1,72 @@
 error: digits of hex or binary literal not grouped by four
-  --> $DIR/unreadable_literal.rs:17:9
+  --> $DIR/unreadable_literal.rs:25:9
    |
 LL |         0x1_234_567,
    |         ^^^^^^^^^^^ help: consider: `0x0123_4567`
    |
    = note: `-D clippy::unusual-byte-groupings` implied by `-D warnings`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:25:17
+  --> $DIR/unreadable_literal.rs:33:17
    |
 LL |     let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32);
    |                 ^^^^^^^^^^^^ help: consider: `0b11_0110_i64`
    |
    = note: `-D clippy::unreadable-literal` implied by `-D warnings`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:25:31
+  --> $DIR/unreadable_literal.rs:33:31
    |
 LL |     let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32);
    |                               ^^^^^^^^^^^^^^^^ help: consider: `0xcafe_babe_usize`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:25:49
+  --> $DIR/unreadable_literal.rs:33:49
    |
 LL |     let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32);
    |                                                 ^^^^^^^^^^ help: consider: `123_456_f32`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:25:61
+  --> $DIR/unreadable_literal.rs:33:61
    |
 LL |     let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32);
    |                                                             ^^^^^^^^^^^^ help: consider: `1.234_567_f32`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:27:20
+  --> $DIR/unreadable_literal.rs:35:20
    |
 LL |     let _bad_sci = 1.123456e1;
    |                    ^^^^^^^^^^ help: consider: `1.123_456e1`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:29:18
+  --> $DIR/unreadable_literal.rs:37:18
    |
-LL |     let _fail9 = 0xabcdef;
+LL |     let _fail1 = 0xabcdef;
    |                  ^^^^^^^^ help: consider: `0x00ab_cdef`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:30:24
+  --> $DIR/unreadable_literal.rs:38:23
    |
-LL |     let _fail10: u32 = 0xBAFEBAFE;
-   |                        ^^^^^^^^^^ help: consider: `0xBAFE_BAFE`
+LL |     let _fail2: u32 = 0xBAFEBAFE;
+   |                       ^^^^^^^^^^ help: consider: `0xBAFE_BAFE`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:31:19
+  --> $DIR/unreadable_literal.rs:39:18
    |
-LL |     let _fail11 = 0xabcdeff;
-   |                   ^^^^^^^^^ help: consider: `0x0abc_deff`
+LL |     let _fail3 = 0xabcdeff;
+   |                  ^^^^^^^^^ help: consider: `0x0abc_deff`
 
 error: long literal lacking separators
-  --> $DIR/unreadable_literal.rs:32:25
+  --> $DIR/unreadable_literal.rs:40:24
    |
-LL |     let _fail12: i128 = 0xabcabcabcabcabcabc;
-   |                         ^^^^^^^^^^^^^^^^^^^^ help: consider: `0x00ab_cabc_abca_bcab_cabc`
+LL |     let _fail4: i128 = 0xabcabcabcabcabcabc;
+   |                        ^^^^^^^^^^^^^^^^^^^^ help: consider: `0x00ab_cabc_abca_bcab_cabc`
 
-error: aborting due to 10 previous errors
+error: long literal lacking separators
+  --> $DIR/unreadable_literal.rs:41:18
+   |
+LL |     let _fail5 = 1.100300400;
+   |                  ^^^^^^^^^^^ help: consider: `1.100_300_400`
+
+error: aborting due to 11 previous errors