Add duplicate bevy dependencies lint

[DaAlbrecht]

A first draft of a quick lint to check if multiple versions of bevy are defined. #15

todos:

• create ui test
• better error message, perhaps use cargo metadata to get information about the crate versions.
• fix the span ( this should point to the crate causing the issue in the Cargo.toml)

error: Multiple versions of `bevy` found
  --> /Users/didi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy-0.13.2/src/lib.rs:1:1
   |
1  | / #![allow(clippy::single_component_path_imports)]
2  | |
3  | | //! [![](https://bevyengine.org/assets/bevy_logo_d...
4  | | //!
...  |
51 | | #[allow(unused_imports)]
52 | | use bevy_dylib;
   | |_______________^
   |
   = note: `#[deny(bevy::duplicate_bevy_dependencies)]` on by default

[BD103]

So far this looks good! I am interested in getting a span to Cargo.toml / Cargo.lock, though I'm unsure if the compiler will know that file exists. Ping me when this is ready for a final review!

[DaAlbrecht]

example of the lint message

error: Mismatching versions of `bevy` found
  --> Cargo.toml:10:26
   |
10 | leafwing-input-manager = "0.13"
   |                          ^^^^^^
   |
help: Expected all crates to use `bevy` 0.14.2
  --> Cargo.toml:8:8
   |
8  | bevy = { version = "0.14.2", features = ["android_shared_stdcxx"] }
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[BD103]

One function you may find useful is was_invoked_from_cargo(), which detects whether we're being run from Cargo or not. We can use this to skip Cargo lints when bevy_lint_driver is called directly!

[DaAlbrecht]

One function you may find useful is was_invoked_from_cargo(), which detects whether we're being run from Cargo or not. We can use this to skip Cargo lints when bevy_lint_driver is called directly!

nice catch! Thanks will add this.

[TimJentzsch]

@DaAlbrecht The tests are failing, because Bevy needs additional system packages to be installed on Linux.

I had the same issue in #222, so we can either merge that PR first and then merge main back in your branch or you can copy the CI-related changes here to fix this :)

[BD103]

Overall, great stuff! I have quite a few minor nitpicks, but they aren't that important. The biggest changes I want are:

• Spawning MetadataCommand in the directory of the input file
• When cargo metadata fails, emit a plain error diagnostic instead of using span_lint()
• Splitting up UI tests and Cargo UI tests into two different files.
• Switching duplicate_bevy_dependencies to the nursery lint group for now.

Let me know if you need any further clarification on stuff! I can do some of the work as well, if you're busy :)

[BD103]

Is there a reason you specify bevy = { version = "0.14.2" } instead of bevy = "0.14.2"? Should we test both formats, or was this just leftover from testing?

[DaAlbrecht]

This is intentional to check that not only bevy = "0.14.2" works. But we should definitely check both variants. I would like to add more tests in general for this but was a bit scared it would blow up CI time.

But I will add some more tomorrow testing all the different variations.

[BD103]

Could you split the Cargo UI tests into a different file, ui_cargo.rs? There's not enough overlap in my opinion to require these two be in the same file. (Not to mention, having two different "test files" means you can select which you want to run with cargo test --test ui or cargo test --test ui_cargo.)

[DaAlbrecht]

Hmm, I can do this but wouldn't this just end in a new test_utils module that contains more or less the entire code from ui.rs (base_config, find_bevy_rlib, ArtifactMessage, ArtifactTarget) as these are used in both?

[DaAlbrecht]

Being able to only run a subset is pretty nice i implemented this once how i think this could look like

[DaAlbrecht]

Overall, great stuff! I have quite a few minor nitpicks, but they aren't that important. The biggest changes I want are:

• Spawning MetadataCommand in the directory of the input file
• When cargo metadata fails, emit a plain error diagnostic instead of using span_lint()
• Splitting up UI tests and Cargo UI tests into two different files.
• Switching duplicate_bevy_dependencies to the nursery lint group for now.

Let me know if you need any further clarification on stuff! I can do some of the work as well, if you're busy :)

Thank you so much for the review!

[DaAlbrecht]

Do you have a better idea of how we can test without needing to create so many packages? Also, let me know if you rather revert to fab055a

[BD103]

Do you have a better idea of how we can test without needing to create so many packages? Also, let me know if you rather revert to fab055a

I don't mind creating that many packages, as it's what many other popular crates use to test their Cargo integration. I'm more concerned with the increase CI time, though thankfully the cache should handle that.

I'm going to merge this as-is, as I'm happy with the initial changes, then open a few follow-up PRs to do additional cleanup. :)

[BD103]

Ugh I broke the Git history again (I really need to stop doing this). I'll open a new PR and merge that ;-;