src/tools/tidy/src/style.rs

   1 //! Tidy check to enforce various stylistic guidelines on the Rust codebase.
   2 //!
   3 //! Example checks are:
   4 //!
   5 //! * No lines over 100 characters (in non-Rust files).
   6 //! * No files with over 3000 lines (in non-Rust files).
   7 //! * No tabs.
   8 //! * No trailing whitespace.
   9 //! * No CR characters.
  10 //! * No `TODO` or `XXX` directives.
  11 //! * No unexplained ` ```ignore ` or ` ```rust,ignore ` doc tests.
  12 //!
  13 //! Note that some of these rules are excluded from Rust files because we enforce rustfmt. It is
  14 //! preferable to be formatted rather than tidy-clean.
  15 //!
  16 //! A number of these checks can be opted-out of with various directives of the form:
  17 //! `// ignore-tidy-CHECK-NAME`.
  18 // ignore-tidy-dbg
  19
  20 use crate::walk::{filter_dirs, walk};
  21 use regex::{Regex, RegexSet};
  22 use std::path::Path;
  23
  24 /// Error code markdown is restricted to 80 columns because they can be
  25 /// displayed on the console with --example.
  26 const ERROR_CODE_COLS: usize = 80;
  27 const COLS: usize = 100;
  28 const GOML_COLS: usize = 120;
  29
  30 const LINES: usize = 3000;
  31
  32 const UNEXPLAINED_IGNORE_DOCTEST_INFO: &str = r#"unexplained "```ignore" doctest; try one:
  33
  34 * make the test actually pass, by adding necessary imports and declarations, or
  35 * use "```text", if the code is not Rust code, or
  36 * use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
  37 * use "```should_panic", if the code is expected to fail at run time, or
  38 * use "```no_run", if the code should type-check but not necessary linkable/runnable, or
  39 * explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
  40
  41 "#;
  42
  43 const LLVM_UNREACHABLE_INFO: &str = r"\
  44 C++ code used llvm_unreachable, which triggers undefined behavior
  45 when executed when assertions are disabled.
  46 Use llvm::report_fatal_error for increased robustness.";
  47
  48 const DOUBLE_SPACE_AFTER_DOT: &str = r"\
  49 Use a single space after dots in comments.";
  50
  51 const ANNOTATIONS_TO_IGNORE: &[&str] = &[
  52     "// @!has",
  53     "// @has",
  54     "// @matches",
  55     "// CHECK",
  56     "// EMIT_MIR",
  57     "// compile-flags",
  58     "// error-pattern",
  59     "// gdb",
  60     "// lldb",
  61     "// cdb",
  62     "// normalize-stderr-test",
  63 ];
  64
  65 // Intentionally written in decimal rather than hex
  66 const PROBLEMATIC_CONSTS: &[u32] = &[
  67     184594741, 2880289470, 2881141438, 2965027518, 2976579765, 3203381950, 3405691582, 3405697037,
  68     3735927486, 3735932941, 4027431614, 4276992702,
  69 ];
  70
  71 const INTERNAL_COMPILER_DOCS_LINE: &str = "#### This error code is internal to the compiler and will not be emitted with normal Rust code.";
  72
  73 /// Parser states for `line_is_url`.
  74 #[derive(Clone, Copy, PartialEq)]
  75 #[allow(non_camel_case_types)]
  76 enum LIUState {
  77     EXP_COMMENT_START,
  78     EXP_LINK_LABEL_OR_URL,
  79     EXP_URL,
  80     EXP_END,
  81 }
  82
  83 /// Returns `true` if `line` appears to be a line comment containing a URL,
  84 /// possibly with a Markdown link label in front, and nothing else.
  85 /// The Markdown link label, if present, may not contain whitespace.
  86 /// Lines of this form are allowed to be overlength, because Markdown
  87 /// offers no way to split a line in the middle of a URL, and the lengths
  88 /// of URLs to external references are beyond our control.
  89 fn line_is_url(is_error_code: bool, columns: usize, line: &str) -> bool {
  90     // more basic check for markdown, to avoid complexity in implementing two state machines
  91     if is_error_code {
  92         return line.starts_with('[') && line.contains("]:") && line.contains("http");
  93     }
  94
  95     use self::LIUState::*;
  96     let mut state: LIUState = EXP_COMMENT_START;
  97     let is_url = |w: &str| w.starts_with("http://") || w.starts_with("https://");
  98
  99     for tok in line.split_whitespace() {
 100         match (state, tok) {
 101             (EXP_COMMENT_START, "//") | (EXP_COMMENT_START, "///") | (EXP_COMMENT_START, "//!") => {
 102                 state = EXP_LINK_LABEL_OR_URL
 103             }
 104
 105             (EXP_LINK_LABEL_OR_URL, w)
 106                 if w.len() >= 4 && w.starts_with('[') && w.ends_with("]:") =>
 107             {
 108                 state = EXP_URL
 109             }
 110
 111             (EXP_LINK_LABEL_OR_URL, w) if is_url(w) => state = EXP_END,
 112
 113             (EXP_URL, w) if is_url(w) || w.starts_with("../") => state = EXP_END,
 114
 115             (_, w) if w.len() > columns && is_url(w) => state = EXP_END,
 116
 117             (_, _) => {}
 118         }
 119     }
 120
 121     state == EXP_END
 122 }
 123
 124 /// Returns `true` if `line` can be ignored. This is the case when it contains
 125 /// an annotation that is explicitly ignored.
 126 fn should_ignore(line: &str) -> bool {
 127     // Matches test annotations like `//~ ERROR text`.
 128     // This mirrors the regex in src/tools/compiletest/src/runtest.rs, please
 129     // update both if either are changed.
 130     let re = Regex::new("\\s*//(\\[.*\\])?~.*").unwrap();
 131     re.is_match(line) || ANNOTATIONS_TO_IGNORE.iter().any(|a| line.contains(a))
 132 }
 133
 134 /// Returns `true` if `line` is allowed to be longer than the normal limit.
 135 fn long_line_is_ok(extension: &str, is_error_code: bool, max_columns: usize, line: &str) -> bool {
 136     match extension {
 137         // fluent files are allowed to be any length
 138         "ftl" => true,
 139         // non-error code markdown is allowed to be any length
 140         "md" if !is_error_code => true,
 141         // HACK(Ezrashaw): there is no way to split a markdown header over multiple lines
 142         "md" if line == INTERNAL_COMPILER_DOCS_LINE => true,
 143         _ => line_is_url(is_error_code, max_columns, line) || should_ignore(line),
 144     }
 145 }
 146
 147 enum Directive {
 148     /// By default, tidy always warns against style issues.
 149     Deny,
 150
 151     /// `Ignore(false)` means that an `ignore-tidy-*` directive
 152     /// has been provided, but is unnecessary. `Ignore(true)`
 153     /// means that it is necessary (i.e. a warning would be
 154     /// produced if `ignore-tidy-*` was not present).
 155     Ignore(bool),
 156 }
 157
 158 fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) -> Directive {
 159     if !can_contain {
 160         return Directive::Deny;
 161     }
 162     // Update `can_contain` when changing this
 163     if contents.contains(&format!("// ignore-tidy-{check}"))
 164         || contents.contains(&format!("# ignore-tidy-{check}"))
 165         || contents.contains(&format!("/* ignore-tidy-{check} */"))
 166     {
 167         Directive::Ignore(false)
 168     } else {
 169         Directive::Deny
 170     }
 171 }
 172
 173 macro_rules! suppressible_tidy_err {
 174     ($err:ident, $skip:ident, $msg:expr) => {
 175         if let Directive::Deny = $skip {
 176             $err($msg);
 177         } else {
 178             $skip = Directive::Ignore(true);
 179         }
 180     };
 181 }
 182
 183 pub fn is_in(full_path: &Path, parent_folder_to_find: &str, folder_to_find: &str) -> bool {
 184     if let Some(parent) = full_path.parent() {
 185         if parent.file_name().map_or_else(
 186             || false,
 187             |f| {
 188                 f.to_string_lossy() == folder_to_find
 189                     && parent
 190                         .parent()
 191                         .and_then(|f| f.file_name())
 192                         .map_or_else(|| false, |f| f == parent_folder_to_find)
 193             },
 194         ) {
 195             true
 196         } else {
 197             is_in(parent, parent_folder_to_find, folder_to_find)
 198         }
 199     } else {
 200         false
 201     }
 202 }
 203
 204 fn skip_markdown_path(path: &Path) -> bool {
 205     // These aren't ready for tidy.
 206     const SKIP_MD: &[&str] = &[
 207         "src/doc/edition-guide",
 208         "src/doc/embedded-book",
 209         "src/doc/nomicon",
 210         "src/doc/reference",
 211         "src/doc/rust-by-example",
 212         "src/doc/rustc-dev-guide",
 213     ];
 214     SKIP_MD.iter().any(|p| path.ends_with(p))
 215 }
 216
 217 fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
 218     if !line.ends_with("```ignore") && !line.ends_with("```rust,ignore") {
 219         return false;
 220     }
 221     if extension == "md" && line.trim().starts_with("//") {
 222         // Markdown examples may include doc comments with ignore inside a
 223         // code block.
 224         return false;
 225     }
 226     true
 227 }
 228
 229 pub fn check(path: &Path, bad: &mut bool) {
 230     fn skip(path: &Path) -> bool {
 231         filter_dirs(path) || skip_markdown_path(path)
 232     }
 233     let problematic_consts_strings: Vec<String> = (PROBLEMATIC_CONSTS.iter().map(u32::to_string))
 234         .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:x}", v)))
 235         .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v)))
 236         .collect();
 237     let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
 238     walk(path, &mut skip, &mut |entry, contents| {
 239         let file = entry.path();
 240         let filename = file.file_name().unwrap().to_string_lossy();
 241         let extensions =
 242             [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h", ".md", ".css", ".ftl", ".goml"];
 243         if extensions.iter().all(|e| !filename.ends_with(e)) || filename.starts_with(".#") {
 244             return;
 245         }
 246
 247         let is_style_file = filename.ends_with(".css");
 248         let under_rustfmt = filename.ends_with(".rs") &&
 249             // This list should ideally be sourced from rustfmt.toml but we don't want to add a toml
 250             // parser to tidy.
 251             !file.ancestors().any(|a| {
 252                 (a.ends_with("tests") && a.join("COMPILER_TESTS.md").exists()) ||
 253                     a.ends_with("src/doc/book")
 254             });
 255
 256         if is_style_file && !is_in(file, "src", "librustdoc") {
 257             // We only check CSS files in rustdoc.
 258             return;
 259         }
 260
 261         if contents.is_empty() {
 262             tidy_error!(bad, "{}: empty file", file.display());
 263         }
 264
 265         let extension = file.extension().unwrap().to_string_lossy();
 266         let is_error_code = extension == "md" && is_in(file, "src", "error_codes");
 267         let is_goml_code = extension == "goml";
 268
 269         let max_columns = if is_error_code {
 270             ERROR_CODE_COLS
 271         } else if is_goml_code {
 272             GOML_COLS
 273         } else {
 274             COLS
 275         };
 276
 277         let can_contain = contents.contains("// ignore-tidy-")
 278             || contents.contains("# ignore-tidy-")
 279             || contents.contains("/* ignore-tidy-");
 280         // Enable testing ICE's that require specific (untidy)
 281         // file formats easily eg. `issue-1234-ignore-tidy.rs`
 282         if filename.contains("ignore-tidy") {
 283             return;
 284         }
 285         // apfloat shouldn't be changed because of license problems
 286         if is_in(file, "compiler", "rustc_apfloat") {
 287             return;
 288         }
 289         let mut skip_cr = contains_ignore_directive(can_contain, &contents, "cr");
 290         let mut skip_undocumented_unsafe =
 291             contains_ignore_directive(can_contain, &contents, "undocumented-unsafe");
 292         let mut skip_tab = contains_ignore_directive(can_contain, &contents, "tab");
 293         let mut skip_line_length = contains_ignore_directive(can_contain, &contents, "linelength");
 294         let mut skip_file_length = contains_ignore_directive(can_contain, &contents, "filelength");
 295         let mut skip_end_whitespace =
 296             contains_ignore_directive(can_contain, &contents, "end-whitespace");
 297         let mut skip_trailing_newlines =
 298             contains_ignore_directive(can_contain, &contents, "trailing-newlines");
 299         let mut skip_leading_newlines =
 300             contains_ignore_directive(can_contain, &contents, "leading-newlines");
 301         let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
 302         let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
 303         let mut leading_new_lines = false;
 304         let mut trailing_new_lines = 0;
 305         let mut lines = 0;
 306         let mut last_safety_comment = false;
 307         let is_test = file.components().any(|c| c.as_os_str() == "tests");
 308         // scanning the whole file for multiple needles at once is more efficient than
 309         // executing lines times needles separate searches.
 310         let any_problematic_line = problematic_regex.is_match(contents);
 311         for (i, line) in contents.split('\n').enumerate() {
 312             if line.is_empty() {
 313                 if i == 0 {
 314                     leading_new_lines = true;
 315                 }
 316                 trailing_new_lines += 1;
 317                 continue;
 318             } else {
 319                 trailing_new_lines = 0;
 320             }
 321
 322             let trimmed = line.trim();
 323
 324             if !trimmed.starts_with("//") {
 325                 lines += 1;
 326             }
 327
 328             let mut err = |msg: &str| {
 329                 tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg);
 330             };
 331
 332             if trimmed.contains("dbg!")
 333                 && !trimmed.starts_with("//")
 334                 && !file.ancestors().any(|a| {
 335                     (a.ends_with("tests") && a.join("COMPILER_TESTS.md").exists())
 336                         || a.ends_with("library/alloc/tests")
 337                 })
 338                 && filename != "tests.rs"
 339             {
 340                 suppressible_tidy_err!(
 341                     err,
 342                     skip_dbg,
 343                     "`dbg!` macro is intended as a debugging tool. It should not be in version control."
 344                 )
 345             }
 346
 347             if !under_rustfmt
 348                 && line.chars().count() > max_columns
 349                 && !long_line_is_ok(&extension, is_error_code, max_columns, line)
 350             {
 351                 suppressible_tidy_err!(
 352                     err,
 353                     skip_line_length,
 354                     &format!("line longer than {max_columns} chars")
 355                 );
 356             }
 357             if !is_style_file && line.contains('\t') {
 358                 suppressible_tidy_err!(err, skip_tab, "tab character");
 359             }
 360             if line.ends_with(' ') || line.ends_with('\t') {
 361                 suppressible_tidy_err!(err, skip_end_whitespace, "trailing whitespace");
 362             }
 363             if is_style_file && line.starts_with(' ') {
 364                 err("CSS files use tabs for indent");
 365             }
 366             if line.contains('\r') {
 367                 suppressible_tidy_err!(err, skip_cr, "CR character");
 368             }
 369             if filename != "style.rs" {
 370                 if trimmed.contains("TODO") {
 371                     err("TODO is deprecated; use FIXME")
 372                 }
 373                 if trimmed.contains("//") && trimmed.contains(" XXX") {
 374                     err("XXX is deprecated; use FIXME")
 375                 }
 376                 if any_problematic_line {
 377                     for s in problematic_consts_strings.iter() {
 378                         if trimmed.contains(s) {
 379                             err("Don't use magic numbers that spell things (consider 0x12345678)");
 380                         }
 381                     }
 382                 }
 383             }
 384             // for now we just check libcore
 385             if trimmed.contains("unsafe {") && !trimmed.starts_with("//") && !last_safety_comment {
 386                 if file.components().any(|c| c.as_os_str() == "core") && !is_test {
 387                     suppressible_tidy_err!(err, skip_undocumented_unsafe, "undocumented unsafe");
 388                 }
 389             }
 390             if trimmed.contains("// SAFETY:") {
 391                 last_safety_comment = true;
 392             } else if trimmed.starts_with("//") || trimmed.is_empty() {
 393                 // keep previous value
 394             } else {
 395                 last_safety_comment = false;
 396             }
 397             if (line.starts_with("// Copyright")
 398                 || line.starts_with("# Copyright")
 399                 || line.starts_with("Copyright"))
 400                 && (trimmed.contains("Rust Developers")
 401                     || trimmed.contains("Rust Project Developers"))
 402             {
 403                 suppressible_tidy_err!(
 404                     err,
 405                     skip_copyright,
 406                     "copyright notices attributed to the Rust Project Developers are deprecated"
 407                 );
 408             }
 409             if is_unexplained_ignore(&extension, line) {
 410                 err(UNEXPLAINED_IGNORE_DOCTEST_INFO);
 411             }
 412             if filename.ends_with(".cpp") && line.contains("llvm_unreachable") {
 413                 err(LLVM_UNREACHABLE_INFO);
 414             }
 415
 416             // For now only enforce in compiler
 417             let is_compiler = || file.components().any(|c| c.as_os_str() == "compiler");
 418             if is_compiler()
 419                 && line.contains("//")
 420                 && line
 421                     .chars()
 422                     .collect::<Vec<_>>()
 423                     .windows(4)
 424                     .any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic()))
 425             {
 426                 err(DOUBLE_SPACE_AFTER_DOT)
 427             }
 428         }
 429         if leading_new_lines {
 430             let mut err = |_| {
 431                 tidy_error!(bad, "{}: leading newline", file.display());
 432             };
 433             suppressible_tidy_err!(err, skip_leading_newlines, "mising leading newline");
 434         }
 435         let mut err = |msg: &str| {
 436             tidy_error!(bad, "{}: {}", file.display(), msg);
 437         };
 438         match trailing_new_lines {
 439             0 => suppressible_tidy_err!(err, skip_trailing_newlines, "missing trailing newline"),
 440             1 => {}
 441             n => suppressible_tidy_err!(
 442                 err,
 443                 skip_trailing_newlines,
 444                 &format!("too many trailing newlines ({n})")
 445             ),
 446         };
 447         if lines > LINES {
 448             let mut err = |_| {
 449                 tidy_error!(
 450                     bad,
 451                     "{}: too many lines ({}) (add `// \
 452                      ignore-tidy-filelength` to the file to suppress this error)",
 453                     file.display(),
 454                     lines
 455                 );
 456             };
 457             suppressible_tidy_err!(err, skip_file_length, "");
 458         }
 459
 460         if let Directive::Ignore(false) = skip_cr {
 461             tidy_error!(bad, "{}: ignoring CR characters unnecessarily", file.display());
 462         }
 463         if let Directive::Ignore(false) = skip_tab {
 464             tidy_error!(bad, "{}: ignoring tab characters unnecessarily", file.display());
 465         }
 466         if let Directive::Ignore(false) = skip_end_whitespace {
 467             tidy_error!(bad, "{}: ignoring trailing whitespace unnecessarily", file.display());
 468         }
 469         if let Directive::Ignore(false) = skip_trailing_newlines {
 470             tidy_error!(bad, "{}: ignoring trailing newlines unnecessarily", file.display());
 471         }
 472         if let Directive::Ignore(false) = skip_leading_newlines {
 473             tidy_error!(bad, "{}: ignoring leading newlines unnecessarily", file.display());
 474         }
 475         if let Directive::Ignore(false) = skip_copyright {
 476             tidy_error!(bad, "{}: ignoring copyright unnecessarily", file.display());
 477         }
 478         // We deliberately do not warn about these being unnecessary,
 479         // that would just lead to annoying churn.
 480         let _unused = skip_line_length;
 481         let _unused = skip_file_length;
 482     })
 483 }