Auto merge of #10760 - NanthR:almost_standard_formulation, r=xFrednet

Standard lint formulations

A WIP that fixes #10660. Fix lints that don't conform to the standard formulation.

changelog: none

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)]

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

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")]

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.

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::<utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath>::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();

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.

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?

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;

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<StandardFormulations<'static>>,
+}
+
+#[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;
+                    }
+                }
+            }
+        }
+    }
+}

tests/ui-internal/check_formulation.rs

@@ -0,0 +1,54 @@
+#![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_tool_lint! {
+    /// # What it does
+    /// Detects uses of incorrect formulations (allowed with attribute)
+    #[allow(clippy::almost_standard_lint_formulation)]
+    #[clippy::version = "pre 1.29.0"]
+    pub clippy::ALLOWED_INVALID,
+    Warn,
+    "One",
+    report_in_external_macro: true
+}
+
+declare_lint_pass!(Pass => [VALID, INVALID1, INVALID2]);
+
+fn main() {}

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
+