Skip to content

Bevy linter #1602

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

Open
alice-i-cecile opened this issue Mar 9, 2021 · 6 comments
Open

Bevy linter #1602

alice-i-cecile opened this issue Mar 9, 2021 · 6 comments
Labels
C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

A linter could be quite helpful to catch and help correct obvious mistakes in ways the compiler can't, or warn about bad practices in Bevy code.

What solution would you like?

A command line tool is introduced to lint Bevy apps. As some initial ideas, this could catch:

What alternative(s) have you considered?

Some of this can be implemented either in compiler errors or run-time errors / warnings. The former has serious technical limitations (see #1519), and the latter is noisy and hard to configure.

Clippy doesn't appear to support custom lints, but it would be quite nice to integrate with it if it did.

Additional context

This is not super high priority, but serves as a nice place to track features that we could put in it. This would be part of #436, but should be able to stand alone.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Mar 9, 2021
@jakobhellermann
Copy link
Contributor

As suggested in the discord, this can also be done as a procedural macro.
I started implementing such a macro in https://github.com./jakobhellermann/bevycheck.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented May 20, 2021

Dylint is a new crate that dynamically loads custom lints and then runs them with a cargo command. This seems like the perfect tool for us to use here.

@alice-i-cecile
Copy link
Member Author

Decrees for bevy engine dev:

I like sorting by "commonness" and "type". Standard derives like Debug / Clone first, custom/most descriptive last (ex: StageLabel). Related derives like Copy/Clone, Eq/PartialEq should live together

@alice-i-cecile
Copy link
Member Author

An existing project for defining custom lints in Rust that could use help getting off the ground: https://github.com./rust-linting

@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Mar 21, 2022
@alice-i-cecile
Copy link
Member Author

A dylint-based prototype has been made by @MinerSebas: https://github.com./MinerSebas/bevy_lint

@alice-i-cecile
Copy link
Member Author

I've written up a few of our use cases for this in the new rust-linting repository as they're looking for user stories!

  1. User story: redundant query parameters in Bevy rust-marker/design#18
  2. User story: invalid system parameter in Bevy system rust-marker/design#19
  3. User story: manual implementation of semi-sealed trait rust-marker/design#20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
None yet
Development

No branches or pull requests

2 participants