Bevy CLI Linter

Warning

This document is outdated. For the latest information, please see the repository at TheBevyFlock/bevy_cli.

See the MVP for motivation behind the Bevy CLI. The prototype repository can be found at TheBevyFlock/bevy_cli.

Goal

The goal is to write a linter, like cargo check and Clippy, that checks projects that use the Bevy game engine 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 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.

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.

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:

• Image formats
• Audio formats
• Shader formats

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

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
• LintStore
• Clippy development
• Common tools for writing lints
• 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
• Features that need to be enabled / disabled for certain patterns
  • File formats (images, sound)
• dynamic_linking enabled by default in Cargo.toml
  • This is considered an anti-pattern in release mode.
• Disable trace level logging.
• 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
• Returning AppExit from fn main()
• Missing #[derive(Reflect)]
• 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, opinionated list by Jan