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 1/2] 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::<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();
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<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;
+ }
+ }
+ }
+ }
+ }
+}
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
+
From 50c93bbfd225f202b4b0060c95bd55a6e504dd36 Mon Sep 17 00:00:00 2001
From: Raghul Nanth A <61490162+NanthR@users.noreply.github.com>
Date: Fri, 2 Jun 2023 23:56:30 +0530
Subject: [PATCH 2/2] refactor(test): Move attribute to lint level
---
.../allow_incorrect_lint_formulations.rs | 22 -------------------
tests/ui-internal/check_formulation.rs | 11 ++++++++++
2 files changed, 11 insertions(+), 22 deletions(-)
delete mode 100644 tests/ui-internal/allow_incorrect_lint_formulations.rs
diff --git a/tests/ui-internal/allow_incorrect_lint_formulations.rs b/tests/ui-internal/allow_incorrect_lint_formulations.rs
deleted file mode 100644
index 7d1e0f26a1b..00000000000
--- a/tests/ui-internal/allow_incorrect_lint_formulations.rs
+++ /dev/null
@@ -1,22 +0,0 @@
-#![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
index d6ebadf04c4..43fc996033e 100644
--- a/tests/ui-internal/check_formulation.rs
+++ b/tests/ui-internal/check_formulation.rs
@@ -38,6 +38,17 @@ declare_tool_lint! {
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() {}