From 80c81b6c9cda23e72138f0e4bd3d6b878046b070 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:56:28 -0400 Subject: [PATCH] Add lint: Plugin not ending in "Plugin" (#111) Closes #55. This adds a lint that verifies that all plugins have the "Plugin" suffix, as that pattern is pretty-much standardized across the ecosystem. This is our first "Style" lint, meaning it tries to suggest idiomatic code. ## Testing Paste the following into `bevy_lint/examples/lint_test.rs`, run `cd bevy_lint`, then run `cargo build && cargo run -- --example lint_test`: ```rust #![feature(register_tool)] #![register_tool(bevy)] use bevy::prelude::*; mod module { pub mod submodule { use std::marker::PhantomData; // Verify generics are not included in name. pub struct Foo(PhantomData); } } // Ensure you cannot spoof the name of the plugin. use module::submodule::Foo as FooPlugin; impl Plugin for FooPlugin { fn build(&self, _: &mut App) {} } fn main() {} ``` --- bevy_lint/src/lints/mod.rs | 3 + .../src/lints/plugin_not_ending_in_plugin.rs | 127 ++++++++++++++++++ bevy_lint/src/paths.rs | 1 + 3 files changed, 131 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 574d0d6..dd9f134 100644 --- a/bevy_lint/src/lints/mod.rs +++ b/bevy_lint/src/lints/mod.rs @@ -4,12 +4,14 @@ use rustc_lint::{Lint, LintStore}; pub mod insert_event_resource; pub mod main_return_without_appexit; pub mod panicking_methods; +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, panicking_methods::PANICKING_QUERY_METHODS, panicking_methods::PANICKING_WORLD_METHODS, + plugin_not_ending_in_plugin::PLUGIN_NOT_ENDING_IN_PLUGIN, ]; pub(crate) fn register_lints(store: &mut LintStore) { @@ -21,4 +23,5 @@ 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(panicking_methods::PanickingMethods)); + 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 0000000..254438b --- /dev/null +++ b/bevy_lint/src/lints/plugin_not_ending_in_plugin.rs @@ -0,0 +1,127 @@ +//! Checks for types who implement `Plugin` but whose names does not end in "Plugin". +//! +//! This does _not_ check function-style plugins (`fn plugin(app: &mut App)`), only structures with +//! `Plugin` explicitly implemented with `impl Plugin for T`. +//! +//! # Motivation +//! +//! 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. +//! +//! # 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 +//! +//! ``` +//! # 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::{diagnostics::span_lint_and_then, match_def_path, path_res}; +use rustc_errors::Applicability; +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, + STYLE, + "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 { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { + // Find `impl` items... + if let ItemKind::Impl(impl_) = item.kind + // ...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 + // 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) + { + // 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, + }) = cx.tcx.opt_item_ident(def_id) + else { + return; + }; + + // If the type's name ends in "Plugin", exit. + if self_name.as_str().ends_with("Plugin") { + return; + } + + span_lint_and_then( + cx, + PLUGIN_NOT_ENDING_IN_PLUGIN.lint, + item.span, + PLUGIN_NOT_ENDING_IN_PLUGIN.lint.desc, + |diag| { + 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, + ); + }, + ); + } + } +} diff --git a/bevy_lint/src/paths.rs b/bevy_lint/src/paths.rs index 26961c7..ddba707 100644 --- a/bevy_lint/src/paths.rs +++ b/bevy_lint/src/paths.rs @@ -8,6 +8,7 @@ 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"]; pub const QUERY: [&str; 4] = ["bevy_ecs", "system", "query", "Query"]; pub const QUERY_STATE: [&str; 4] = ["bevy_ecs", "query", "state", "QueryState"]; pub const WORLD: [&str; 3] = ["bevy_ecs", "world", "World"];