From 79c8adebe11151e4ff7bf13a9a5fa1429776db6e Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Wed, 25 Sep 2024 17:28:35 -0400 Subject: [PATCH 01/11] feat: setup infrastructure --- bevy_lint/src/lints/mod.rs | 3 +++ .../src/lints/plugin_not_ending_in_plugin.rs | 15 +++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 bevy_lint/src/lints/plugin_not_ending_in_plugin.rs diff --git a/bevy_lint/src/lints/mod.rs b/bevy_lint/src/lints/mod.rs index d400c46a..b7c0a1a8 100644 --- a/bevy_lint/src/lints/mod.rs +++ b/bevy_lint/src/lints/mod.rs @@ -3,10 +3,12 @@ use rustc_lint::{Lint, LintStore}; pub mod insert_event_resource; pub mod main_return_without_appexit; +pub mod plugin_not_ending_in_plugin; pub(crate) static LINTS: &[&BevyLint] = &[ insert_event_resource::INSERT_EVENT_RESOURCE, main_return_without_appexit::MAIN_RETURN_WITHOUT_APPEXIT, + plugin_not_ending_in_plugin::PLUGIN_NOT_ENDING_IN_PLUGIN, ]; pub(crate) fn register_lints(store: &mut LintStore) { @@ -17,4 +19,5 @@ pub(crate) fn register_lints(store: &mut LintStore) { pub(crate) fn register_passes(store: &mut LintStore) { store.register_late_pass(|_| Box::new(insert_event_resource::InsertEventResource)); store.register_late_pass(|_| Box::new(main_return_without_appexit::MainReturnWithoutAppExit)); + store.register_late_pass(|_| Box::new(plugin_not_ending_in_plugin::PluginNotEndingInPlugin)); } diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs new file mode 100644 index 00000000..c3001c14 --- /dev/null +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -0,0 +1,15 @@ +use crate::declare_bevy_lint; +use rustc_lint::LateLintPass; +use rustc_session::declare_lint_pass; + +declare_bevy_lint! { + pub PLUGIN_NOT_ENDING_IN_PLUGIN, + STYLE, + "implemented `Plugin` for a structure whose name does end in \"Plugin\"", +} + +declare_lint_pass! { + PluginNotEndingInPlugin => [PLUGIN_NOT_ENDING_IN_PLUGIN.lint] +} + +impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin {} From 32bc6c45ec5e5be804d18db893a4bc75aba70a03 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Wed, 25 Sep 2024 23:05:52 -0400 Subject: [PATCH 02/11] feat: first working iteration --- .../src/lints/plugin_not_ending_in_plugin.rs | 34 +++++++++++++++++-- bevy_lint/src/paths.rs | 1 + 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index c3001c14..1eb1a847 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -1,15 +1,43 @@ +//! TODO + use crate::declare_bevy_lint; -use rustc_lint::LateLintPass; +use clippy_utils::{diagnostics::span_lint, match_def_path}; +use rustc_hir::{def::Res, Item, ItemKind, QPath, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; declare_bevy_lint! { pub PLUGIN_NOT_ENDING_IN_PLUGIN, STYLE, - "implemented `Plugin` for a structure whose name does end in \"Plugin\"", + "implemented `Plugin` for a structure whose name does not end in \"Plugin\"", } declare_lint_pass! { PluginNotEndingInPlugin => [PLUGIN_NOT_ENDING_IN_PLUGIN.lint] } -impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin {} +impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { + if let ItemKind::Impl(impl_) = item.kind + && let Some(of_trait) = impl_.of_trait + && let Res::Def(_, def_id) = of_trait.path.res + && match_def_path(cx, def_id, &crate::paths::PLUGIN) + && let TyKind::Path(QPath::Resolved(_, self_path)) = impl_.self_ty.kind + { + let Some(self_name) = self_path.segments.last() else { + return; + }; + + if self_name.ident.as_str().ends_with("Plugin") { + return; + } + + span_lint( + cx, + PLUGIN_NOT_ENDING_IN_PLUGIN.lint, + self_path.span, + PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc, + ); + } + } +} diff --git a/bevy_lint/src/paths.rs b/bevy_lint/src/paths.rs index c36a21f7..e8309d76 100644 --- a/bevy_lint/src/paths.rs +++ b/bevy_lint/src/paths.rs @@ -8,3 +8,4 @@ pub const APP: [&str; 3] = ["bevy_app", "app", "App"]; pub const EVENTS: [&str; 3] = ["bevy_ecs", "event", "Events"]; +pub const PLUGIN: [&str; 3] = ["bevy_app", "plugin", "Plugin"]; From e2876afe5ac3abb1a7a55ff28bd8dbf16b392ba4 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:35:52 -0400 Subject: [PATCH 03/11] feat: add further comments --- .../src/lints/plugin_not_ending_in_plugin.rs | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index 1eb1a847..f3b4f090 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -2,7 +2,7 @@ use crate::declare_bevy_lint; use clippy_utils::{diagnostics::span_lint, match_def_path}; -use rustc_hir::{def::Res, Item, ItemKind, QPath, TyKind}; +use rustc_hir::{def::Res, Item, ItemKind, Path, QPath, Ty, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -18,16 +18,27 @@ declare_lint_pass! { impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { + // Find `impl T` items... if let ItemKind::Impl(impl_) = item.kind + // ...that implement a trait for `T`... && let Some(of_trait) = impl_.of_trait + // ...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 + // ...where the trait being implemented is Bevy's `Plugin`... && match_def_path(cx, def_id, &crate::paths::PLUGIN) - && let TyKind::Path(QPath::Resolved(_, self_path)) = impl_.self_ty.kind + // ...and the type the trait is being implemented for has a user-defined name. See + // `extract_path_from_hir_ty`'s documentation for more info on what this does. + && let Some(self_path) = extract_path_from_hir_ty(impl_.self_ty) { + // Find the last segment of the path, such as `Foo` for `bar::baz::Foo`. This is + // considered the name of the type. let Some(self_name) = self_path.segments.last() else { return; }; + // If the name ends with "Plugin", do not emit a lint. if self_name.ident.as_str().ends_with("Plugin") { return; } @@ -41,3 +52,27 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { } } } + +/// A best-effort utilitiy that tries to extract the [`Path`] of an HIR [`Ty`] if the name can +/// easily be changed by the user. +/// +/// Kinds of types that are extracted are paths (`module::submodule::Type`) and references to paths +/// (`&module::Type` or `*const Type`). Types that are not extracted, and just return [`None`], +/// include slices (`[T]`), trait objects (`dyn Trait`), tuples (`(A, B, ...)`), and more. +fn extract_path_from_hir_ty<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Path<'tcx>> { + match hir_ty.kind { + // The type is a path, such as `module::Type`. + TyKind::Path(qpath) => match qpath { + // If the qualified path points to a resolved piece of code, return that path. + QPath::Resolved(_, path) => Some(path), + // The alternatives are lang items (which cannot be renamed without `#![no_core]`) and + // relative paths (`::AssociatedType`), which also cannot be renamed easily. + _ => None, + }, + // If the type is a reference or pointer, recursively check the inner type. For instance, + // `*const module::Type` would return `module::Type`, while `&[usize; 10]` would return + // `None` because `[usize; 10]` returns `None`. + TyKind::Ref(_, mut_ty) | TyKind::Ptr(mut_ty) => extract_path_from_hir_ty(mut_ty.ty), + _ => None, + } +} From 1d587af747571f2bc068542a31fc4c0b309ea49b Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:46:28 -0400 Subject: [PATCH 04/11] feat: do not lint on references and pointers You cannot implement `Plugin` for `*const T` since they are always foreign, and it's pretty-much pointless to implement `Plugin` for `&T` since `Plugin: 'static`. --- .../src/lints/plugin_not_ending_in_plugin.rs | 33 ++++--------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index f3b4f090..def7326e 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -28,9 +28,12 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { && let Res::Def(_, def_id) = of_trait.path.res // ...where the trait being implemented is Bevy's `Plugin`... && match_def_path(cx, def_id, &crate::paths::PLUGIN) - // ...and the type the trait is being implemented for has a user-defined name. See - // `extract_path_from_hir_ty`'s documentation for more info on what this does. - && let Some(self_path) = extract_path_from_hir_ty(impl_.self_ty) + // ...and the type `Plugin` is being implemented for is a path to a user-defined type. + // This purposefully evaluates as false for references, since implementing `Plugin` for + // them is useless due to `Plugin`'s `'static` requirement. The other kinds of types, + // such as lang items and primitives, are also skipped because they cannot be easily + // renamed. + && let TyKind::Path(QPath::Resolved(_, self_path)) = impl_.self_ty.kind { // Find the last segment of the path, such as `Foo` for `bar::baz::Foo`. This is // considered the name of the type. @@ -52,27 +55,3 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { } } } - -/// A best-effort utilitiy that tries to extract the [`Path`] of an HIR [`Ty`] if the name can -/// easily be changed by the user. -/// -/// Kinds of types that are extracted are paths (`module::submodule::Type`) and references to paths -/// (`&module::Type` or `*const Type`). Types that are not extracted, and just return [`None`], -/// include slices (`[T]`), trait objects (`dyn Trait`), tuples (`(A, B, ...)`), and more. -fn extract_path_from_hir_ty<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Path<'tcx>> { - match hir_ty.kind { - // The type is a path, such as `module::Type`. - TyKind::Path(qpath) => match qpath { - // If the qualified path points to a resolved piece of code, return that path. - QPath::Resolved(_, path) => Some(path), - // The alternatives are lang items (which cannot be renamed without `#![no_core]`) and - // relative paths (`::AssociatedType`), which also cannot be renamed easily. - _ => None, - }, - // If the type is a reference or pointer, recursively check the inner type. For instance, - // `*const module::Type` would return `module::Type`, while `&[usize; 10]` would return - // `None` because `[usize; 10]` returns `None`. - TyKind::Ref(_, mut_ty) | TyKind::Ptr(mut_ty) => extract_path_from_hir_ty(mut_ty.ty), - _ => None, - } -} From d86fe38cd3aa8738709e03694792f47d6c3d1ce1 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:46:26 -0400 Subject: [PATCH 05/11] feat: add suggestion in diagnostic --- .../src/lints/plugin_not_ending_in_plugin.rs | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index def7326e..2b3a93d6 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -1,8 +1,11 @@ //! TODO use crate::declare_bevy_lint; -use clippy_utils::{diagnostics::span_lint, match_def_path}; -use rustc_hir::{def::Res, Item, ItemKind, Path, QPath, Ty, TyKind}; +use clippy_utils::{ + diagnostics::span_lint_and_then, match_def_path, path_res, source::snippet_opt, +}; +use rustc_errors::Applicability; +use rustc_hir::{def::Res, Item, ItemKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -46,11 +49,29 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { return; } - span_lint( + // Resolve the `DefId` of our HIR `Ty`, then lookup the symbol and span if it exists. + // We use this to add an optional suggestion for renaming the type. + let source_definition = path_res(cx, impl_.self_ty) + .opt_def_id() + .and_then(|did| cx.tcx.opt_item_ident(did)); + + span_lint_and_then( cx, PLUGIN_NOT_ENDING_IN_PLUGIN.lint, self_path.span, PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc, + |diag| { + if let Some(source_definition) = source_definition + && let Some(name) = snippet_opt(cx, source_definition.span) + { + diag.span_suggestion( + source_definition.span, + "rename the plugin", + format!("{name}Plugin"), + Applicability::MaybeIncorrect, + ); + } + }, ); } } From d2c9d513f4a8ee7ee2c861ae38305a9721004e25 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 20:19:02 -0400 Subject: [PATCH 06/11] feat: begin lint documentation --- bevy_lint/src/lints/plugin_not_ending_in_plugin.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index 2b3a93d6..f67dabf2 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -1,4 +1,14 @@ -//! TODO +//! Checks for types who implement `Plugin` whose names do not end in "Plugin". +//! +//! This does _not_ check function-style plugins, only structures with `Plugin` explicitly +//! implemented with `impl Plugin for T`. +//! +//! # Motivation +//! +//! It is common practice to suffix all plugin names with "Plugin", since doing so signals the +//! primary purpose of the type. As compared to traits like [`Clone`] and `Serialize`, the primary +//! purpose of a type that implements `Plugin` is to be a Bevy plugin, which is why a distinction +//! is made in the name. use crate::declare_bevy_lint; use clippy_utils::{ From 639cb463e3ac64ee22f2ba959909d1ab97a357a7 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 20:26:37 -0400 Subject: [PATCH 07/11] feat: finish documentation --- .../src/lints/plugin_not_ending_in_plugin.rs | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index f67dabf2..a12cb95f 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -1,14 +1,41 @@ -//! Checks for types who implement `Plugin` whose names do not end in "Plugin". +//! Checks for types who implement `Plugin` but whose names does not end in "Plugin". //! -//! This does _not_ check function-style plugins, only structures with `Plugin` explicitly -//! implemented with `impl Plugin for T`. +//! This does _not_ check function-style plugins (`fn plugin(app: &mut App)`), only structures with +//! `Plugin` explicitly implemented with `impl Plugin for T`. //! //! # Motivation //! -//! It is common practice to suffix all plugin names with "Plugin", since doing so signals the -//! primary purpose of the type. As compared to traits like [`Clone`] and `Serialize`, the primary -//! purpose of a type that implements `Plugin` is to be a Bevy plugin, which is why a distinction -//! is made in the name. +//! Unlike traits like [`Clone`] or [`Debug`], the primary purpose of a type that implements +//! `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. +//! +//! # Example +//! +//! ``` +//! # use bevy::prelude::*; +//! # +//! struct Physics; +//! +//! impl Plugin for Physics { +//! fn build(&self, app: &mut App) { +//! // ... +//! } +//! } +//! ``` +//! +//! Use instead: +//! +//! ``` +//! # use bevy::prelude::*; +//! # +//! struct PhysicsPlugin; +//! +//! impl Plugin for PhysicsPlugin { +//! fn build(&self, app: &mut App) { +//! // ... +//! } +//! } +//! ``` use crate::declare_bevy_lint; use clippy_utils::{ From da93d8af0f9f1f6fe73210a0618d8a406e2de596 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 21:30:54 -0400 Subject: [PATCH 08/11] fix: lint can be tricked with `use Foo as FooPlugin` --- .../src/lints/plugin_not_ending_in_plugin.rs | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index a12cb95f..30292ec2 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -38,13 +38,12 @@ //! ``` use crate::declare_bevy_lint; -use clippy_utils::{ - diagnostics::span_lint_and_then, match_def_path, path_res, source::snippet_opt, -}; +use clippy_utils::{diagnostics::span_lint_and_then, match_def_path, path_res}; use rustc_errors::Applicability; -use rustc_hir::{def::Res, Item, ItemKind, QPath, TyKind}; +use rustc_hir::{def::Res, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; +use rustc_span::symbol::Ident; declare_bevy_lint! { pub PLUGIN_NOT_ENDING_IN_PLUGIN, @@ -58,9 +57,9 @@ declare_lint_pass! { impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { - // Find `impl T` items... + // Find `impl` items... if let ItemKind::Impl(impl_) = item.kind - // ...that implement a trait for `T`... + // ...that implement a trait... && let Some(of_trait) = impl_.of_trait // ...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 @@ -68,46 +67,40 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { && let Res::Def(_, def_id) = of_trait.path.res // ...where the trait being implemented is Bevy's `Plugin`... && match_def_path(cx, def_id, &crate::paths::PLUGIN) - // ...and the type `Plugin` is being implemented for is a path to a user-defined type. - // This purposefully evaluates as false for references, since implementing `Plugin` for - // them is useless due to `Plugin`'s `'static` requirement. The other kinds of types, - // such as lang items and primitives, are also skipped because they cannot be easily - // renamed. - && let TyKind::Path(QPath::Resolved(_, self_path)) = impl_.self_ty.kind { - // Find the last segment of the path, such as `Foo` for `bar::baz::Foo`. This is - // considered the name of the type. - let Some(self_name) = self_path.segments.last() else { + // Try to resolve the original definition of this type, finding its original name and + // span. (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, + }) = path_res(cx, impl_.self_ty) + .opt_def_id() + .and_then(|def_id| cx.tcx.opt_item_ident(def_id)) + else { return; }; - // If the name ends with "Plugin", do not emit a lint. - if self_name.ident.as_str().ends_with("Plugin") { + // If the type's name ends in "Plugin", exit. + if self_name.as_str().ends_with("Plugin") { return; } - // Resolve the `DefId` of our HIR `Ty`, then lookup the symbol and span if it exists. - // We use this to add an optional suggestion for renaming the type. - let source_definition = path_res(cx, impl_.self_ty) - .opt_def_id() - .and_then(|did| cx.tcx.opt_item_ident(did)); - span_lint_and_then( cx, PLUGIN_NOT_ENDING_IN_PLUGIN.lint, - self_path.span, + self_span, PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc, |diag| { - if let Some(source_definition) = source_definition - && let Some(name) = snippet_opt(cx, source_definition.span) - { - diag.span_suggestion( - source_definition.span, - "rename the plugin", - format!("{name}Plugin"), - Applicability::MaybeIncorrect, - ); - } + diag.span_suggestion( + self_span, + "rename the plugin", + format!("{self_name}Plugin"), + // There may be other references that also need to be renamed. + Applicability::MaybeIncorrect, + ); + + diag.span_help(item.span, "`Plugin` implemented here"); }, ); } From ebb5079d886cc2bfe204f67c08c8b7924a19add0 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 26 Sep 2024 21:43:21 -0400 Subject: [PATCH 09/11] feat: add section on known issues --- bevy_lint/src/lints/plugin_not_ending_in_plugin.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index 30292ec2..b91ceb5f 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -9,6 +9,12 @@ //! `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 //! //! ``` From 20ac53a0304b3695c12d4c08c317d4e2dca7256b Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Fri, 27 Sep 2024 08:57:07 -0400 Subject: [PATCH 10/11] fix: do not lint on generic parameters --- .../src/lints/plugin_not_ending_in_plugin.rs | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index b91ceb5f..fbdb2974 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -74,15 +74,30 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { // ...where the trait being implemented is Bevy's `Plugin`... && match_def_path(cx, def_id, &crate::paths::PLUGIN) { - // Try to resolve the original definition of this type, finding its original name and - // span. (We don't use the name from the path, since that can be spoofed through - // `use Foo as FooPlugin`.) + // Try to resolve where this type was originally defined. This will result in a `DefId` + // pointing to the original `struct Foo` definition, or `impl ` if it's a generic + // parameter. + let Some(def_id) = path_res(cx, impl_.self_ty).opt_def_id() else { + return; + }; + + // If this type is a generic parameter, exit. Their names, such as `T`, cannot be + // referenced by others. + if impl_ + .generics + .params + .iter() + .any(|param| param.def_id.to_def_id() == 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, - }) = path_res(cx, impl_.self_ty) - .opt_def_id() - .and_then(|def_id| cx.tcx.opt_item_ident(def_id)) + }) = cx.tcx.opt_item_ident(def_id) else { return; }; From a15c994b4ce1a95fffdfb480404498dd02c38e92 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Mon, 30 Sep 2024 22:26:54 -0400 Subject: [PATCH 11/11] feat: clarify what is being linted This makes it more clear that the `impl` item is linted, not the original structure definition. --- bevy_lint/src/lints/plugin_not_ending_in_plugin.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs index fbdb2974..254438bd 100644 --- a/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -110,7 +110,7 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { span_lint_and_then( cx, PLUGIN_NOT_ENDING_IN_PLUGIN.lint, - self_span, + item.span, PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc, |diag| { diag.span_suggestion( @@ -120,8 +120,6 @@ impl<'tcx> LateLintPass<'tcx> for PluginNotEndingInPlugin { // There may be other references that also need to be renamed. Applicability::MaybeIncorrect, ); - - diag.span_help(item.span, "`Plugin` implemented here"); }, ); }