Skip to content

Rewrite and merge LiteralDigitGrouping and LiteralRepresentation passes #17412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Jarcho wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Rewrite and merge LiteralDigitGrouping and LiteralRepresentation passes #17412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions clippy_lints/src/declared_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,13 +263,13 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::lifetimes::ELIDABLE_LIFETIME_NAMES_INFO,
crate::lifetimes::EXTRA_UNUSED_LIFETIMES_INFO,
crate::lifetimes::NEEDLESS_LIFETIMES_INFO,
crate::literal_representation::DECIMAL_LITERAL_REPRESENTATION_INFO,
crate::literal_representation::INCONSISTENT_DIGIT_GROUPING_INFO,
crate::literal_representation::LARGE_DIGIT_GROUPS_INFO,
crate::literal_representation::MISTYPED_LITERAL_SUFFIXES_INFO,
crate::literal_representation::UNREADABLE_LITERAL_INFO,
crate::literal_representation::UNUSUAL_BYTE_GROUPINGS_INFO,
crate::literal_string_with_formatting_args::LITERAL_STRING_WITH_FORMATTING_ARGS_INFO,
crate::literals::DECIMAL_LITERAL_REPRESENTATION_INFO,
crate::literals::INCONSISTENT_DIGIT_GROUPING_INFO,
crate::literals::LARGE_DIGIT_GROUPS_INFO,
crate::literals::MISTYPED_LITERAL_SUFFIXES_INFO,
crate::literals::UNREADABLE_LITERAL_INFO,
crate::literals::UNUSUAL_BYTE_GROUPINGS_INFO,
crate::loops::CHAR_INDICES_AS_BYTE_INDICES_INFO,
crate::loops::EMPTY_LOOP_INFO,
crate::loops::EXPLICIT_COUNTER_LOOP_INFO,
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ mod let_if_seq;
mod let_underscore;
mod let_with_type_underscore;
mod lifetimes;
mod literal_representation;
mod literal_string_with_formatting_args;
mod literals;
mod loops;
mod macro_metavars_in_unsafe;
mod macro_use;
Expand Down Expand Up @@ -510,8 +510,7 @@ rustc_lint::early_lint_methods!(
UnusedUnit: unused_unit::UnusedUnit = unused_unit::UnusedUnit,
Precedence: precedence::Precedence = precedence::Precedence,
NeedlessArbitrarySelfType: needless_arbitrary_self_type::NeedlessArbitrarySelfType = needless_arbitrary_self_type::NeedlessArbitrarySelfType,
LiteralDigitGrouping: literal_representation::LiteralDigitGrouping = literal_representation::LiteralDigitGrouping::new(conf),
DecimalLiteralRepresentation: literal_representation::DecimalLiteralRepresentation = literal_representation::DecimalLiteralRepresentation::new(conf),
EarlyLiterals: literals::EarlyLiterals = literals::EarlyLiterals::new(conf),
TabsInDocComments: tabs_in_doc_comments::TabsInDocComments = tabs_in_doc_comments::TabsInDocComments,
SingleComponentPathImports: single_component_path_imports::SingleComponentPathImports = single_component_path_imports::SingleComponentPathImports::default(),
OptionEnvUnwrap: option_env_unwrap::OptionEnvUnwrap = option_env_unwrap::OptionEnvUnwrap,
Expand Down
137 changes: 1 addition & 136 deletions clippy_lints/src/literal_representation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,141 +10,6 @@ use rustc_session::impl_lint_pass;
use rustc_span::Span;
use std::iter;

declare_clippy_lint! {
/// ### What it does
/// Warns if there is a better representation for a numeric literal.
///
/// ### Why restrict this?
/// Especially for big powers of 2, a hexadecimal representation is usually more
/// readable than a decimal representation.
///
/// ### Example
/// ```text
/// `255` => `0xFF`
/// `65_535` => `0xFFFF`
/// `4_042_322_160` => `0xF0F0_F0F0`
/// ```
#[clippy::version = "pre 1.29.0"]
pub DECIMAL_LITERAL_REPRESENTATION,
restriction,
"using decimal representation when hexadecimal would be better"
}

declare_clippy_lint! {
/// ### What it does
/// Warns if an integral or floating-point constant is
/// grouped inconsistently with underscores.
///
/// ### Why is this bad?
/// Readers may incorrectly interpret inconsistently
/// grouped digits.
///
/// ### Example
/// ```no_run
/// # let _: u64 =
/// 618_64_9189_73_511
/// # ;
/// ```
///
/// Use instead:
/// ```no_run
/// # let _: u64 =
/// 61_864_918_973_511
/// # ;
/// ```
#[clippy::version = "pre 1.29.0"]
pub INCONSISTENT_DIGIT_GROUPING,
style,
"integer literals with digits grouped inconsistently"
}

declare_clippy_lint! {
/// ### What it does
/// Warns if the digits of an integral or floating-point
/// constant are grouped into groups that
/// are too large.
///
/// ### Why is this bad?
/// Negatively impacts readability.
///
/// ### Example
/// ```no_run
/// let x: u64 = 6186491_8973511;
/// ```
#[clippy::version = "pre 1.29.0"]
pub LARGE_DIGIT_GROUPS,
pedantic,
"grouping digits into groups that are too large"
}

declare_clippy_lint! {
/// ### What it does
/// Warns for mistyped suffix in literals
///
/// ### Why is this bad?
/// This is most probably a typo
///
/// ### Known problems
/// - Does not match on integers too large to fit in the corresponding unsigned type
/// - Does not match on `_127` since that is a valid grouping for decimal and octal numbers
///
/// ### Example
/// ```ignore
/// `2_32` => `2_i32`
/// `250_8 => `250_u8`
/// ```
#[clippy::version = "1.30.0"]
pub MISTYPED_LITERAL_SUFFIXES,
correctness,
"mistyped literal suffix"
}

declare_clippy_lint! {
/// ### What it does
/// Warns if a long integral or floating-point constant does
/// not contain underscores.
///
/// ### Why is this bad?
/// Reading long numbers is difficult without separators.
///
/// ### Example
/// ```no_run
/// # let _: u64 =
/// 61864918973511
/// # ;
/// ```
///
/// Use instead:
/// ```no_run
/// # let _: u64 =
/// 61_864_918_973_511
/// # ;
/// ```
#[clippy::version = "pre 1.29.0"]
pub UNREADABLE_LITERAL,
pedantic,
"long literal without underscores"
}

declare_clippy_lint! {
/// ### What it does
/// Warns if hexadecimal or binary literals are not grouped
/// by nibble or byte.
///
/// ### Why is this bad?
/// Negatively impacts readability.
///
/// ### Example
/// ```no_run
/// let x: u32 = 0xFFF_FFF;
/// let y: u8 = 0b01_011_101;
/// ```
#[clippy::version = "1.49.0"]
pub UNUSUAL_BYTE_GROUPINGS,
style,
"binary or hex literals that aren't grouped by four"
}

impl_lint_pass!(DecimalLiteralRepresentation => [DECIMAL_LITERAL_REPRESENTATION]);

impl_lint_pass!(LiteralDigitGrouping => [
Expand Down Expand Up @@ -182,7 +47,7 @@ impl WarningType {
Self::DecimalRepresentation => (
DECIMAL_LITERAL_REPRESENTATION,
"integer literal has a better hexadecimal representation",
"consider",
"use a hex literal",
),
Self::UnusualByteGroupings => (
UNUSUAL_BYTE_GROUPINGS,
Expand Down
74 changes: 74 additions & 0 deletions clippy_lints/src/literals/decimal_literal_representation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use arrayvec::ArrayVec;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::numeric_literal::{IntStr, Radix};
use clippy_utils::source::SpanExt;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, LintContext};
use rustc_span::Span;

use super::DECIMAL_LITERAL_REPRESENTATION;

pub(super) fn check(cx: &EarlyContext<'_>, threshold: u64, num: IntStr<'_>, sp: Span) {
if let Radix::Dec = num.radix
&& num.suffix.is_none_or(|s| !s.is_float())
&& let Some(val @ 1..) = num.parse_as_u128()
&& val >= u128::from(threshold)
&& check_val(val)
&& let sp_data = sp.data()
&& !sp_data.ctxt.in_external_macro(cx.sess().source_map())
&& sp_data.check_text(cx, |src| num.eq_str(src))
{
span_lint_and_then(
cx,
DECIMAL_LITERAL_REPRESENTATION,
sp,
"integer literal has a better hexadecimal representation",
|diag| {
diag.span_suggestion_verbose(
num.trim_sp_to_digits(&sp_data),
"use a hex literal",
fmt(val),
Applicability::MachineApplicable,
);
},
);
}
}

fn check_val(mut val: u128) -> bool {
// Power-of-two or power-of-two minus one.
// Ignore the lowest digit for larger numbers.
let pval = if val > 0xfff { val >> 4 } else { val };
if pval.count_ones() == 1 || pval.wrapping_add(1).count_ones() <= 1 {
return true;
}
// All hex digits are either `0` or `f`.
for _ in 0..15 {
if !matches!(val & 0xff, 0 | 0xf | 0xf0 | 0xff) {
break;
}
val >>= 8;
}
matches!(val, 0 | 0x7 | 0xf | 0x70 | 0x7f | 0xf0 | 0xff)
}

fn fmt(mut num: u128) -> String {
let mut buf = ArrayVec::<u8, { 128 / 16 + 128 / 16 / 4 + 2 }>::new();
let mut i = 4u8;
while num != 0 {
if i == 0 {
let _ = buf.try_push(b'_');
i = 3;
} else {
i -= 1;
}
let _ = buf.try_push(match num % 16 {
c @ 0..10 => c as u8 + b'0',
c => c as u8 - 10 + b'a',
});
num /= 16;
}
let _ = buf.try_extend_from_slice(b"x0");
buf.reverse();
String::from_utf8(buf.to_vec()).unwrap_or(String::new())
}
Loading
Loading