From: varkor Date: Mon, 22 Apr 2019 19:56:13 +0000 (+0100) Subject: Check for other unused tidy check directives X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=7f7d15dcc9c18bb9172d941ead41abcc26f50a98;p=rust.git Check for other unused tidy check directives --- diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index b373725617c..9ab88d6e9ae 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -9,8 +9,8 @@ //! * No `TODO` or `XXX` directives. //! * No unexplained ` ```ignore ` or ` ```rust,ignore ` doc tests. //! -//! A number of these checks can be opted-out of with various directives like -//! `// ignore-tidy-copyright`. +//! A number of these checks can be opted-out of with various directives of the form: +//! `// ignore-tidy-CHECK-NAME`. use std::fs::File; use std::io::prelude::*; @@ -90,9 +90,34 @@ fn long_line_is_ok(line: &str) -> bool { false } -fn contains_ignore_directive(contents: &String, check: &str) -> bool { - contents.contains(&format!("// ignore-tidy-{}", check)) || - contents.contains(&format!("# ignore-tidy-{}", check)) +enum Directive { + /// By default, tidy always warns against style issues. + Deny, + + /// `Ignore(false)` means that an `ignore-tidy-*` directive + /// has been provided, but is unnecessary. `Ignore(true)` + /// means that it is necessary (i.e. a warning would be + /// produced if `ignore-tidy-*` was not present). + Ignore(bool), +} + +fn contains_ignore_directive(contents: &String, check: &str) -> Directive { + if contents.contains(&format!("// ignore-tidy-{}", check)) || + contents.contains(&format!("# ignore-tidy-{}", check)) { + Directive::Ignore(false) + } else { + Directive::Deny + } +} + +macro_rules! suppressible_tidy_err { + ($err:ident, $skip:ident, $msg:expr) => { + if let Directive::Deny = $skip { + $err($msg); + } else { + $skip = Directive::Ignore(true); + } + }; } pub fn check(path: &Path, bad: &mut bool) { @@ -112,32 +137,32 @@ pub fn check(path: &Path, bad: &mut bool) { tidy_error!(bad, "{}: empty file", file.display()); } - let skip_cr = contains_ignore_directive(&contents, "cr"); - let skip_tab = contains_ignore_directive(&contents, "tab"); - let skip_length = contains_ignore_directive(&contents, "linelength"); - let skip_end_whitespace = contains_ignore_directive(&contents, "end-whitespace"); - let skip_copyright = contains_ignore_directive(&contents, "copyright"); + let mut skip_cr = contains_ignore_directive(&contents, "cr"); + let mut skip_tab = contains_ignore_directive(&contents, "tab"); + let mut skip_length = contains_ignore_directive(&contents, "linelength"); + let mut skip_end_whitespace = contains_ignore_directive(&contents, "end-whitespace"); + let mut skip_copyright = contains_ignore_directive(&contents, "copyright"); let mut leading_new_lines = false; let mut trailing_new_lines = 0; - let mut contained_long_line = false; for (i, line) in contents.split('\n').enumerate() { let mut err = |msg: &str| { tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg); }; if line.chars().count() > COLS && !long_line_is_ok(line) { - contained_long_line = true; - if !skip_length { - err(&format!("line longer than {} chars", COLS)); - } + suppressible_tidy_err!( + err, + skip_length, + &format!("line longer than {} chars", COLS) + ); } - if !skip_tab && line.contains('\t') { - err("tab character"); + if line.contains('\t') { + suppressible_tidy_err!(err, skip_tab, "tab character"); } - if !skip_end_whitespace && (line.ends_with(' ') || line.ends_with('\t')) { - err("trailing whitespace"); + if line.ends_with(' ') || line.ends_with('\t') { + suppressible_tidy_err!(err, skip_end_whitespace, "trailing whitespace"); } - if !skip_cr && line.contains('\r') { - err("CR character"); + if line.contains('\r') { + suppressible_tidy_err!(err, skip_cr, "CR character"); } if filename != "style.rs" { if line.contains("TODO") { @@ -147,12 +172,16 @@ pub fn check(path: &Path, bad: &mut bool) { err("XXX is deprecated; use FIXME") } } - if !skip_copyright && (line.starts_with("// Copyright") || - line.starts_with("# Copyright") || - line.starts_with("Copyright")) - && (line.contains("Rust Developers") || - line.contains("Rust Project Developers")) { - err("copyright notices attributed to the Rust Project Developers are deprecated"); + if (line.starts_with("// Copyright") || + line.starts_with("# Copyright") || + line.starts_with("Copyright")) + && (line.contains("Rust Developers") || + line.contains("Rust Project Developers")) { + suppressible_tidy_err!( + err, + skip_copyright, + "copyright notices attributed to the Rust Project Developers are deprecated" + ); } if line.ends_with("```ignore") || line.ends_with("```rust,ignore") { err(UNEXPLAINED_IGNORE_DOCTEST_INFO); @@ -177,8 +206,21 @@ pub fn check(path: &Path, bad: &mut bool) { 1 => {} n => tidy_error!(bad, "{}: too many trailing newlines ({})", file.display(), n), }; - if !contained_long_line && skip_length { + + if let Directive::Ignore(false) = skip_cr { + tidy_error!(bad, "{}: ignoring CR characters unnecessarily", file.display()); + } + if let Directive::Ignore(false) = skip_tab { + tidy_error!(bad, "{}: ignoring tab characters unnecessarily", file.display()); + } + if let Directive::Ignore(false) = skip_length { tidy_error!(bad, "{}: ignoring line length unnecessarily", file.display()); } + if let Directive::Ignore(false) = skip_end_whitespace { + tidy_error!(bad, "{}: ignoring trailing whitespace unnecessarily", file.display()); + } + if let Directive::Ignore(false) = skip_copyright { + tidy_error!(bad, "{}: ignoring copyright unnecessarily", file.display()); + } }) }