Make tidy check for magic numbers that spell things

Remove existing problematic cases.
rust-lang · Jan 1, 2022 · 0d55bd1 · 0d55bd1
1 parent 4d2e0fd
commit 0d55bd1
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 12 deletions.
diff --git a/library/std/src/net/ip.rs b/library/std/src/net/ip.rs
@@ -1140,8 +1140,8 @@ impl From<Ipv4Addr> 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<u32> 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
@@ -31,7 +31,7 @@ static mut frobulator: usize = 0xdeadbeef;
 
 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
@@ -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
@@ -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
@@ -9,27 +9,27 @@ 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`
 
 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
@@ -56,6 +56,12 @@ const ANNOTATIONS_TO_IGNORE: &[&str] = &[
     "// 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<String> = (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 @@ pub fn check(path: &Path, bad: &mut 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