Make lint silence locations more intuitive (#162)

Fixes #132.

`plugin_not_ending_in_plugin` and `main_return_without_appexit` check
for `impl` blocks, but emit the lint on the original structure
definition. This results in confusing locations for `#[allow(...)]`
attributes:

```rust
// This doesn't actually silence the lint.
#[allow(bevy::plugin_not_ending_in_plugin)]
struct Foo;

// But this does.
#[allow(bevy::plugin_not_ending_in_plugin)]
impl Plugin for Foo {
    fn build(&self, _app: &mut App) {}
}
```

This can be fixed by calling `span_lint_hir()`, which lets you specify
the `HirId` of the item that the lint is attached to.

This PR switches `main_return_without_appexit` and
`plugin_not_ending_in_plugin` to specify the `HirId` of the structure
definition, not the `impl` block, making the `#[allow(...)]` location
more intuitive.

bevy_lint/src/lints/main_return_without_appexit.rs

@@ -9,11 +9,6 @@
 //! it from `main()` will set the exit code, which allows external processes to detect whether there
 //! was an error.
 //!
-//! # Known issues
-//!
-//! If you wish to silence this lint, you must add `#[allow(bevy::main_return_without_appexit)]` to
-//! `fn main()`, not the line that calls `App::run()`.
-//!
 //! # Example
 //!
 //! ```
@@ -37,7 +32,8 @@
 
 use crate::declare_bevy_lint;
 use clippy_utils::{
-    diagnostics::span_lint_and_then, is_entrypoint_fn, sym, ty::match_type, visitors::for_each_expr,
+    diagnostics::span_lint_hir_and_then, is_entrypoint_fn, sym, ty::match_type,
+    visitors::for_each_expr,
 };
 use rustc_errors::Applicability;
 use rustc_hir::{
@@ -93,13 +89,15 @@ impl<'tcx> LateLintPass<'tcx> for MainReturnWithoutAppExit {
 
                     // If `src` is a Bevy `App`, emit the lint.
                     if match_type(cx, ty, &crate::paths::APP) {
-                        span_lint_and_then(
+                        span_lint_hir_and_then(
                             cx,
                             MAIN_RETURN_WITHOUT_APPEXIT.lint,
+                            expr.hir_id,
                             method_span,
                             MAIN_RETURN_WITHOUT_APPEXIT.lint.desc,
                             |diag| {
                                 diag.note("`App::run()` returns `AppExit`, which can be used to determine whether the app exited successfully or not");
+
                                 match declaration.output {
                                     // When it is just `fn main()`, we need to suggest the `->`.
                                     FnRetTy::DefaultReturn(fn_return_span) => diag.span_suggestion(

bevy_lint/src/lints/plugin_not_ending_in_plugin.rs

@@ -9,12 +9,6 @@
 //! `Plugin` is to be a Bevy plugin. As such, it is common practice to suffix plugin names with
 //! "Plugin" to signal how they should be used.
 //!
-//! # Known issues
-//!
-//! Due to technical reasons, if you wish to silence this lint you need to annotate the
-//! `impl Plugin for T` line with `#[allow(bevy::plugin_not_ending_in_plugin)]`, not the `struct T`
-//! line.
-//!
 //! # Example
 //!
 //! ```
@@ -44,9 +38,9 @@
 //! ```
 
 use crate::declare_bevy_lint;
-use clippy_utils::{diagnostics::span_lint_and_then, match_def_path, path_res};
+use clippy_utils::{diagnostics::span_lint_hir_and_then, match_def_path, path_res};
 use rustc_errors::Applicability;
-use rustc_hir::{def::Res, Item, ItemKind};
+use rustc_hir::{def::Res, HirId, Item, ItemKind, OwnerId};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::symbol::Ident;
@@ -70,14 +64,14 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin {
             // ...where the trait is a path to user code... (I don't believe this will ever be
             // false, since the alternatives are primitives, `Self`, and others that wouldn't make
             // since in this scenario.)
-            && let Res::Def(_, def_id) = of_trait.path.res
+            && let Res::Def(_, trait_def_id) = of_trait.path.res
             // ...where the trait being implemented is Bevy's `Plugin`...
-            && match_def_path(cx, def_id, &crate::paths::PLUGIN)
+            && match_def_path(cx, trait_def_id, &crate::paths::PLUGIN)
         {
             // Try to resolve where this type was originally defined. This will result in a `DefId`
             // pointing to the original `struct Foo` definition, or `impl <T>` if it's a generic
             // parameter.
-            let Some(def_id) = path_res(cx, impl_.self_ty).opt_def_id() else {
+            let Some(struct_def_id) = path_res(cx, impl_.self_ty).opt_def_id() else {
                 return;
             };
 
@@ -87,39 +81,56 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin {
                 .generics
                 .params
                 .iter()
-                .any(|param| param.def_id.to_def_id() == def_id)
+                .any(|param| param.def_id.to_def_id() == struct_def_id)
             {
                 return;
             }
 
             // Find the original name and span of the type. (We don't use the name from the path,
             // since that can be spoofed through `use Foo as FooPlugin`.)
             let Some(Ident {
-                name: self_name,
-                span: self_span,
-            }) = cx.tcx.opt_item_ident(def_id)
+                name: struct_name,
+                span: struct_span,
+            }) = cx.tcx.opt_item_ident(struct_def_id)
             else {
                 return;
             };
 
             // If the type's name ends in "Plugin", exit.
-            if self_name.as_str().ends_with("Plugin") {
+            if struct_name.as_str().ends_with("Plugin") {
+                return;
+            }
+
+            // Convert the `DefId` of the structure to a `LocalDefId`. If it cannot be converted
+            // then the struct is from an external crate, in which case this lint should not be
+            // emitted. (The user cannot easily rename that struct if they didn't define it.)
+            let Some(struct_local_def_id) = struct_def_id.as_local() else {
                 return;
+            };
+
+            // Convert struct `LocalDefId` to an `HirId` so that we can emit the lint for the
+            // correct HIR node.
+            let struct_hir_id: HirId = OwnerId {
+                def_id: struct_local_def_id,
             }
+            .into();
 
-            span_lint_and_then(
+            span_lint_hir_and_then(
                 cx,
                 PLUGIN_NOT_ENDING_IN_PLUGIN.lint,
-                item.span,
+                struct_hir_id,
+                struct_span,
                 PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc,
                 |diag| {
                     diag.span_suggestion(
-                        self_span,
+                        struct_span,
                         "rename the plugin",
-                        format!("{self_name}Plugin"),
+                        format!("{struct_name}Plugin"),
                         // There may be other references that also need to be renamed.
                         Applicability::MaybeIncorrect,
                     );
+
+                    diag.span_note(item.span, "`Plugin` implemented here");
                 },
             );
         }

bevy_lint/tests/ui/main_return_without_appexit/muted.rs

@@ -0,0 +1,15 @@
+//! A version of `main.rs` where the lint is muted on the expression that called `App::run()`. This
+//! should pass without any errors.
+
+//@check-pass
+
+#![feature(register_tool)]
+#![register_tool(bevy)]
+#![deny(bevy::main_return_without_appexit)]
+
+use bevy::prelude::*;
+
+fn main() {
+    #[allow(bevy::main_return_without_appexit)]
+    App::new().run();
+}

bevy_lint/tests/ui/plugin_not_ending_in_plugin/main.rs

@@ -1,14 +1,16 @@
 #![feature(register_tool)]
 #![register_tool(bevy)]
 #![deny(bevy::plugin_not_ending_in_plugin)]
+//~^ NOTE: the lint level is defined here
 
 use bevy::prelude::*;
 
 // This should raise an error, since it does not end in "Plugin".
 struct Foo;
-//~^ HELP: rename the plugin
+//~^ ERROR: implemented `Plugin` for a structure whose name does not end in "Plugin"
+//~| HELP: rename the plugin
 
-//~v ERROR: implemented `Plugin` for a structure whose name does not end in "Plugin"
+//~v NOTE: `Plugin` implemented here
 impl Plugin for Foo {
     fn build(&self, _app: &mut App) {}
 }
@@ -20,11 +22,10 @@ impl Plugin for BarPlugin {
     fn build(&self, _app: &mut App) {}
 }
 
-// Though this does not end in "Plugin", the lint is silenced for the `impl` blog, so no error is
-// raised.
+// Though this does not end in "Plugin", the lint is silenced, so no error is raised.
+#[allow(bevy::plugin_not_ending_in_plugin)]
 struct Baz;
 
-#[allow(bevy::plugin_not_ending_in_plugin)]
 impl Plugin for Baz {
     fn build(&self, _app: &mut App) {}
 }

bevy_lint/tests/ui/plugin_not_ending_in_plugin/main.stderr

@@ -1,14 +1,16 @@
 error: implemented `Plugin` for a structure whose name does not end in "Plugin"
-  --> tests/ui/plugin_not_ending_in_plugin/main.rs:12:1
+  --> tests/ui/plugin_not_ending_in_plugin/main.rs:9:8
    |
-8  |   struct Foo;
-   |          --- help: rename the plugin: `FooPlugin`
-...
-12 | / impl Plugin for Foo {
-13 | |     fn build(&self, _app: &mut App) {}
-14 | | }
-   | |_^
+9  | struct Foo;
+   |        ^^^ help: rename the plugin: `FooPlugin`
+   |
+note: `Plugin` implemented here
+  --> tests/ui/plugin_not_ending_in_plugin/main.rs:14:1
    |
+14 | / impl Plugin for Foo {
+15 | |     fn build(&self, _app: &mut App) {}
+16 | | }
+   | |_^
 note: the lint level is defined here
   --> tests/ui/plugin_not_ending_in_plugin/main.rs:3:9
    |

bevy_lint/tests/ui/plugin_not_ending_in_plugin/spoofed_name.rs

@@ -4,20 +4,22 @@
 #![feature(register_tool)]
 #![register_tool(bevy)]
 #![deny(bevy::plugin_not_ending_in_plugin)]
+//~^ NOTE: the lint level is defined here
 
 use bevy::prelude::*;
 
 mod bar {
     pub mod baz {
         pub struct Foo;
-        //~^ HELP: rename the plugin
+        //~^ ERROR: implemented `Plugin` for a structure whose name does not end in "Plugin"
+        //~| HELP: rename the plugin
     }
 }
 
 // We try to be sneaky, but it doesn't work.
 use self::bar::baz::Foo as FooPlugin;
 
-//~v ERROR: implemented `Plugin` for a structure whose name does not end in "Plugin"
+//~v NOTE: `Plugin` implemented here
 impl Plugin for FooPlugin {
     fn build(&self, _app: &mut App) {}
 }

bevy_lint/tests/ui/plugin_not_ending_in_plugin/spoofed_name.stderr

@@ -1,14 +1,16 @@
 error: implemented `Plugin` for a structure whose name does not end in "Plugin"
-  --> tests/ui/plugin_not_ending_in_plugin/spoofed_name.rs:21:1
+  --> tests/ui/plugin_not_ending_in_plugin/spoofed_name.rs:13:20
    |
-12 |           pub struct Foo;
-   |                      --- help: rename the plugin: `FooPlugin`
-...
-21 | / impl Plugin for FooPlugin {
-22 | |     fn build(&self, _app: &mut App) {}
-23 | | }
-   | |_^
+13 |         pub struct Foo;
+   |                    ^^^ help: rename the plugin: `FooPlugin`
+   |
+note: `Plugin` implemented here
+  --> tests/ui/plugin_not_ending_in_plugin/spoofed_name.rs:23:1
    |
+23 | / impl Plugin for FooPlugin {
+24 | |     fn build(&self, _app: &mut App) {}
+25 | | }
+   | |_^
 note: the lint level is defined here
   --> tests/ui/plugin_not_ending_in_plugin/spoofed_name.rs:6:9
    |