diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 13760e8c0fb..f893a3d45e3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -116,7 +116,7 @@ pub mod large_enum_variant; pub mod len_zero; pub mod let_if_seq; pub mod lifetimes; -pub mod literal_digit_grouping; +pub mod literal_representation; pub mod loops; pub mod map_clone; pub mod matches; @@ -353,7 +353,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold)); reg.register_late_lint_pass(box explicit_write::Pass); reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue); - reg.register_early_lint_pass(box literal_digit_grouping::LiteralDigitGrouping); + reg.register_early_lint_pass(box literal_representation::LiteralDigitGrouping); + reg.register_early_lint_pass(box literal_representation::LiteralRepresentation); reg.register_late_lint_pass(box use_self::UseSelf); reg.register_late_lint_pass(box bytecount::ByteCount); reg.register_late_lint_pass(box infinite_iter::Pass); @@ -482,9 +483,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { let_if_seq::USELESS_LET_IF_SEQ, lifetimes::NEEDLESS_LIFETIMES, lifetimes::UNUSED_LIFETIMES, - literal_digit_grouping::INCONSISTENT_DIGIT_GROUPING, - literal_digit_grouping::LARGE_DIGIT_GROUPS, - literal_digit_grouping::UNREADABLE_LITERAL, + literal_representation::INCONSISTENT_DIGIT_GROUPING, + literal_representation::LARGE_DIGIT_GROUPS, + literal_representation::UNREADABLE_LITERAL, + literal_representation::BAD_LITERAL_REPRESENTATION, loops::EMPTY_LOOP, loops::EXPLICIT_COUNTER_LOOP, loops::EXPLICIT_INTO_ITER_LOOP, diff --git a/clippy_lints/src/literal_digit_grouping.rs b/clippy_lints/src/literal_representation.rs similarity index 73% rename from clippy_lints/src/literal_digit_grouping.rs rename to clippy_lints/src/literal_representation.rs index 011b5ec1d5e..91f01bae257 100644 --- a/clippy_lints/src/literal_digit_grouping.rs +++ b/clippy_lints/src/literal_representation.rs @@ -62,7 +62,25 @@ declare_lint! { "grouping digits into groups that are too large" } -#[derive(Debug)] +/// **What it does:** Warns if there is a better representation for a numeric literal. +/// +/// **Why is this bad?** Especially for big powers of 2 a hexadecimal representation is more +/// readable than a decimal representation. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// +/// `255` => `0xFF` +/// `65_535` => `0xFFFF` +/// `4_042_322_160` => `0xF0F0_F0F0` +declare_lint! { + pub BAD_LITERAL_REPRESENTATION, + Warn, + "using decimal representation when hexadecimal would be better" +} + +#[derive(Debug, PartialEq)] enum Radix { Binary, Octal, @@ -168,7 +186,12 @@ impl<'a> DigitInfo<'a> { .map(|chunk| chunk.into_iter().collect()) .collect::>() .join("_"); - format!("{}.{}{}", int_part_hint, frac_part_hint, self.suffix.unwrap_or("")) + format!( + "{}.{}{}", + int_part_hint, + frac_part_hint, + self.suffix.unwrap_or("") + ) } else { let hint = self.digits .chars() @@ -180,7 +203,12 @@ impl<'a> DigitInfo<'a> { .rev() .collect::>() .join("_"); - format!("{}{}{}", self.prefix.unwrap_or(""), hint, self.suffix.unwrap_or("")) + format!( + "{}{}{}", + self.prefix.unwrap_or(""), + hint, + self.suffix.unwrap_or("") + ) } } } @@ -189,9 +217,9 @@ enum WarningType { UnreadableLiteral, InconsistentDigitGrouping, LargeDigitGroups, + BadRepresentation, } - impl WarningType { pub fn display(&self, grouping_hint: &str, cx: &EarlyContext, span: &syntax_pos::Span) { match *self { @@ -216,6 +244,13 @@ impl WarningType { "digits grouped inconsistently by underscores", &format!("consider: {}", grouping_hint), ), + WarningType::BadRepresentation => span_help_and_lint( + cx, + BAD_LITERAL_REPRESENTATION, + *span, + "bad representation of integer literal", + &format!("consider: {}", grouping_hint), + ), }; } } @@ -225,7 +260,11 @@ pub struct LiteralDigitGrouping; impl LintPass for LiteralDigitGrouping { fn get_lints(&self) -> LintArray { - lint_array!(UNREADABLE_LITERAL, INCONSISTENT_DIGIT_GROUPING, LARGE_DIGIT_GROUPS) + lint_array!( + UNREADABLE_LITERAL, + INCONSISTENT_DIGIT_GROUPING, + LARGE_DIGIT_GROUPS + ) } } @@ -353,3 +392,80 @@ impl LiteralDigitGrouping { } } } + +#[derive(Copy, Clone)] +pub struct LiteralRepresentation; + +impl LintPass for LiteralRepresentation { + fn get_lints(&self) -> LintArray { + lint_array!(BAD_LITERAL_REPRESENTATION) + } +} + +impl EarlyLintPass for LiteralRepresentation { + fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { + if in_external_macro(cx, expr.span) { + return; + } + + if let ExprKind::Lit(ref lit) = expr.node { + self.check_lit(cx, lit) + } + } +} + +impl LiteralRepresentation { + fn check_lit(&self, cx: &EarlyContext, lit: &Lit) { + // Lint integral literals. + if_chain! { + if let LitKind::Int(..) = 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 digit_info = DigitInfo::new(&src, false); + if digit_info.radix == Radix::Decimal { + let hex = format!("{:#X}", digit_info.digits + .chars() + .filter(|&c| c != '_') + .collect::() + .parse::().unwrap()); + let digit_info = DigitInfo::new(&hex[..], false); + let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| { + warning_type.display(&digit_info.grouping_hint(), cx, &lit.span) + }); + } + } + } + } + + fn do_lint(digits: &str) -> Result<(), WarningType> { + if digits.len() == 2 && digits == "FF" { + return Err(WarningType::BadRepresentation); + } else if digits.len() == 3 { + // Lint for Literals with a hex-representation of 3 digits + let f = &digits[0..1]; // first digit + let s = &digits[1..]; // suffix + // Powers of 2 minus 1 + if (f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && s.eq("FF") { + return Err(WarningType::BadRepresentation); + } + } else if digits.len() > 3 { + // Lint for Literals with a hex-representation of 4 digits or more + let f = &digits[0..1]; // first digit + let m = &digits[1..digits.len() - 1]; // middle digits, except last + let s = &digits[1..]; // suffix + // Powers of 2 with a margin of +15/-16 + if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && m.chars().all(|c| c == '0')) + || ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && m.chars().all(|c| c == 'F')) + // Lint for representations with only 0s and Fs, while allowing 7 as the first + // digit + || ((f.eq("7") || f.eq("F")) && s.chars().all(|c| c == '0' || c == 'F')) + { + return Err(WarningType::BadRepresentation); + } + } + + Ok(()) + } +} diff --git a/tests/ui/bad_literal_representation.rs b/tests/ui/bad_literal_representation.rs new file mode 100644 index 00000000000..00126152fe1 --- /dev/null +++ b/tests/ui/bad_literal_representation.rs @@ -0,0 +1,23 @@ + + + +#[warn(bad_literal_representation)] +#[allow(unused_variables)] +fn main() { + // Hex: 7F, 80, 100, 800, FFA, F0F3, 7F0F_F00D + let good = (127, 128, 256, 2048, 4090, 61_683, 2_131_750_925); + let bad = ( // Hex: + 255, // 0xFF + 511, // 0x1FF + 1023, // 0x3FF + 2047, // 0x7FF + 4095, // 0xFFF + 4096, // 0x1000 + 16_371, // 0x3FF3 + 32_773, // 0x8005 + 65_280, // 0xFF00 + 2_131_750_927, // 0x7F0F_F00F + 2_147_483_647, // 0x7FFF_FFFF + 4_042_322_160, // 0xF0F0_F0F0 + ); +} diff --git a/tests/ui/bad_literal_representation.stderr b/tests/ui/bad_literal_representation.stderr new file mode 100644 index 00000000000..f57956c23d6 --- /dev/null +++ b/tests/ui/bad_literal_representation.stderr @@ -0,0 +1,97 @@ +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:10:9 + | +10 | 255, // 0xFF + | ^^^ + | + = note: `-D bad-literal-representation` implied by `-D warnings` + = help: consider: 0xFF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:11:9 + | +11 | 511, // 0x1FF + | ^^^ + | + = help: consider: 0x1FF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:12:9 + | +12 | 1023, // 0x3FF + | ^^^^ + | + = help: consider: 0x3FF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:13:9 + | +13 | 2047, // 0x7FF + | ^^^^ + | + = help: consider: 0x7FF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:14:9 + | +14 | 4095, // 0xFFF + | ^^^^ + | + = help: consider: 0xFFF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:15:9 + | +15 | 4096, // 0x1000 + | ^^^^ + | + = help: consider: 0x1000 + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:16:9 + | +16 | 16_371, // 0x3FF3 + | ^^^^^^ + | + = help: consider: 0x3FF3 + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:17:9 + | +17 | 32_773, // 0x8005 + | ^^^^^^ + | + = help: consider: 0x8005 + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:18:9 + | +18 | 65_280, // 0xFF00 + | ^^^^^^ + | + = help: consider: 0xFF00 + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:19:9 + | +19 | 2_131_750_927, // 0x7F0F_F00F + | ^^^^^^^^^^^^^ + | + = help: consider: 0x7F0F_F00F + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:20:9 + | +20 | 2_147_483_647, // 0x7FFF_FFFF + | ^^^^^^^^^^^^^ + | + = help: consider: 0x7FFF_FFFF + +error: bad representation of integer literal + --> $DIR/bad_literal_representation.rs:21:9 + | +21 | 4_042_322_160, // 0xF0F0_F0F0 + | ^^^^^^^^^^^^^ + | + = help: consider: 0xF0F0_F0F0 + diff --git a/tests/ui/drop_forget_copy.rs b/tests/ui/drop_forget_copy.rs index 9fef06b0ede..a4d38d99c95 100644 --- a/tests/ui/drop_forget_copy.rs +++ b/tests/ui/drop_forget_copy.rs @@ -42,7 +42,7 @@ fn main() { forget(s4); forget(s5); - let a1 = AnotherStruct {x: 255, y: 0, z: vec![1, 2, 3]}; + let a1 = AnotherStruct {x: 0xFF, y: 0, z: vec![1, 2, 3]}; let a2 = &a1; let mut a3 = a1.clone(); let ref a4 = a1; diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index 1ed9f974d43..d4bc7df4424 100644 --- a/tests/ui/identity_op.rs +++ b/tests/ui/identity_op.rs @@ -29,5 +29,5 @@ fn main() { -1 & x; let u : u8 = 0; - u & 255; + u & 0xFF; } diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index c1ce8d2ec4c..b3f7bb713e6 100644 --- a/tests/ui/identity_op.stderr +++ b/tests/ui/identity_op.stderr @@ -45,6 +45,6 @@ error: the operation is ineffective. Consider reducing it to `x` error: the operation is ineffective. Consider reducing it to `u` --> $DIR/identity_op.rs:32:5 | -32 | u & 255; - | ^^^^^^^ +32 | u & 0xFF; + | ^^^^^^^^