From f70a3247254cdb50ebdc0be3c4f887f0dc9e8af2 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 6 Oct 2024 01:36:32 +0300 Subject: [PATCH] Uplift `clippy::zero_prefixed_literal` as `leading_zeros_in_decimal_literals` --- compiler/rustc_lint/messages.ftl | 4 + compiler/rustc_lint/src/builtin.rs | 75 ++++++++++++++++- compiler/rustc_lint/src/lib.rs | 1 + compiler/rustc_lint/src/lints.rs | 9 +++ .../clippy/clippy_lints/src/declared_lints.rs | 1 - .../clippy_lints/src/deprecated_lints.rs | 2 + .../clippy/clippy_lints/src/misc_early/mod.rs | 47 +---------- .../src/misc_early/zero_prefixed_literal.rs | 33 -------- src/tools/clippy/tests/ui/literals.rs | 1 - src/tools/clippy/tests/ui/literals.stderr | 80 +++++++++++-------- .../leading-zeros-in-decimal-literals.fixed | 12 +++ .../lint/leading-zeros-in-decimal-literals.rs | 12 +++ .../leading-zeros-in-decimal-literals.stderr | 47 +++++++++++ 13 files changed, 209 insertions(+), 115 deletions(-) delete mode 100644 src/tools/clippy/clippy_lints/src/misc_early/zero_prefixed_literal.rs create mode 100644 tests/ui/lint/leading-zeros-in-decimal-literals.fixed create mode 100644 tests/ui/lint/leading-zeros-in-decimal-literals.rs create mode 100644 tests/ui/lint/leading-zeros-in-decimal-literals.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 40ffed480381a..fcd3a98d04f07 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -464,6 +464,10 @@ lint_invalid_reference_casting_note_book = for more information, visit bool { + /// unix_mode & 0111 != 0 + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// In some languages (including the infamous C language and most of its family), + /// a leading zero marks an octal constant. In Rust however, a `0o` prefix is used instead. + /// Thus, a leading zero can be confusing for both the writer and a reader. + /// + /// In Rust: + /// ```rust,no_run + /// fn main() { + /// let a = 0123; + /// println!("{}", a); + /// } + /// ``` + /// + /// prints `123`, while in C: + /// + /// ```c + /// #include + /// + /// int main() { + /// int a = 0123; + /// printf("%d\n", a); + /// } + /// ``` + /// + /// prints `83` (as `83 == 0o123` while `123 == 0o173`). + pub LEADING_ZEROS_IN_DECIMAL_LITERALS, + Warn, + "leading `0` in decimal integer literals", +} + +declare_lint_pass!(LeadingZerosInDecimals => [LEADING_ZEROS_IN_DECIMAL_LITERALS]); + +impl EarlyLintPass for LeadingZerosInDecimals { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { + if let ExprKind::Lit(literal) = expr.kind + && let token::Lit { kind: token::LitKind::Integer, symbol, suffix: _ } = literal + && let s = symbol.as_str() + && let Some(tail) = s.strip_prefix('0') + && !tail.starts_with(['b', 'o', 'x']) + && let nonzero_digits = tail.trim_start_matches(['0', '_']) + && !nonzero_digits.is_empty() + { + let lit_span = expr.span; + let zeros_offset = s.len() - nonzero_digits.len(); + let leading_zeros_span = lit_span.with_hi(lit_span.lo() + BytePos(zeros_offset as u32)); + let can_be_octal = !nonzero_digits.contains(['8', '9']); + let (remove_zeros_applicability, prefix_octal) = if can_be_octal { + (Applicability::MaybeIncorrect, Some(lit_span.with_hi(lit_span.lo() + BytePos(1)))) + } else { + (Applicability::MachineApplicable, None) + }; + cx.emit_span_lint(LEADING_ZEROS_IN_DECIMAL_LITERALS, lit_span, LeadingZeros { + remove_zeros: (leading_zeros_span, remove_zeros_applicability), + prefix_octal, + }); + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a99c94592b302..022f8139a0383 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -179,6 +179,7 @@ early_lint_methods!( UnusedDocComment: UnusedDocComment, Expr2024: Expr2024, Precedence: Precedence, + LeadingZerosInDecimals: LeadingZerosInDecimals, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index e6eae7e736832..9e114372c7f09 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -3071,3 +3071,12 @@ pub(crate) struct ReservedMultihash { #[suggestion(code = " ", applicability = "machine-applicable")] pub suggestion: Span, } + +#[derive(LintDiagnostic)] +#[diag(lint_leading_zeros_in_decimal_literals)] +pub(crate) struct LeadingZeros { + #[suggestion(lint_suggestion_remove_zeros, code = "")] + pub remove_zeros: (Span, Applicability), + #[suggestion(lint_suggestion_prefix_octal, code = "0o", applicability = "maybe-incorrect")] + pub prefix_octal: Option, +} diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index c4a0c8f186510..d02f84de10cf7 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -511,7 +511,6 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::misc_early::UNNEEDED_FIELD_PATTERN_INFO, crate::misc_early::UNNEEDED_WILDCARD_PATTERN_INFO, crate::misc_early::UNSEPARATED_LITERAL_SUFFIX_INFO, - crate::misc_early::ZERO_PREFIXED_LITERAL_INFO, crate::mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER_INFO, crate::missing_assert_message::MISSING_ASSERT_MESSAGE_INFO, crate::missing_asserts_for_indexing::MISSING_ASSERTS_FOR_INDEXING_INFO, diff --git a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs index 77dbe9b78a182..65225a638ac27 100644 --- a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs +++ b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs @@ -177,5 +177,7 @@ declare_with_version! { RENAMED(RENAMED_VERSION): &[(&str, &str)] = &[ ("clippy::vtable_address_comparisons", "ambiguous_wide_pointer_comparisons"), #[clippy::version = ""] ("clippy::reverse_range_loop", "clippy::reversed_empty_ranges"), + #[clippy::version = "CURRENT_RUSTC_VERSION"] + ("clippy::zero_prefixed_literal", "leading_zeros_in_decimal_literals"), // end renamed lints. used by `cargo dev rename_lint` ]} diff --git a/src/tools/clippy/clippy_lints/src/misc_early/mod.rs b/src/tools/clippy/clippy_lints/src/misc_early/mod.rs index 37d7427f9a5d2..1f2a61b616071 100644 --- a/src/tools/clippy/clippy_lints/src/misc_early/mod.rs +++ b/src/tools/clippy/clippy_lints/src/misc_early/mod.rs @@ -6,7 +6,6 @@ mod redundant_at_rest_pattern; mod redundant_pattern; mod unneeded_field_pattern; mod unneeded_wildcard_pattern; -mod zero_prefixed_literal; use clippy_utils::diagnostics::span_lint; use clippy_utils::source::snippet_opt; @@ -188,45 +187,6 @@ declare_clippy_lint! { "literals whose suffix is separated by an underscore" } -declare_clippy_lint! { - /// ### What it does - /// Warns if an integral constant literal starts with `0`. - /// - /// ### Why is this bad? - /// In some languages (including the infamous C language - /// and most of its - /// family), this marks an octal constant. In Rust however, this is a decimal - /// constant. This could - /// be confusing for both the writer and a reader of the constant. - /// - /// ### Example - /// - /// In Rust: - /// ```no_run - /// fn main() { - /// let a = 0123; - /// println!("{}", a); - /// } - /// ``` - /// - /// prints `123`, while in C: - /// - /// ```c - /// #include - /// - /// int main() { - /// int a = 0123; - /// printf("%d\n", a); - /// } - /// ``` - /// - /// prints `83` (as `83 == 0o123` while `123 == 0o173`). - #[clippy::version = "pre 1.29.0"] - pub ZERO_PREFIXED_LITERAL, - complexity, - "integer literals starting with `0`" -} - declare_clippy_lint! { /// ### What it does /// Warns if a generic shadows a built-in type. @@ -356,7 +316,6 @@ declare_lint_pass!(MiscEarlyLints => [ MIXED_CASE_HEX_LITERALS, UNSEPARATED_LITERAL_SUFFIX, SEPARATED_LITERAL_SUFFIX, - ZERO_PREFIXED_LITERAL, BUILTIN_TYPE_SHADOW, REDUNDANT_PATTERN, UNNEEDED_WILDCARD_PATTERN, @@ -432,7 +391,7 @@ impl MiscEarlyLints { }; let lit_kind = LitKind::from_token_lit(lit); - if let Ok(LitKind::Int(value, lit_int_type)) = lit_kind { + if let Ok(LitKind::Int(_value, lit_int_type)) = lit_kind { let suffix = match lit_int_type { LitIntType::Signed(ty) => ty.name_str(), LitIntType::Unsigned(ty) => ty.name_str(), @@ -441,10 +400,6 @@ impl MiscEarlyLints { literal_suffix::check(cx, span, &lit_snip, suffix, "integer"); if lit_snip.starts_with("0x") { mixed_case_hex_literals::check(cx, span, suffix, &lit_snip); - } else if lit_snip.starts_with("0b") || lit_snip.starts_with("0o") { - // nothing to do - } else if value != 0 && lit_snip.starts_with('0') { - zero_prefixed_literal::check(cx, span, &lit_snip); } } else if let Ok(LitKind::Float(_, LitFloatType::Suffixed(float_ty))) = lit_kind { let suffix = float_ty.name_str(); diff --git a/src/tools/clippy/clippy_lints/src/misc_early/zero_prefixed_literal.rs b/src/tools/clippy/clippy_lints/src/misc_early/zero_prefixed_literal.rs deleted file mode 100644 index 61f4684c9e379..0000000000000 --- a/src/tools/clippy/clippy_lints/src/misc_early/zero_prefixed_literal.rs +++ /dev/null @@ -1,33 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use rustc_errors::Applicability; -use rustc_lint::EarlyContext; -use rustc_span::Span; - -use super::ZERO_PREFIXED_LITERAL; - -pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str) { - let trimmed_lit_snip = lit_snip.trim_start_matches(['_', '0']); - span_lint_and_then( - cx, - ZERO_PREFIXED_LITERAL, - lit_span, - "this is a decimal constant", - |diag| { - diag.span_suggestion( - lit_span, - "if you mean to use a decimal constant, remove the `0` to avoid confusion", - trimmed_lit_snip.to_string(), - Applicability::MaybeIncorrect, - ); - // do not advise to use octal form if the literal cannot be expressed in base 8. - if !lit_snip.contains(['8', '9']) { - diag.span_suggestion( - lit_span, - "if you mean to use an octal constant, use `0o`", - format!("0o{trimmed_lit_snip}"), - Applicability::MaybeIncorrect, - ); - } - }, - ); -} diff --git a/src/tools/clippy/tests/ui/literals.rs b/src/tools/clippy/tests/ui/literals.rs index c275b04d886bb..63653c33f2ddf 100644 --- a/src/tools/clippy/tests/ui/literals.rs +++ b/src/tools/clippy/tests/ui/literals.rs @@ -30,7 +30,6 @@ fn main() { //~^ ERROR: integer type suffix should be separated by an underscore //~| NOTE: `-D clippy::unseparated-literal-suffix` implied by `-D warnings` //~| ERROR: this is a decimal constant - //~| NOTE: `-D clippy::zero-prefixed-literal` implied by `-D warnings` let ok9 = 0; let ok10 = 0_i64; diff --git a/src/tools/clippy/tests/ui/literals.stderr b/src/tools/clippy/tests/ui/literals.stderr index 564e0bc4f7472..99c18334571ca 100644 --- a/src/tools/clippy/tests/ui/literals.stderr +++ b/src/tools/clippy/tests/ui/literals.stderr @@ -1,3 +1,12 @@ +error: lint `clippy::zero_prefixed_literal` has been renamed to `leading_zeros_in_decimal_literals` + --> tests/ui/literals.rs:5:9 + | +LL | #![warn(clippy::zero_prefixed_literal)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `leading_zeros_in_decimal_literals` + | + = note: `-D renamed-and-removed-lints` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(renamed_and_removed_lints)]` + error: integer type suffix should not be separated by an underscore --> tests/ui/literals.rs:13:15 | @@ -67,46 +76,48 @@ error: this is a decimal constant LL | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ | - = note: `-D clippy::zero-prefixed-literal` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::zero_prefixed_literal)]` -help: if you mean to use a decimal constant, remove the `0` to avoid confusion + = note: `-D leading-zeros-in-decimal-literals` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(leading_zeros_in_decimal_literals)]` +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let fail_multi_zero = 000_123usize; +LL + let fail_multi_zero = 123usize; | -LL | let fail_multi_zero = 123usize; - | ~~~~~~~~ -help: if you mean to use an octal constant, use `0o` +help: if you meant to use an octal constant, prefix it with `0o` instead | -LL | let fail_multi_zero = 0o123usize; - | ~~~~~~~~~~ +LL | let fail_multi_zero = 0o00_123usize; + | ~~ error: integer type suffix should not be separated by an underscore - --> tests/ui/literals.rs:36:16 + --> tests/ui/literals.rs:35:16 | LL | let ok10 = 0_i64; | ^^^^^ help: remove the underscore: `0i64` error: this is a decimal constant - --> tests/ui/literals.rs:38:17 + --> tests/ui/literals.rs:37:17 | LL | let fail8 = 0123; | ^^^^ | -help: if you mean to use a decimal constant, remove the `0` to avoid confusion +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion | -LL | let fail8 = 123; - | ~~~ -help: if you mean to use an octal constant, use `0o` +LL - let fail8 = 0123; +LL + let fail8 = 123; + | +help: if you meant to use an octal constant, prefix it with `0o` instead | LL | let fail8 = 0o123; - | ~~~~~ + | ~~ error: integer type suffix should not be separated by an underscore - --> tests/ui/literals.rs:48:16 + --> tests/ui/literals.rs:47:16 | LL | let ok17 = 0x123_4567_8901_usize; | ^^^^^^^^^^^^^^^^^^^^^ help: remove the underscore: `0x123_4567_8901usize` error: digits grouped inconsistently by underscores - --> tests/ui/literals.rs:52:18 + --> tests/ui/literals.rs:51:18 | LL | let fail19 = 12_3456_21; | ^^^^^^^^^^ help: consider: `12_345_621` @@ -115,19 +126,19 @@ LL | let fail19 = 12_3456_21; = help: to override `-D warnings` add `#[allow(clippy::inconsistent_digit_grouping)]` error: digits grouped inconsistently by underscores - --> tests/ui/literals.rs:55:18 + --> tests/ui/literals.rs:54:18 | LL | let fail22 = 3__4___23; | ^^^^^^^^^ help: consider: `3_423` error: digits grouped inconsistently by underscores - --> tests/ui/literals.rs:57:18 + --> tests/ui/literals.rs:56:18 | LL | let fail23 = 3__16___23; | ^^^^^^^^^^ help: consider: `31_623` error: digits of hex, binary or octal literal not in groups of equal size - --> tests/ui/literals.rs:60:18 + --> tests/ui/literals.rs:59:18 | LL | let fail24 = 0xAB_ABC_AB; | ^^^^^^^^^^^ help: consider: `0x0ABA_BCAB` @@ -136,37 +147,40 @@ LL | let fail24 = 0xAB_ABC_AB; = help: to override `-D warnings` add `#[allow(clippy::unusual_byte_groupings)]` error: this is a decimal constant - --> tests/ui/literals.rs:70:13 + --> tests/ui/literals.rs:69:13 | LL | let _ = 08; | ^^ | -help: if you mean to use a decimal constant, remove the `0` to avoid confusion +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 08; +LL + let _ = 8; | -LL | let _ = 8; - | ~ error: this is a decimal constant - --> tests/ui/literals.rs:72:13 + --> tests/ui/literals.rs:71:13 | LL | let _ = 09; | ^^ | -help: if you mean to use a decimal constant, remove the `0` to avoid confusion +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 09; +LL + let _ = 9; | -LL | let _ = 9; - | ~ error: this is a decimal constant - --> tests/ui/literals.rs:74:13 + --> tests/ui/literals.rs:73:13 | LL | let _ = 089; | ^^^ | -help: if you mean to use a decimal constant, remove the `0` to avoid confusion +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 089; +LL + let _ = 89; | -LL | let _ = 89; - | ~~ -error: aborting due to 20 previous errors +error: aborting due to 21 previous errors diff --git a/tests/ui/lint/leading-zeros-in-decimal-literals.fixed b/tests/ui/lint/leading-zeros-in-decimal-literals.fixed new file mode 100644 index 0000000000000..9239ca479c212 --- /dev/null +++ b/tests/ui/lint/leading-zeros-in-decimal-literals.fixed @@ -0,0 +1,12 @@ +//@ check-pass +//@ run-rustfix +//@ rustfix-only-machine-applicable + +fn main() { + let _ = 0111; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 0007; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 8; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 0_0_; + let _ = 00; + let _ = 0; +} diff --git a/tests/ui/lint/leading-zeros-in-decimal-literals.rs b/tests/ui/lint/leading-zeros-in-decimal-literals.rs new file mode 100644 index 0000000000000..2b3cfd372268f --- /dev/null +++ b/tests/ui/lint/leading-zeros-in-decimal-literals.rs @@ -0,0 +1,12 @@ +//@ check-pass +//@ run-rustfix +//@ rustfix-only-machine-applicable + +fn main() { + let _ = 0111; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 0007; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 0008; //~ WARNING [leading_zeros_in_decimal_literals] + let _ = 0_0_; + let _ = 00; + let _ = 0; +} diff --git a/tests/ui/lint/leading-zeros-in-decimal-literals.stderr b/tests/ui/lint/leading-zeros-in-decimal-literals.stderr new file mode 100644 index 0000000000000..92f0628f2574a --- /dev/null +++ b/tests/ui/lint/leading-zeros-in-decimal-literals.stderr @@ -0,0 +1,47 @@ +warning: this is a decimal constant + --> $DIR/leading-zeros-in-decimal-literals.rs:6:13 + | +LL | let _ = 0111; + | ^^^^ + | + = note: `#[warn(leading_zeros_in_decimal_literals)]` on by default +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 0111; +LL + let _ = 111; + | +help: if you meant to use an octal constant, prefix it with `0o` instead + | +LL | let _ = 0o111; + | ~~ + +warning: this is a decimal constant + --> $DIR/leading-zeros-in-decimal-literals.rs:7:13 + | +LL | let _ = 0007; + | ^^^^ + | +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 0007; +LL + let _ = 7; + | +help: if you meant to use an octal constant, prefix it with `0o` instead + | +LL | let _ = 0o007; + | ~~ + +warning: this is a decimal constant + --> $DIR/leading-zeros-in-decimal-literals.rs:8:13 + | +LL | let _ = 0008; + | ^^^^ + | +help: if you meant to use a decimal constant, remove leading zeros to avoid confusion + | +LL - let _ = 0008; +LL + let _ = 8; + | + +warning: 3 warnings emitted +