# Bevy CLI Linter

:::warning
**Warning**

This document is outdated. For the latest information, please see the repository at [TheBevyFlock/bevy_cli](https://github.com/TheBevyFlock/bevy_cli).
:::

See [the MVP](/cCHAfbtaSviU_MDnbNHKxg) for motivation behind the Bevy CLI. The prototype repository can be found at [TheBevyFlock/bevy_cli](https://github.com/TheBevyFlock/bevy_cli).

## Goal

The goal is to write a linter, like `cargo check` and [Clippy](https://doc.rust-lang.org/stable/clippy/), that checks projects that use the [Bevy game engine](https://bevyengine.org) for best practices, potential footguns, and style.

The aim is for the MVP of this linter to be released with Bevy 0.15, around November / December of 2024. [BD103](https://github.com/BD103) intends to lead this effort, though others are welcome to join!

## How it works

- Expose the `bevy lint` command in the main CLI.
- Export the `bevy_lint_driver` binary, installed alongside the rest of the CLI.
- When `bevy lint` is called, it will call `RUSTC_WORKSPACE_WRAPPER=path/to/bevy_lint_driver cargo check`.
- This instructs Cargo to pass all arguments to `bevy_lint_driver` instead of the default `rustc`. ([Cargo Environmental Variables])
- `bevy_lint_driver` will use [`rustc_driver::RunCompiler`] to run the compiler with a custom [`Callbacks`] struct.
- The custom callback will set the [`Config::register_lints`] callback with one that registers Bevy-specific lints in the [`LintStore`].
- `rustc` will then call these lints alongside its builtin ones, displaying the results to the caller.

[Cargo Environmental Variables]: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-reads
[`rustc_driver::RunCompiler`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_driver/struct.RunCompiler.html
[`Callbacks`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_driver/trait.Callbacks.html
[`Config::register_lints`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/interface/struct.Config.html#structfield.register_lints

[`LintStore`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/context/struct.LintStore.html

## Philosophy

Lints should be:

- Useful, even if not to everyone
- _Extensively_ documented (both to users and contributors)
- Categorized
- Configurable, but with sensible defaults
- Actionable (with `cargo fix` support)

## Categories

Rust supports organizing lints into categories that can be toggled in bulk. The planned categories introduced by this linter are:

- Correctness (Deny)
- Outright wrong / useless code
- Carefully picked, no false positives
- Suspicious (Warn)
- Strange code that is likely wrong
- Like Correctness, but may include false positives
- Performance (Warn)
- Detects potential performance footguns
- Style (Allow, eventually Warn)
- Checks for unidiomatic code and tries to make it more readable
- This is the most opinionated, which is why it's Allow for now. It will eventually migrate to Warn after multiple rounds of feedback are received.
- Restriction (Allow)
- Restricts certain patterns that a developer may not want

These categories are largely inspired by [Clippy's lint categories](https://doc.rust-lang.org/clippy/lints.html).

## MVP lints

These lints are going to be implemented before the linter is released.

### `main_return_without_appexit`

#### What it does

Checks for `fn main()` entrypoints that call `App::run()` but do not return `AppExit`.

#### Why is it bad?

`AppExit` is used to determine whether the `App` exited successful or due to an error. Returning it from `main()` will set the exit code, which allows external processes to detect whether there was an error.

#### Example

fn main() {
    App::new().run();
}

Use instead:

fn main() -> AppExit {
    // Note the removed semicolon.
    App::new().run()
}

### `plugin_not_ending_in_plugin`

#### What it does

Checks if a struct's name does not end in `Plugin` if it implements `Plugin`.

#### Why is it bad?

It is convention for all plugins to end in `Plugin`, since doing so signals how the struct is meant to be used.

#### Example

struct Rendering;

impl Plugin for Rendering {
    fn build(_: &mut App) {}
}

Use instead:

struct RenderingPlugin;

impl Plugin for RenderingPlugin {
    fn build(_: &mut App) {}
}

### `pre_bevy_0_14`

#### What it does

Checks if the version of Bevy installed is older than 0.14.

#### Why is this bad?

This linter was written for Bevy 0.14 and later, so it may not work on earlier versions.

## Post-MVP Lints

These lints are good ideas that have generally been approved, but should only be worked on once the MVP ones have been implemented.

### `file_format_without_feature`

#### What it does

Checks if an asset format is placed inside the `assets` folder without the associated feature flag being enabled.

For a list of supported file formats, see:

<!-- Make this a table in the final documentation. -->

- [Image formats](https://github.com/bevyengine/bevy/blob/d65eb39277c4f85749ba27460b8398f815ef3802/crates/bevy_internal/Cargo.toml#L32-L46)
- [Audio formats](https://github.com/bevyengine/bevy/blob/d65eb39277c4f85749ba27460b8398f815ef3802/crates/bevy_internal/Cargo.toml#L54-L65)
- [Shader formats](https://github.com/bevyengine/bevy/blob/d65eb39277c4f85749ba27460b8398f815ef3802/crates/bevy_internal/Cargo.toml#L67-L72)

#### Why is this bad?

Bevy will throw an error if it is unable to read a file. It's common for users to set `default-features = false` but forget to enable the features required for their assets.

### `dynamic_linking_enabled_in_manifest`

#### What it does

Checks for Bevy's `dynamic_linking` feature being enabled in `Cargo.toml` by default.

#### Why is this bad?

`dynamic_linking` is not recommended for `--release` builds because it requires distributing the generated `libbevy_dylib` alongside your game and is generally slower. Because Cargo features cannot easily be disabled once turned on, `dynamic_linking` should not be enabled by default.

#### Example

[dependencies]
bevy = { version = "0.14", features = ["dynamic_linking"] }

Use instead:

[dependencies]
bevy = "0.14"

cargo run --features bevy/dynamic_linking

<!-- The Bevy CLI may have an option to make this easier. -->

### `name_stutter`

#### What it does

Checks for components, resources, and events whose names end in `Component`, `Resource`, and `Event`.

#### Why is this bad?

Each of these types usually have a `#[derive(...)]` annotation above them that clearly describes what they are. This is different from plugins and systems, where the trait implementation is either separated from the definition or automatically implemented.

#### Example

#[derive(Component)]
struct PowerComponent(f32);

#[derive(Resource)]
struct ScoreResource(usize);

#[derive(Event)]
struct GameOverEvent;

Use instead:

#[derive(Component)]
struct Power(f32);

#[derive(Resource)]
struct Score(usize);

#[derive(Event)]
struct GameOver;

## Resources

- [Errors and Lints](https://rustc-dev-guide.rust-lang.org/diagnostics.html)
- [`LintStore`](https://rustc-dev-guide.rust-lang.org/diagnostics/lintstore.html)
- [Clippy development](https://doc.rust-lang.org/stable/clippy/development/index.html)
- [Common tools for writing lints](https://doc.rust-lang.org/stable/clippy/development/common_tools_writing_lints.html)
- [Write Rust lints without forking Clippy](https://blog.trailofbits.com/2021/11/09/write-rust-lints-without-forking-clippy/)

## Lint ideas

Feel free to add your lint ideas below! All are considered, though they may be rejected for one reason or another.

- [ ] [OS dependencies not installed](https://bevyengine.org/learn/quick-start/getting-started/setup/#installing-os-dependencies)
- [x] Features that need to be enabled / disabled for certain patterns
- File formats (images, sound)
- [x] `dynamic_linking` enabled by default in `Cargo.toml`
- This is considered an anti-pattern in release mode.
- [ ] Disable trace level logging.
- [x] Stutter in components, resources, events, systems
- A component named `NameComponent` is unnecessarily redundant, `Name` is much better.
- [ ] Bad queries
- `Query<&Foo, With<Foo>>` needlessly filters for `Foo`
- Forgetting to query a reference: `Query<Foo>` will fail, try `Query<&Foo>`
- [ ] Unregistered types (events, resources, reflected)
- [ ] System ordering ambiguity
- [ ] Logging without registering `LogPlugin`
- [ ] `impl Plugin` vs. `fn plugin(&mut App)`
- Only when `impl Plugin` just implements `build()`
- Prefer `fn plugin()` when `pub(super)`, maybe not when `pub`
- [x] Returning `AppExit` from `fn main()`
- [ ] Missing `#[derive(Reflect)]`
- [x] Plugin names should end in `Plugin`
- [ ] ~~Every file should contain a plugin~~
- [ ] Systems sets should end in `System` or `Set`
- [ ] Not adding `#[reflect(Foo)]` when it is derived
- [ ] Calling panicking `single()` instead of `get()`
- [ ] Using `before()` and `after()` in places where `chain()` could be used instead
- [ ] Duplicate Bevy dependencies
- [ ] unneeded `ResMut` (not actually used as mutable)
- [ ] unneeded `&mut Component`
- [ ] unneeded `Ref<Component>`
- [ ] `Res<Events<T>>` (trap because of double-reading events)
- [ ] `.init_resource` on `Event` resource (won't set up cleanup systems)
- [ ] manual construction of `SystemState` in an exclusive system (use `&mut SystemState` system param)
- [ ] manual `Mut::set_if_neq` implementation

[Initial discussion](https://discord.com/channels/691052431525675048/1277626501197463576/1278058521933381714), [opinionated list by Jan](https://discord.com/channels/691052431525675048/1277626501197463576/1278063696224059525)