From 7ac15f90002b653f9ba61185858581057d731ee0 Mon Sep 17 00:00:00 2001 From: Raghul Nanth A <61490162+NanthR@users.noreply.github.com> Date: Sun, 7 May 2023 12:10:04 +0530 Subject: [PATCH] Add lint to check lint formulation messages Fix lints that don't conform to the standard formulation --- clippy_lints/Cargo.toml | 3 +- clippy_lints/src/casts/mod.rs | 2 +- clippy_lints/src/declared_lints.rs | 2 + .../src/default_constructed_unit_structs.rs | 2 +- clippy_lints/src/lib.rs | 3 + clippy_lints/src/loops/mod.rs | 6 +- clippy_lints/src/matches/mod.rs | 2 +- clippy_lints/src/utils/internal_lints.rs | 1 + .../almost_standard_lint_formulation.rs | 87 +++++++++++++++++++ .../allow_incorrect_lint_formulations.rs | 22 +++++ tests/ui-internal/check_formulation.rs | 43 +++++++++ tests/ui-internal/check_formulation.stderr | 19 ++++ 12 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 clippy_lints/src/utils/internal_lints/almost_standard_lint_formulation.rs create mode 100644 tests/ui-internal/allow_incorrect_lint_formulations.rs create mode 100644 tests/ui-internal/check_formulation.rs create mode 100644 tests/ui-internal/check_formulation.stderr diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index be9261a4704..c23054443bb 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -22,6 +22,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", optional = true } tempfile = { version = "3.3.0", optional = true } toml = "0.7.3" +regex = { version = "1.5", optional = true } unicode-normalization = "0.1" unicode-script = { version = "0.5", default-features = false } semver = "1.0" @@ -31,7 +32,7 @@ url = "2.2" [features] deny-warnings = ["clippy_utils/deny-warnings"] # build clippy with internal lints enabled, off by default -internal = ["clippy_utils/internal", "serde_json", "tempfile"] +internal = ["clippy_utils/internal", "serde_json", "tempfile", "regex"] [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)] diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 4e8d5c3bccd..d2166872d93 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -522,7 +522,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Check for the usage of `as _` conversion using inferred type. + /// Checks for the usage of `as _` conversion using inferred type. /// /// ### Why is this bad? /// The conversion might include lossy conversion and dangerous cast that might go diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a0b8e055f1d..01ef3ef431c 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -3,6 +3,8 @@ // Manual edits will be overwritten. pub(crate) static LINTS: &[&crate::LintInfo] = &[ + #[cfg(feature = "internal")] + crate::utils::internal_lints::almost_standard_lint_formulation::ALMOST_STANDARD_LINT_FORMULATION_INFO, #[cfg(feature = "internal")] crate::utils::internal_lints::clippy_lints_internal::CLIPPY_LINTS_INTERNAL_INFO, #[cfg(feature = "internal")] diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index fb037bbcbf3..ca9514ccc7d 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// Check for construction on unit struct using `default`. + /// Checks for construction on unit struct using `default`. /// /// ### Why is this bad? /// This adds code complexity and an unnecessary function call. diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4a23edb58aa..43ab8a96d86 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -564,6 +564,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::new(utils::internal_lints::outer_expn_data_pass::OuterExpnDataPass)); store.register_late_pass(|_| Box::new(utils::internal_lints::msrv_attr_impl::MsrvAttrImpl)); + store.register_late_pass(|_| { + Box::new(utils::internal_lints::almost_standard_lint_formulation::AlmostStandardFormulation::new()) + }); } let arithmetic_side_effects_allowed = conf.arithmetic_side_effects_allowed.clone(); diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index f83ad388a74..c7c11809752 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -479,7 +479,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Check for unnecessary `if let` usage in a for loop + /// Checks for unnecessary `if let` usage in a for loop /// where only the `Some` or `Ok` variant of the iterator element is used. /// /// ### Why is this bad? @@ -511,7 +511,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Check for empty spin loops + /// Checks for empty spin loops /// /// ### Why is this bad? /// The loop body should have something like `thread::park()` or at least @@ -547,7 +547,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Check for manual implementations of Iterator::find + /// Checks for manual implementations of Iterator::find /// /// ### Why is this bad? /// It doesn't affect performance, but using `find` is shorter and easier to read. diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 0d91051632a..2d27be499f9 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -777,7 +777,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Check for temporaries returned from function calls in a match scrutinee that have the + /// Checks for temporaries returned from function calls in a match scrutinee that have the /// `clippy::has_significant_drop` attribute. /// /// ### Why is this bad? diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 71f6c9909dd..e222a5448c9 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,3 +1,4 @@ +pub mod almost_standard_lint_formulation; pub mod clippy_lints_internal; pub mod collapsible_calls; pub mod compiler_lint_functions; diff --git a/clippy_lints/src/utils/internal_lints/almost_standard_lint_formulation.rs b/clippy_lints/src/utils/internal_lints/almost_standard_lint_formulation.rs new file mode 100644 index 00000000000..e4cb5316a98 --- /dev/null +++ b/clippy_lints/src/utils/internal_lints/almost_standard_lint_formulation.rs @@ -0,0 +1,87 @@ +use crate::utils::internal_lints::lint_without_lint_pass::is_lint_ref_type; +use clippy_utils::diagnostics::span_lint_and_help; +use regex::Regex; +use rustc_ast as ast; +use rustc_hir::{Item, ItemKind, Mutability}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + +declare_clippy_lint! { + /// ### What it does + /// Checks if lint formulations have a standardized format. + /// + /// ### Why is this bad? + /// It's not neccessarily bad, but we try to enforce a standard in Clippy. + /// + /// ### Example + /// `Checks for use...` can be written as `Checks for usage...` . + pub ALMOST_STANDARD_LINT_FORMULATION, + internal, + "lint formulations must have a standardized format." +} + +impl_lint_pass!(AlmostStandardFormulation => [ALMOST_STANDARD_LINT_FORMULATION]); + +pub struct AlmostStandardFormulation { + standard_formulations: Vec>, +} + +#[derive(Debug)] +struct StandardFormulations<'a> { + wrong_pattern: Regex, + correction: &'a str, +} + +impl AlmostStandardFormulation { + pub fn new() -> Self { + let standard_formulations = vec![StandardFormulations { + wrong_pattern: Regex::new(r"^(Check for|Detects? uses?)").unwrap(), + correction: "Checks for", + }]; + Self { standard_formulations } + } +} + +impl<'tcx> LateLintPass<'tcx> for AlmostStandardFormulation { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + let mut check_next = false; + if let ItemKind::Static(ty, Mutability::Not, _) = item.kind { + let lines = cx + .tcx + .hir() + .attrs(item.hir_id()) + .iter() + .filter_map(|attr| ast::Attribute::doc_str(attr).map(|sym| (sym, attr))); + if is_lint_ref_type(cx, ty) { + for (line, attr) in lines { + let cur_line = line.as_str().trim(); + if check_next && !cur_line.is_empty() { + for formulation in &self.standard_formulations { + let starts_with_correct_formulation = cur_line.starts_with(formulation.correction); + if !starts_with_correct_formulation && formulation.wrong_pattern.is_match(cur_line) { + if let Some(ident) = attr.ident() { + span_lint_and_help( + cx, + ALMOST_STANDARD_LINT_FORMULATION, + ident.span, + "non-standard lint formulation", + None, + &format!("try using `{}` instead", formulation.correction), + ); + } + return; + } + } + return; + } else if cur_line.contains("What it does") { + check_next = true; + } else if cur_line.contains("Why is this bad") { + // Formulation documentation is done. Can add check to ensure that missing formulation is added + // and add a check if it matches no accepted formulation + return; + } + } + } + } + } +} diff --git a/tests/ui-internal/allow_incorrect_lint_formulations.rs b/tests/ui-internal/allow_incorrect_lint_formulations.rs new file mode 100644 index 00000000000..7d1e0f26a1b --- /dev/null +++ b/tests/ui-internal/allow_incorrect_lint_formulations.rs @@ -0,0 +1,22 @@ +#![allow(clippy::almost_standard_lint_formulation)] +#![feature(rustc_private)] + +#[macro_use] +extern crate rustc_middle; +#[macro_use] +extern crate rustc_session; +extern crate rustc_lint; + +declare_tool_lint! { + /// # What it does + /// Detects uses of incorrect formulations + #[clippy::version = "pre 1.29.0"] + pub clippy::ALLOWED_INVALID, + Warn, + "One", + report_in_external_macro: true +} + +declare_lint_pass!(Pass => [ALLOWED_INVALID]); + +fn main() {} diff --git a/tests/ui-internal/check_formulation.rs b/tests/ui-internal/check_formulation.rs new file mode 100644 index 00000000000..d6ebadf04c4 --- /dev/null +++ b/tests/ui-internal/check_formulation.rs @@ -0,0 +1,43 @@ +#![warn(clippy::almost_standard_lint_formulation)] +#![feature(rustc_private)] + +#[macro_use] +extern crate rustc_middle; +#[macro_use] +extern crate rustc_session; +extern crate rustc_lint; + +declare_tool_lint! { + /// # What it does + /// + /// Checks for usage of correct lint formulations + #[clippy::version = "pre 1.29.0"] + pub clippy::VALID, + Warn, + "One", + report_in_external_macro: true +} + +declare_tool_lint! { + /// # What it does + /// Check for lint formulations that are correct + #[clippy::version = "pre 1.29.0"] + pub clippy::INVALID1, + Warn, + "One", + report_in_external_macro: true +} + +declare_tool_lint! { + /// # What it does + /// Detects uses of incorrect formulations + #[clippy::version = "pre 1.29.0"] + pub clippy::INVALID2, + Warn, + "One", + report_in_external_macro: true +} + +declare_lint_pass!(Pass => [VALID, INVALID1, INVALID2]); + +fn main() {} diff --git a/tests/ui-internal/check_formulation.stderr b/tests/ui-internal/check_formulation.stderr new file mode 100644 index 00000000000..10eabca4b9d --- /dev/null +++ b/tests/ui-internal/check_formulation.stderr @@ -0,0 +1,19 @@ +error: non-standard lint formulation + --> $DIR/check_formulation.rs:23:5 + | +LL | /// Check for lint formulations that are correct + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try using `Checks for` instead + = note: `-D clippy::almost-standard-lint-formulation` implied by `-D warnings` + +error: non-standard lint formulation + --> $DIR/check_formulation.rs:33:5 + | +LL | /// Detects uses of incorrect formulations + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try using `Checks for` instead + +error: aborting due to 2 previous errors +