Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create lint groups #98

Merged
merged 5 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bevy_lint/src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ impl Callbacks for BevyLintCallback {
(previous)(session, store);
}

store.register_lints(crate::lints::LINTS);
crate::lints::register_lints(store);
crate::lints::register_passes(store);
}));
}
Expand Down
64 changes: 64 additions & 0 deletions bevy_lint/src/groups.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use crate::LintGroup;
use rustc_lint::Level;

/// A group of deny-by-default lints that check for outright wrong or useless code.
///
/// These lints are carefully picked to be free of false positives. You should avoid
/// `#[allow(...)]`-ing these lints without a _very_ good reason.
pub static CORRECTNESS: LintGroup = LintGroup {
name: "bevy::correctness",
level: Level::Deny,
};

/// A group similar to [`CORRECTNESS`] that checks for suspicious or usually wrong code.
///
/// The linted code may have been written intentionally, but should probably still be fixed.
pub static SUSPICIOUS: LintGroup = LintGroup {
name: "bevy::suspicious",
level: Level::Warn,
};

/// A group that offers suggestions on how to simplify your code.
pub static COMPLEXITY: LintGroup = LintGroup {
name: "bevy::complexity",
level: Level::Warn,
};

/// A group that suggests how to increase the performance of your code.
pub static PERFORMANCE: LintGroup = LintGroup {
name: "bevy::performance",
level: Level::Warn,
};

/// A group of lints that encourage idiomatic code.
///
/// These lints are opinionated and may be freely disabled if you disagree with their suggestions.
pub static STYLE: LintGroup = LintGroup {
name: "bevy::style",
level: Level::Warn,
};

/// A group of lints that make the linter incredibly nit-picky.
///
/// If you enable this group, expect to liberally apply `#[allow(...)]` attributes throughout your
/// code.
pub static PEDANTIC: LintGroup = LintGroup {
name: "bevy::pedantic",
level: Level::Allow,
};

/// A group of opt-in lints that restrict you from writing certain code.
///
/// These are designed for scenarios where you want to increase the consistency of your code-base
/// and reject certain patterns. They should not all be enabled at once, but instead specific lints
/// should be individually enabled.
Comment on lines +52 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the obvious question is, "if they shouldn't all be enabled at once, why are they in a group"? Or are we saying, this group is for categorising but not for actual use 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want every lint to be within a group so that it's clear what its general purpose is. These lint groups are just as important for documentation as they are for bulk-toggling.

So yes, this group is a category that's never intended to be named directly. (Clippy has the same things!)

pub static RESTRICTION: LintGroup = LintGroup {
name: "bevy::restriction",
level: Level::Allow,
};

/// A group of unstable lints that may be removed at any time for any reason.
pub static NURSERY: LintGroup = LintGroup {
name: "bevy::nursery",
level: Level::Allow,
};
7 changes: 6 additions & 1 deletion bevy_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ extern crate rustc_session;
extern crate rustc_span;

mod callback;
pub mod groups;
mod lint;
pub mod lints;
mod paths;

pub use self::callback::BevyLintCallback;
pub use self::{
callback::BevyLintCallback,
lint::{BevyLint, LintGroup},
};
47 changes: 47 additions & 0 deletions bevy_lint/src/lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use rustc_lint::{Level, Lint};

/// A Bevy lint definition and its associated group.
///
/// The level of the lint must be the same as the level of the group.
pub struct BevyLint {
pub lint: &'static Lint,
pub group: &'static LintGroup,
}

/// Represents a lint group.
pub struct LintGroup {
/// The name of the lint group.
///
/// This will be used when trying to enable / disable the group, such as through
/// `#![allow(group)]`. By convention, this should start with `bevy::`.
pub name: &'static str,

// The default level all lints within this group should be.
pub level: Level,
}

#[macro_export]
macro_rules! declare_bevy_lint {
{
$(#[$attr:meta])*
$vis:vis $name:ident,
$group:ident,
$desc:expr$(,)?
} => {
$(#[$attr])*
$vis static $name: &$crate::lint::BevyLint = &$crate::lint::BevyLint {
lint: &::rustc_lint::Lint {
name: concat!("bevy::", stringify!($name)),
default_level: $crate::groups::$group.level,
desc: $desc,
edition_lint_opts: None,
report_in_external_macro: false,
future_incompatible: None,
is_externally_loaded: true,
feature_gate: None,
crate_level_only: false,
},
group: &$crate::groups::$group,
};
};
}
17 changes: 9 additions & 8 deletions bevy_lint/src/lints/insert_event_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
//! App::new().add_event::<MyEvent>().run();
//! ```

use crate::declare_bevy_lint;
use clippy_utils::{
diagnostics::span_lint_and_sugg, source::snippet_with_applicability, sym, ty::match_type,
};
Expand All @@ -43,18 +44,18 @@ use rustc_hir::{Expr, ExprKind, GenericArg, GenericArgs, Path, PathSegment, QPat
use rustc_hir_analysis::lower_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{Ty, TyKind};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::declare_lint_pass;
use rustc_span::Span;
use std::borrow::Cow;

declare_tool_lint! {
pub bevy::INSERT_EVENT_RESOURCE,
Deny,
"called `App::insert_resource(Events<T>)` or `App::init_resource::<Events<T>>()` instead of `App::add_event::<T>()`"
declare_bevy_lint! {
pub INSERT_EVENT_RESOURCE,
SUSPICIOUS,
"called `App::insert_resource(Events<T>)` or `App::init_resource::<Events<T>>()` instead of `App::add_event::<T>()`",
}

declare_lint_pass! {
InsertEventResource => [INSERT_EVENT_RESOURCE]
InsertEventResource => [INSERT_EVENT_RESOURCE.lint]
}

impl<'tcx> LateLintPass<'tcx> for InsertEventResource {
Expand Down Expand Up @@ -103,7 +104,7 @@ fn check_insert_resource(cx: &LateContext<'_>, args: &[Expr], method_span: Span)

span_lint_and_sugg(
cx,
INSERT_EVENT_RESOURCE,
INSERT_EVENT_RESOURCE.lint,
method_span,
"called `App::insert_resource(Events<T>)` instead of `App::add_event::<T>()`",
"inserting an `Events` resource does not fully setup that event",
Expand Down Expand Up @@ -165,7 +166,7 @@ fn check_init_resource<'tcx>(cx: &LateContext<'tcx>, path: &PathSegment<'tcx>, m

span_lint_and_sugg(
cx,
INSERT_EVENT_RESOURCE,
INSERT_EVENT_RESOURCE.lint,
method_span,
"called `App::init_resource::<Events<T>>()` instead of `App::add_event::<T>()`",
"inserting an `Events` resource does not fully setup that event",
Expand Down
17 changes: 9 additions & 8 deletions bevy_lint/src/lints/main_return_without_appexit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
//! }
//! ```

use crate::declare_bevy_lint;
use clippy_utils::{
diagnostics::span_lint_and_then, is_entrypoint_fn, sym, ty::match_type, visitors::for_each_expr,
};
Expand All @@ -38,18 +39,18 @@ use rustc_hir::{
def_id::LocalDefId, intravisit::FnKind, Body, ExprKind, FnDecl, FnRetTy, Ty, TyKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::declare_lint_pass;
use rustc_span::Span;
use std::ops::ControlFlow;

declare_tool_lint! {
pub bevy::MAIN_RETURN_WITHOUT_APPEXIT,
Warn,
"an entrypoint that calls `App::run()` does not return `AppExit`"
declare_bevy_lint! {
pub MAIN_RETURN_WITHOUT_APPEXIT,
PEDANTIC,
"an entrypoint that calls `App::run()` does not return `AppExit`",
}

declare_lint_pass! {
MainReturnWithoutAppExit => [MAIN_RETURN_WITHOUT_APPEXIT]
MainReturnWithoutAppExit => [MAIN_RETURN_WITHOUT_APPEXIT.lint]
}

impl<'tcx> LateLintPass<'tcx> for MainReturnWithoutAppExit {
Expand Down Expand Up @@ -89,9 +90,9 @@ impl<'tcx> LateLintPass<'tcx> for MainReturnWithoutAppExit {
if match_type(cx, ty, &crate::paths::APP) {
span_lint_and_then(
cx,
MAIN_RETURN_WITHOUT_APPEXIT,
MAIN_RETURN_WITHOUT_APPEXIT.lint,
method_span,
MAIN_RETURN_WITHOUT_APPEXIT.desc,
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 {
Expand Down
8 changes: 7 additions & 1 deletion bevy_lint/src/lints/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
use crate::BevyLint;
use rustc_lint::{Lint, LintStore};

pub mod insert_event_resource;
pub mod main_return_without_appexit;

pub static LINTS: &[&Lint] = &[
pub static LINTS: &[&BevyLint] = &[
insert_event_resource::INSERT_EVENT_RESOURCE,
main_return_without_appexit::MAIN_RETURN_WITHOUT_APPEXIT,
];

pub(crate) fn register_lints(store: &mut LintStore) {
let lints: Vec<&Lint> = LINTS.iter().map(|x| x.lint).collect();
store.register_lints(&lints);
}

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));
Expand Down