feat(needless_impl_lint_pass): new internal lint

clippy_lints_internal/src/lib.rs

@@ -36,6 +36,7 @@ mod derive_deserialize_allowing_unknown;
 mod internal_paths;
 mod lint_without_lint_pass;
 mod msrv_attr_impl;
+mod needless_impl_lint_pass;
 mod outer_expn_data_pass;
 mod produce_ice;
 mod repeated_is_diagnostic_item;
@@ -62,6 +63,7 @@ static LINTS: &[&Lint] = &[
     unnecessary_def_path::UNNECESSARY_DEF_PATH,
     unsorted_clippy_utils_paths::UNSORTED_CLIPPY_UTILS_PATHS,
     unusual_names::UNUSUAL_NAMES,
+    needless_impl_lint_pass::NEEDLESS_IMPL_LINT_PASS,
 ];
 
 pub fn register_lints(store: &mut LintStore) {
@@ -79,4 +81,5 @@ pub fn register_lints(store: &mut LintStore) {
     store.register_late_pass(|_| Box::new(almost_standard_lint_formulation::AlmostStandardFormulation::new()));
     store.register_late_pass(|_| Box::new(unusual_names::UnusualNames));
     store.register_late_pass(|_| Box::new(repeated_is_diagnostic_item::RepeatedIsDiagnosticItem));
+    store.register_late_pass(|_| Box::new(needless_impl_lint_pass::NeedlessImplLintPass));
 }

clippy_lints_internal/src/needless_impl_lint_pass.rs

@@ -0,0 +1,116 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_indent;
+use clippy_utils::{is_expn_of, sym};
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Item, ItemKind, QPath, TyKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{BytePos, Pos};
+
+declare_tool_lint! {
+    /// ### What it does
+    /// Checks for usage of `impl_lint_pass!` on lint pass structs without fields, and suggests
+    /// to use `declare_lint_pass!` instead.
+    ///
+    /// ### Why is this bad?
+    /// Using `impl_lint_pass!` is only necessary when the lint pass struct needs to have fields.
+    /// Without them, `declare_lint_pass!` can be used for more concise code.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// use rustc_session::impl_lint_pass;
+    ///
+    /// struct LintPassWithoutFields;
+    ///
+    /// impl_lint_pass!(LintPassWithoutFields => []);
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust,ignore
+    /// use rustc_session::declare_lint_pass;
+    ///
+    /// declare_lint_pass!(LintPassWithoutFields => [SOME_LINT]);
+    /// ```
+    ///
+    /// ### Known problems
+    /// FN if `impl_lint_pass!` is used in a qualified way, e.g. as `rustc_session::impl_lint_pass!`
+    pub clippy::NEEDLESS_IMPL_LINT_PASS,
+    Warn,
+    "using `impl_lint_pass!` for a lint pass struct without fields",
+    report_in_external_macro: false
+}
+
+declare_lint_pass!(NeedlessImplLintPass => [NEEDLESS_IMPL_LINT_PASS]);
+
+impl<'tcx> LateLintPass<'tcx> for NeedlessImplLintPass {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let Some(impl_lint_pass_span) = is_expn_of(item.span, sym::impl_lint_pass)
+            // `declare_lint_pass!` itself calls `impl_lint_pass!` -- make sure that we're not inside the former
+            && is_expn_of(impl_lint_pass_span, sym::declare_lint_pass).is_none()
+            // `impl_lint_pass!` contains two impls, one of which is an inherent impl
+            // on the lint pass struct -- that's the one we'll look for
+            && let ItemKind::Impl(impl_) = item.kind
+            && impl_.of_trait.is_none()
+            && let TyKind::Path(QPath::Resolved(None, path)) = impl_.self_ty.kind
+            && let Res::Def(DefKind::Struct, lint_pass_struct) = path.res
+            // check that the lint pass struct doesn't have any fields
+            && cx.tcx.adt_def(lint_pass_struct).all_fields().next().is_none()
+        {
+            span_lint_and_then(
+                cx,
+                NEEDLESS_IMPL_LINT_PASS,
+                impl_lint_pass_span,
+                "`impl_lint_pass!` on a lint pass struct without fields",
+                |diag| {
+                    let Some(lint_pass_struct) = lint_pass_struct.as_local() else {
+                        // Shouldn't really happen, as lint passes are basically always impl'd where they're defined
+                        return;
+                    };
+                    let lint_pass_decl = cx.tcx.hir_node_by_def_id(lint_pass_struct).expect_item();
+                    diag.span_label(lint_pass_decl.span, "struct defined here");
+                    // This is `DiagExt::suggest_remove_item`, but doesn't actually create the suggestion
+                    // -- we want to combine it with the macro replacement suggestion instead
+                    let remove_lint_pass_span = {
+                        let mut remove_span = lint_pass_decl.span;
+                        let fmpos = cx.sess().source_map().lookup_byte_offset(remove_span.hi());
+
+                        if let Some(ref src) = fmpos.sf.src {
+                            let non_whitespace_offset =
+                                src[fmpos.pos.to_usize()..].find(|c| c != ' ' && c != '\t' && c != '\n');
+
+                            if let Some(non_whitespace_offset) = non_whitespace_offset {
+                                remove_span = remove_span.with_hi(
+                                    remove_span.hi()
+                                        + BytePos(non_whitespace_offset.try_into().expect("offset too large")),
+                                );
+                            }
+                        }
+                        remove_span
+                    };
+
+                    let module_id = cx.tcx.parent_module_from_def_id(lint_pass_struct);
+                    let (module, _, _) = cx.tcx.hir_get_module(module_id);
+                    let import_span = module.spans.inject_use_span;
+                    let import_sugg = format!(
+                        "use rustc_session::declare_lint_pass;\n{indent_of_imports}",
+                        indent_of_imports = snippet_indent(cx.sess(), import_span).unwrap_or_default(),
+                    );
+
+                    let sugg = vec![
+                        (import_span, import_sugg),
+                        (remove_lint_pass_span, String::new()),
+                        // Cut out the `impl` from `impl_lint_pass!` and replace it with `declare`
+                        // -- sidesteps the annoyance of creating the snippets
+                        (impl_lint_pass_span.split_at(4).0, "declare".to_string()),
+                    ];
+                    diag.multipart_suggestion_verbose(
+                        "use `declare_lint_pass!`",
+                        sugg,
+                        Applicability::MaybeIncorrect, // because of the import
+                    );
+                },
+            );
+        }
+    }
+}

tests/ui-internal/lint_without_lint_pass.rs

@@ -1,5 +1,5 @@
 #![deny(clippy::lint_without_lint_pass)]
-#![allow(clippy::missing_clippy_version_attribute)]
+#![allow(clippy::missing_clippy_version_attribute, clippy::needless_impl_lint_pass)]
 #![feature(rustc_private)]
 
 #[macro_use]

tests/ui-internal/needless_impl_lint_pass.fixed

@@ -0,0 +1,48 @@
+#![feature(rustc_private)]
+#![warn(clippy::needless_impl_lint_pass)]
+
+extern crate rustc_lint;
+extern crate rustc_middle;
+extern crate rustc_session;
+
+use rustc_session::{declare_lint_pass, impl_lint_pass};
+
+mod semi {
+    use rustc_session::declare_lint_pass;
+    use super::*;
+
+    declare_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+}
+
+mod braces {
+    use rustc_session::declare_lint_pass;
+    use super::*;
+
+    declare_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+}
+
+mod after_macro {
+    use rustc_session::declare_lint_pass;
+    use super::*;
+
+    declare_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+
+    }
+
+mod has_fields {
+    use super::*;
+    use clippy_utils::msrvs::Msrv;
+
+    struct WithFields {
+        msrv: Msrv,
+    }
+
+    // don't lint: can't use `declare_lint_pass!` because of the field
+    impl_lint_pass!(WithFields => []);
+}
+
+// don't lint: `impl_lint_pass!` not written by the user
+declare_lint_pass!(NoFields2 => []);

tests/ui-internal/needless_impl_lint_pass.rs

@@ -0,0 +1,50 @@
+#![feature(rustc_private)]
+#![warn(clippy::needless_impl_lint_pass)]
+
+extern crate rustc_lint;
+extern crate rustc_middle;
+extern crate rustc_session;
+
+use rustc_session::{declare_lint_pass, impl_lint_pass};
+
+mod semi {
+    use super::*;
+
+    struct WithoutFields;
+
+    impl_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+}
+
+mod braces {
+    use super::*;
+
+    struct WithoutFields {}
+
+    impl_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+}
+
+mod after_macro {
+    use super::*;
+
+    impl_lint_pass!(WithoutFields => []);
+    //~^ needless_impl_lint_pass
+
+    struct WithoutFields;
+}
+
+mod has_fields {
+    use super::*;
+    use clippy_utils::msrvs::Msrv;
+
+    struct WithFields {
+        msrv: Msrv,
+    }
+
+    // don't lint: can't use `declare_lint_pass!` because of the field
+    impl_lint_pass!(WithFields => []);
+}
+
+// don't lint: `impl_lint_pass!` not written by the user
+declare_lint_pass!(NoFields2 => []);

tests/ui-internal/needless_impl_lint_pass.stderr

@@ -0,0 +1,58 @@
+error: `impl_lint_pass!` on a lint pass struct without fields
+  --> tests/ui-internal/needless_impl_lint_pass.rs:15:5
+   |
+LL |     struct WithoutFields;
+   |     --------------------- struct defined here
+LL |
+LL |     impl_lint_pass!(WithoutFields => []);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::needless-impl-lint-pass` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::needless_impl_lint_pass)]`
+help: use `declare_lint_pass!`
+   |
+LL ~     use rustc_session::declare_lint_pass;
+LL ~     use super::*;
+LL |
+LL ~     declare_lint_pass!(WithoutFields => []);
+   |
+
+error: `impl_lint_pass!` on a lint pass struct without fields
+  --> tests/ui-internal/needless_impl_lint_pass.rs:24:5
+   |
+LL |     struct WithoutFields {}
+   |     ----------------------- struct defined here
+LL |
+LL |     impl_lint_pass!(WithoutFields => []);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `declare_lint_pass!`
+   |
+LL ~     use rustc_session::declare_lint_pass;
+LL ~     use super::*;
+LL |
+LL ~     declare_lint_pass!(WithoutFields => []);
+   |
+
+error: `impl_lint_pass!` on a lint pass struct without fields
+  --> tests/ui-internal/needless_impl_lint_pass.rs:31:5
+   |
+LL |     impl_lint_pass!(WithoutFields => []);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     struct WithoutFields;
+   |     --------------------- struct defined here
+   |
+help: use `declare_lint_pass!`
+   |
+LL ~     use rustc_session::declare_lint_pass;
+LL ~     use super::*;
+LL |
+LL ~     declare_lint_pass!(WithoutFields => []);
+LL |
+LL |
+LL ~     }
+   |
+
+error: aborting due to 3 previous errors
+