Auto merge of #6421 - xFrednet:4176-unreadable-literal-lint-fractal-option, 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
This commit is contained in:
bors 2020-12-06 07:18:33 +00:00
commit c1664c50b2
10 changed files with 118 additions and 37 deletions

View file

@ -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;

View file

@ -11,7 +11,7 @@
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 @@
/// ```
pub UNREADABLE_LITERAL,
pedantic,
"long integer literal without underscores"
"long literal without underscores"
}
declare_clippy_lint! {
@ -208,7 +208,13 @@ fn display(&self, suggested_format: String, cx: &EarlyContext<'_>, span: rustc_s
}
}
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 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
}
if let ExprKind::Lit(ref lit) = expr.kind {
Self::check_lit(cx, lit)
self.check_lit(cx, lit)
}
}
}
@ -232,7 +238,13 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
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 @@ fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
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 @@ fn parts_consistent(
/// 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 @@ fn get_group_size<'a>(groups: impl Iterator<Item = &'a str>, radix: Radix) -> Re
} else {
Ok(Some(second))
}
} else if first > 5 {
} else if first > 5 && lint_unreadable {
Err(WarningType::UnreadableLiteral)
} else {
Ok(None)

View file

@ -170,6 +170,8 @@ fn $config() -> $Ty {
(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 {

View file

@ -0,0 +1 @@
unreadable-literal-lint-fractions = false

View file

@ -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;
}

View file

@ -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

View file

@ -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

View file

@ -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!();
}

View file

@ -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!();
}

View file

@ -1,5 +1,5 @@
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`
@ -7,7 +7,7 @@ LL | 0x1_234_567,
= 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`
@ -15,52 +15,58 @@ LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32);
= 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