Skip to content

bevy_lint --fix #505

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

Merged
merged 8 commits into from
Jul 7, 2025
Merged

bevy_lint --fix #505

merged 8 commits into from
Jul 7, 2025

Conversation

BD103
Copy link
Member

@BD103 BD103 commented Jul 3, 2025

This PR adds support for automatically fixing any machine-applicable suggestions bevy_lint emits with the --fix flag. In the process, it also implements CLI argument parsing and a custom --help screen.

How does --fix work?

It's surprisingly simple! Cargo supports automatically fixing builtin lints by running cargo fix. We can make cargo fix work with bevy_lint by setting RUSTC_WORKSPACE_WRAPPER=bevy_lint_driver, which is the same thing we do for cargo check.

So the end result is that, when --fix is passed, we run cargo fix instead of cargo check. Nothing else changes :)

Why pico_args?

The biggest thing I wanted to avoid was clap. It's a wonderful crate, but using it could significantly hurt bevy_lint's fast compile times. It's overkill for the simple stuff that the linter needs!

When looking for an alternative, I consulted blessed.rs and found pico_args. It's a small, zero-dependency library for quick and simple CLI parsing, and was exactly what I was looking for!

pico_args is essentially stable, with no additional features planned. The author appears trustworthy, having contributed to quite a few open-source Rust projects in the past, although they look to currently be inactive. Given how pico_args is unlikely to change in the future, this shouldn't be a problem. Should we run into an issue, we can switch to another library (like clap if need be).

Why a custom help screen?

Before, bevy_lint --help just printed cargo check --help. cargo check doesn't support --fix, however, so we need to make our own help screen instead.

The approach I used to make the help screen is simple and fast, but somewhat rigid. Instead of computing the help screen dynamically, like clap, we just embed a string literal with the contents. I also use anstyle to make the help screen colorful, and anstream so strip out the ANSI color codes for terminals and streams that don't support them:

image

The main benefits to this approach is that it:

  • Is super fast to compile and run (the string is one giant constant)
  • Is easy to edit
  • Gives us full control over the contents

There are a few drawbacks, however. This approach:

  • Can lead to consistencies over time as many people modify the --help text
  • Is difficult to read in the source code with all the {LITERAL} template blocks

I think the pros outweigh the cons here. My bet is that the --help screen will be rarely edited, so we won't run into the issues as much as other projects might. (For example, I would never do this for the Bevy CLI. Its CLI interface changes far too much!)

Testing

First, install this branch's version of the linter:

cargo install --git https://github.com/TheBevyFlock/bevy_cli.git --branch bevy-lint-fix --locked bevy_lint

Next, setup a new project with the follow in main.rs:

#![feature(register_tool)]
#![register_tool(bevy)]

use bevy::prelude::*;

fn main() {
    App::new().add_systems(Startup, spawn_empty);
}

fn spawn_empty(mut commands: Commands) {
    // This will raise a warning and machine-applicable suggestion.
    commands.spawn(());
}

commands.spawn(()) will cause the bevy::insert_unit_bundle lint to be upset. You can verify it by running bevy_lint:

warning: inserted a `Bundle` containing a unit `()` type
  --> bevy_lint/examples/lint_test.rs:11:20
   |
11 |     commands.spawn(());
   |              ------^^- help: try: `spawn_empty()`
   |
   = note: unit `()` types are skipped instead of spawned
   = note: `#[warn(bevy::insert_unit_bundle)]` on by default

Now, fix the lint by running bevy_lint --fix. commands.spawn(()) should now be replaced with commands.spawn_empty()!

fn spawn_empty(mut commands: Commands) {
    commands.spawn_empty();
}

BD103 added 6 commits July 2, 2025 20:54
We don't use this, as all of our flags are booleans, not options with values. If that were to ever change in the future, I'd re-enable this feature.
@BD103 BD103 added A-Linter Related to the linter and custom lints C-Usability An improvement that makes the API more pleasant D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review The PR needs to be reviewed before it can be merged X-Contentious There are nontrivial implications that should be thought through labels Jul 3, 2025
@BD103 BD103 linked an issue Jul 3, 2025 that may be closed by this pull request
@BD103 BD103 added this to the `bevy_lint` v0.4.0 milestone Jul 3, 2025
@BD103 BD103 added C-Feature Make something new possible and removed C-Usability An improvement that makes the API more pleasant labels Jul 3, 2025
Copy link
Collaborator

@DaAlbrecht DaAlbrecht left a comment

Choose a reason for hiding this comment

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

Nice, this is such a big win!

// `anstream` automatically removes ANSI escape codes if the terminal does not support it. This
// text is formatted to look like `bevy -h`, `cargo clippy --help`, and `cargo check -h`. Keep
// the printed text no wider than 80 characters, so it fits on most terminals without wrapping.
anstream::println!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

While this looks kinda scary ^^ I think it will be completely fine. We probably won't need to edit it often.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, that's my hope. If it becomes unbearable, we'll switch to clap and call it a day!

@BD103 BD103 enabled auto-merge (squash) July 7, 2025 13:41
@BD103 BD103 merged commit 3357cd9 into main Jul 7, 2025
10 checks passed
@BD103 BD103 deleted the bevy-lint-fix branch July 7, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Related to the linter and custom lints C-Feature Make something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review The PR needs to be reviewed before it can be merged X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bevy_lint --fix
2 participants