Add a Bevy Linter

[MinerSebas]

Objective

This adds a Linter for Bevy Code.

Solution

Uses dylint to run Lints.
To use this (once it is released) a User needs to add this to their Cargo.toml:

[workspace.metadata.dylint]
libraries = [
    { git = "https://github.com./bevyengine/bevy", tag = "v0.6.0", pattern = "crates/bevy_lint" },
]

And then run these Commands:

cargo install cargo-dylint dylint-link
cargo dylint bevy_lint

Currently, it only checks if you have any Function that accepts Querys that contain an unnecessary With Filter.

error: Unnecessary `With` Filter
  --> $DIR/unnecessary_with.rs:10:34
   |
LL | fn test_query1(_query: Query<&A, With<A>>) {}
   |                                  ^^^^^^^
   |
   = note: `-D unnecessary-with` implied by `-D warnings

(Copied from UI-Test File)

Testing

If you want to test this right now, then the Cargo.toml should look like this.

[workspace.metadata.dylint]
libraries = [
   { git = "https://github.com./MinerSebas/bevy", branch= "Linter", pattern = "crates/bevy_lint" },
]

If you are on Windows and use MSVC, you will need to build the executables yourself, until trailofbits/dylint#43 and trailofbits/dylint#45 are merged and released:

git clone https://github.com./MinerSebas/dylint
cd dylint
git checkout linker
cargo install --path cargo-dylint
cargo install --path dylint-link

If you use the GNU Toolchain on Windows, you are out of luck.
For some reason, the GNU Linker does not want to link the Lint Library successfully.
(More Information here trailofbits/dylint#45 (comment))

[alice-i-cecile]

Partial solution to #1602.

[DJMcNab]

Obviously, that specific lint should probably also be added as a bevy internal check

We could even steal cargo style reporting for that, although obviously our ability to select spans is a more limited.

Once we get editor integration in some form, we'd also want to wire up the warnings there

[alice-i-cecile]

I'm excited for the possibilities here. We can add more lints once this is merged IMO.

I'd like to see a proper README for this new crate; mostly containing the information from this PR description but also linking or containing to some advice on how to add new lints.

[MinerSebas]

Obviously, that specific lint should probably also be added as a bevy internal check

Already done: https://github.com./MinerSebas/bevy/blob/Linter/.github/workflows/ci.yml#L94-L95

We could even steal cargo style reporting for that, although obviously our ability to select spans is a more limited.

Once we get editor integration in some form, we'd also want to wire up the warnings there

What do you mean with cargo style?

As for editor Integration, rust-analyzer can also show these Lints (VSCode Setting Example taken from here):

    "rust-analyzer.checkOnSave.overrideCommand": [
        "cargo",
        "dylint",
        "--all",
        "--workspace",
        "--",
        "--message-format=json"
    ]

[MinerSebas]

Also what name should this crate have?

• bevy_dylint
• bevy_clippy
• Other suggestions?

[alice-i-cecile]

IMO bevy_lint is best. It's clear, simple and doesn't tie us to dylint in the future if we ever choose to swap.

[alice-i-cecile]

If you're looking for some more easy lints:

1. Trivially empty queries where you have both &Foo and Without<Foo>, or With<Foo> and Without<Foo>.
2. &Entity query parameters. It'll fail to compile once [Merged by Bors] - Implement and require #[derive(Component)] on all component structs #2254 lands, but this will help users pinpoint the error.
3. Inserting resources with their default or from_world values (use init_resource instead).

[MinerSebas]

1. Trivially empty queries where you have both `&Foo` and `Without<Foo>`, or `With<Foo>` and `Without<Foo>`.

Already in my Personal List. Others in the Query category include useless Or: Or<()>, Or<With<Foo>>, Or<(With<Foo>, Without<Foo>> and not quering Data Query<()>

2. `&Entity` query parameters. It'll fail to compile once [Implement and require `#[derive(Component)]` on all component structs #2254](https://github.com./bevyengine/bevy/pull/2254) lands, but this will help users pinpoint the error.

I thought about it, but i dont think i should invest time into it, because it will no longer be posible.
It will also not help pinpoint the problem (once the derive is added), as the compilation will fail before the Linter is run.

3. Inserting resources with their default or from_world values (use init_resource instead).

👍

[cart]

I'm not sure I want to commit the bevy org to maintaining custom lints at this point:

1. This still requires non-standard setup (installing a custom cargo command, adding custom things to cargo.toml, avoiding msvc on windows, etc). Imo its not something we should be recommending right out of the gate as "the recommended / official development setup".
2. we'd be committing to fixing every lint every time we break something (which still happens regularly)
3. its one more class of pr that we need to review and merge (and we're already swamped). what constitutes a "good lint" or a "bad lint" will be a constant debate. Its worth having those conversations, but they take up time and energy that I don't really have right now.

This feels like a perfect candidate for "third party maintenance", at least while bevy apis stabilize and the lints are still being actively developed and experimented with. Its an easy way to parallelize / decentralize decision making and would almost certainly allow bevy_lint to mature faster.

Ultimately I think it makes sense for us to manage this stuff, but I'm not convinced now is the time to do it. My current position is that we should reserve the bevy_lint crate name (which cant be published with the current impl anyway due to git dependencies), let @MinerSebas manage the project themselves (in their own repo) while the lints develop and dylint stabilizes, then revisit pulling it into the "bevy org" umbrella once we're ready for it.

Curious what everyone else thinks.

[alice-i-cecile]

I'd be happy to have it third-party maintained for now, but used on Bevy itself (it's a useful tool!) + prominently promoted.

Reducing the Cart bottleneck for it does sound appealing.

[MinerSebas]

A reasonable stance to take.
I moved the code to a separate Repo: https://github.com./MinerSebas/bevy_lint