From 0d55bd11009686dba662b0f4e697183691e7b308 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Fri, 31 Dec 2021 21:13:07 -0800 Subject: [PATCH] Make tidy check for magic numbers that spell things Remove existing problematic cases. --- library/std/src/net/ip.rs | 8 ++++---- src/test/ui/link-section.rs | 2 +- .../clippy/tests/ui/unreadable_literal.fixed | 2 +- src/tools/clippy/tests/ui/unreadable_literal.rs | 2 +- .../clippy/tests/ui/unreadable_literal.stderr | 10 +++++----- src/tools/tidy/src/style.rs | 15 +++++++++++++++ 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/library/std/src/net/ip.rs b/library/std/src/net/ip.rs index 4029125a375..70a7d7a8cab 100644 --- a/library/std/src/net/ip.rs +++ b/library/std/src/net/ip.rs @@ -1140,8 +1140,8 @@ impl From for u32 { /// ``` /// use std::net::Ipv4Addr; /// - /// let addr = Ipv4Addr::new(0xca, 0xfe, 0xba, 0xbe); - /// assert_eq!(0xcafebabe, u32::from(addr)); + /// let addr = Ipv4Addr::new(0x12, 0x34, 0x56, 0x78); + /// assert_eq!(0x12345678, u32::from(addr)); /// ``` #[inline] fn from(ip: Ipv4Addr) -> u32 { @@ -1159,8 +1159,8 @@ impl From for Ipv4Addr { /// ``` /// use std::net::Ipv4Addr; /// - /// let addr = Ipv4Addr::from(0xcafebabe); - /// assert_eq!(Ipv4Addr::new(0xca, 0xfe, 0xba, 0xbe), addr); + /// let addr = Ipv4Addr::from(0x12345678); + /// assert_eq!(Ipv4Addr::new(0x12, 0x34, 0x56, 0x78), addr); /// ``` #[inline] fn from(ip: u32) -> Ipv4Addr { diff --git a/src/test/ui/link-section.rs b/src/test/ui/link-section.rs index 6958eeda697..48efb07ff48 100644 --- a/src/test/ui/link-section.rs +++ b/src/test/ui/link-section.rs @@ -31,7 +31,7 @@ fn i_live_in_more_text() -> &'static str { pub fn main() { unsafe { - frobulator = 0xcafebabe; + frobulator = 0x12345678; println!("{} {} {}", i_live_in_more_text(), magic, frobulator); } } diff --git a/src/tools/clippy/tests/ui/unreadable_literal.fixed b/src/tools/clippy/tests/ui/unreadable_literal.fixed index c2e38037add..e726b652ef1 100644 --- a/src/tools/clippy/tests/ui/unreadable_literal.fixed +++ b/src/tools/clippy/tests/ui/unreadable_literal.fixed @@ -30,7 +30,7 @@ fn main() { 1_234.123_f32, 1.123_4_f32, ); - let _bad = (0b11_0110_i64, 0xcafe_babe_usize, 123_456_f32, 1.234_567_f32); + let _bad = (0b11_0110_i64, 0x1234_5678_usize, 123_456_f32, 1.234_567_f32); let _good_sci = 1.1234e1; let _bad_sci = 1.123_456e1; diff --git a/src/tools/clippy/tests/ui/unreadable_literal.rs b/src/tools/clippy/tests/ui/unreadable_literal.rs index 8296945b25e..5bbb2fc9dc1 100644 --- a/src/tools/clippy/tests/ui/unreadable_literal.rs +++ b/src/tools/clippy/tests/ui/unreadable_literal.rs @@ -30,7 +30,7 @@ fn main() { 1_234.123_f32, 1.123_4_f32, ); - let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); + let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); let _good_sci = 1.1234e1; let _bad_sci = 1.123456e1; diff --git a/src/tools/clippy/tests/ui/unreadable_literal.stderr b/src/tools/clippy/tests/ui/unreadable_literal.stderr index 8436aac17ac..ee5466fd517 100644 --- a/src/tools/clippy/tests/ui/unreadable_literal.stderr +++ b/src/tools/clippy/tests/ui/unreadable_literal.stderr @@ -9,7 +9,7 @@ LL | 0x1_234_567, error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:17 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `0b11_0110_i64` | = note: `-D clippy::unreadable-literal` implied by `-D warnings` @@ -17,19 +17,19 @@ LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:31 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); - | ^^^^^^^^^^^^^^^^ help: consider: `0xcafe_babe_usize` +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); + | ^^^^^^^^^^^^^^^^ help: consider: `0x1234_5678_usize` error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:49 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^ help: consider: `123_456_f32` error: long literal lacking separators --> $DIR/unreadable_literal.rs:33:61 | -LL | let _bad = (0b110110_i64, 0xcafebabe_usize, 123456_f32, 1.234567_f32); +LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `1.234_567_f32` error: long literal lacking separators diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 6ece9477140..ca79c835b9f 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -56,6 +56,12 @@ "// normalize-stderr-test", ]; +// Intentionally written in decimal rather than hex +const PROBLEMATIC_CONSTS: &[u32] = &[ + 184594741, 2880289470, 2881141438, 2965027518, 2976579765, 3203381950, 3405691582, 3405697037, + 3735927486, 4027431614, 4276992702, +]; + /// Parser states for `line_is_url`. #[derive(Clone, Copy, PartialEq)] #[allow(non_camel_case_types)] @@ -217,6 +223,10 @@ pub fn check(path: &Path, bad: &mut bool) { fn skip(path: &Path) -> bool { super::filter_dirs(path) || skip_markdown_path(path) } + let problematic_consts_strings: Vec = (PROBLEMATIC_CONSTS.iter().map(u32::to_string)) + .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:x}", v))) + .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v))) + .collect(); super::walk(path, &mut skip, &mut |entry, contents| { let file = entry.path(); let filename = file.file_name().unwrap().to_string_lossy(); @@ -306,6 +316,11 @@ fn skip(path: &Path) -> bool { if line.contains("//") && line.contains(" XXX") { err("XXX is deprecated; use FIXME") } + for s in problematic_consts_strings.iter() { + if line.contains(s) { + err("Don't use magic numbers that spell things (consider 0x12345678)"); + } + } } let is_test = || file.components().any(|c| c.as_os_str() == "tests"); // for now we just check libcore