Improve linter UI test runners (and fix them on Windows!)

.gitattributes

@@ -0,0 +1,3 @@
+# From: https://docs.github.com/en/github/getting-started-with-github/configuring-git-to-handle-line-endings
+# Set the default behavior, in case people don't have core.autocrlf set.
+* text=auto

.github/workflows/ci.yml

@@ -48,11 +48,12 @@ jobs:
           save-if: ${{ github.ref == 'refs/heads/main' }}
 
       - name: Run tests
-        run: |
-          cargo test --workspace --all-features --all-targets
-          # Workaround for https://github.com/rust-lang/cargo/issues/6669. `--doc` is incompatible
-          # with `--all-targets`, so we run them separately.
-          cargo test --workspace --all-features --doc
+        run: cargo test --workspace --all-features --all-targets
+
+      # Workaround for https://github.com/rust-lang/cargo/issues/6669. `--doc` is incompatible
+      # with `--all-targets`, so we run them separately.
+      - name: Run doc tests
+        run: cargo test --workspace --all-features --doc
 
   clippy:
     name: Check with Clippy

Cargo.toml

@@ -1,5 +1,11 @@
 [workspace]
-members = ["bevy_lint"]
+members = [
+    # The Bevy Linter
+    "bevy_lint",
+
+    # A dummy crate used to build dependencies that the linter needs for its UI tests.
+    "bevy_lint/tests/dependencies",
+]
 
 [workspace.lints.clippy]
 # Prefer `str.to_owned()`, rather than `str.to_string()`.

bevy_lint/Cargo.toml

@@ -53,19 +53,16 @@ toml = { version = "0.9.8", default-features = false, features = [
 ] }
 
 [dev-dependencies]
-# Used when running UI tests.
+# Ensures the error messages for lints do not regress.
+ui_test = "0.30.1"
+
+# Used in doc tests.
 bevy = { version = "0.17.2", default-features = false, features = [
     "std",
     # used for the `camera_modification_in_fixed_update` lint
     "bevy_render",
 ] }
 
-# Used to deserialize `--message-format=json` messages from Cargo.
-serde_json = "1.0.145"
-
-# Ensures the error messages for lints do not regress.
-ui_test = "0.30.1"
-
 [lints]
 workspace = true
 

bevy_lint/tests/dependencies/Cargo.toml

@@ -0,0 +1,19 @@
+[package]
+name = "lint-ui-dependencies"
+description = "A dummy crate that builds dependencies that `bevy_lint`'s UI tests need to import"
+edition = "2024"
+publish = false
+
+[dependencies]
+bevy = { version = "0.17.2", default-features = false, features = [
+    "std",
+    # used for the `camera_modification_in_fixed_update` lint
+    "bevy_render",
+] }
+
+# Some of Bevy's macros try to intelligently detect whether to import `bevy::ecs` or `bevy_ecs`.
+# This works in normal circumstances, but our dependency setup is too complex for the macros to
+# understand, so they default to the `bevy_ecs` form. This makes `bevy_ecs` and `bevy_reflect`
+# directly importable, and matches the crate version to whatever version `bevy` is pulling in.
+bevy_ecs = "*"
+bevy_reflect = "*"

bevy_lint/tests/dependencies/src/lib.rs

@@ -0,0 +1 @@
+//! This file is intentionally blank. :)

bevy_lint/tests/test_utils/mod.rs

@@ -1,137 +1,18 @@
-use std::{
-    ffi::OsStr,
-    path::{Path, PathBuf},
-    process::{Command, Stdio},
-};
+use std::process::{Command, Stdio};
 
-use serde::Deserialize;
-use ui_test::{
-    CommandBuilder, Config,
-    color_eyre::{self, eyre::ensure},
-};
-
-// This is set by Cargo to the absolute path of `bevy_lint_driver`.
-const DRIVER_PATH: &str = env!("CARGO_BIN_EXE_bevy_lint_driver");
-
-/// Generates a custom [`Config`] for `bevy_lint`'s UI tests.
-pub fn base_config(test_dir: &str) -> color_eyre::Result<Config> {
-    let driver_path = Path::new(DRIVER_PATH);
-
-    ensure!(
-        driver_path.is_file(),
-        "`bevy_lint_driver` could not be found at {}, make sure to build it with `cargo build -p bevy_lint --bin bevy_lint_driver`",
-        driver_path.display(),
-    );
-
-    let config = Config {
-        // When `host` is `None`, `ui_test` will attempt to auto-discover the host by calling
-        // `program -vV`. Unfortunately, `bevy_lint_driver` does not yet support the version flag,
-        // so we manually specify the host as an empty string. This means that, for now, host-
-        // specific configuration in UI tests will not work.
-        host: Some(String::new()),
-        program: CommandBuilder {
-            // We don't need `rustup run` here because we're already using the correct toolchain
-            // due to `rust-toolchain.toml`.
-            program: driver_path.into(),
-            args: vec![
-                // `bevy_lint_driver` expects the first argument to be the path to `rustc`.
-                "rustc".into(),
-                // This is required so that `ui_test` can parse warnings and errors.
-                "--error-format=json".into(),
-                // These two lines tell `rustc` to search in `target/debug/deps` for dependencies.
-                // This is required for UI tests to import `bevy`.
-                "-L".into(),
-                "all=../target/debug/deps".into(),
-                // Make the `bevy` crate directly importable from the UI tests.
-                format!("--extern=bevy={}", find_bevy_rlib()?.display()).into(),
-            ],
-            out_dir_flag: Some("--out-dir".into()),
-            input_file_flag: None,
-            envs: Vec::new(),
-            cfg_flag: Some("--print=cfg".into()),
-        },
-        out_dir: PathBuf::from("../target/ui"),
-        ..Config::rustc(Path::new("tests").join(test_dir))
-    };
-
-    Ok(config)
-}
-
-/// An artifact message printed to stdout by Cargo.
-///
-/// This only deserializes the fields necessary to run UI tests, the rest of skipped.
-///
-/// See <https://doc.rust-lang.org/cargo/reference/external-tools.html#artifact-messages> for more
-/// information on the exact format.
-#[derive(Deserialize, Debug)]
-#[serde(rename = "compiler-artifact", tag = "reason")]
-struct ArtifactMessage<'a> {
-    #[serde(borrow)]
-    target: ArtifactTarget<'a>,
-
-    #[serde(borrow)]
-    filenames: Vec<&'a Path>,
-}
-
-/// The `"target"` field of an [`ArtifactMessage`].
-#[derive(Deserialize, Debug)]
-struct ArtifactTarget<'a> {
-    name: &'a str,
-
-    #[serde(borrow)]
-    kind: Vec<&'a str>,
-}
-
-/// Tries to find the path to `libbevy.rlib` that UI tests import.
-///
-/// `bevy` is a dev-dependency, and as such is only built for tests and examples. We can force it
-/// to be built by calling `cargo build --test=ui --message-format=json`, then scan the printed
-/// JSON for the artifact message with the path to `libbevy.rlib`.
-///
-/// The reason we specify `--extern bevy=PATH` instead of just `--extern bevy` is because `rustc`
-/// will fail to compile if multiple `libbevy.rlib` files are found, which usually is the case.
-fn find_bevy_rlib() -> color_eyre::Result<PathBuf> {
-    // `bevy` is a dev-dependency, so building a test will require it to be built as well.
-    let output = Command::new("cargo")
-        .arg("build")
-        .arg("--test=ui")
-        .arg("--message-format=json")
-        // Show error messages to the user for easier debugging.
+/// Queries the host tuple from `rustc` and returns it as a string.
+pub fn host_tuple() -> String {
+    let output = Command::new("rustc")
+        .arg("--print=host-tuple")
+        // Show errors directly to the user, rather than capturing them.
         .stderr(Stdio::inherit())
-        .output()?;
-
-    ensure!(output.status.success(), "`cargo build --test=ui` failed.");
-
-    // It's theoretically possible for there to be multiple messages about building `libbevy.rlib`.
-    // We support this, but optimize for just 1 message.
-    let mut messages = Vec::with_capacity(1);
-
-    // Convert the `stdout` to a string, replacing invalid characters with `�`.
-    let stdout = String::from_utf8_lossy(&output.stdout);
-
-    // Iterate over each line in stdout, trying to deserialize it from JSON.
-    for line in stdout.lines() {
-        if let Ok(message) = serde_json::from_str::<ArtifactMessage>(line)
-            // If the message passes the following conditions, it's probably the one we want.
-            && message.target.name == "bevy"
-            && message.target.kind.contains(&"lib")
-        {
-            messages.push(message);
-        }
-    }
-
-    ensure!(
-        messages.len() == 1,
-        "More than one `libbevy.rlib` was built for UI tests. Please ensure there is not more than 1 version of Bevy in `Cargo.lock`.",
-    );
-
-    // The message usually has multiple files, often `libbevy.rlib` and `libbevy.rmeta`. Filter
-    // through these to find the `rlib`.
-    let rlib = messages[0]
-        .filenames
-        .iter()
-        .find(|p| p.extension() == Some(OsStr::new("rlib")))
-        .expect("`libbevy.rlib` not found within artifact message filenames.");
-
-    Ok(rlib.to_path_buf())
+        .output()
+        .expect("failed to run `rustc --print=host-tuple`");
+
+    // `rustc` only works with UTF-8, so it's safe to error if invalid UTF-8 is found.
+    str::from_utf8(&output.stdout)
+        .expect("`rustc --print=host-tuple` did not emit valid UTF-8")
+        // Remove the trailing `\n`.
+        .trim_end()
+        .to_owned()
 }

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.stderr

@@ -1,13 +1,13 @@
 error: multiple versions of the `bevy` crate found
-  --> Cargo.toml:12:26
+  --> Cargo.toml:14:26
    |
-12 | leafwing-input-manager = "0.13"
+14 | leafwing-input-manager = "0.13"
    |                          ^^^^^^
    |
 help: expected all crates to use `bevy` 0.17.2, but `leafwing-input-manager` uses `bevy` ^0.13
-  --> Cargo.toml:11:8
+  --> Cargo.toml:13:8
    |
-11 | bevy = { version = "0.17.2", default-features = false }
+13 | bevy = { version = "0.17.2", default-features = false }
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 note: the lint level is defined here
   --> src/main.rs:3:9

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.toml

@@ -1,3 +1,5 @@
+#@exit-status: 101
+
 [package]
 name = "multiple-bevy-versions"
 publish = false

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/pass/Cargo.toml

@@ -1,3 +1,5 @@
+#@check-pass
+
 [package]
 name = "single-bevy-version"
 publish = false

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.stderr

@@ -1,13 +1,13 @@
 error: multiple versions of the `bevy` crate found
-  --> Cargo.toml:12:26
+  --> Cargo.toml:14:26
    |
-12 | leafwing-input-manager = "0.13"
+14 | leafwing-input-manager = "0.13"
    |                          ^^^^^^
    |
 help: expected all crates to use `bevy` 0.17.2, but `leafwing-input-manager` uses `bevy` ^0.13
-  --> Cargo.toml:11:8
+  --> Cargo.toml:13:8
    |
-11 | bevy = "0.17.2"
+13 | bevy = "0.17.2"
    |        ^^^^^^^^
 note: the lint level is defined here
   --> src/main.rs:3:9

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.toml

@@ -1,3 +1,5 @@
+#@exit-status: 101
+
 [package]
 name = "multiple-bevy-versions"
 publish = false

bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/pass/Cargo.toml

@@ -1,3 +1,5 @@
+#@check-pass
+
 [package]
 name = "single-bevy-version"
 publish = false

bevy_lint/tests/ui.rs

@@ -1,13 +1,62 @@
-// A convenience feature used in `find_bevy_rlib()` that lets you chain multiple `if let`
-// statements together with `&&`. This feature flag is needed in all integration tests that use the
-// test_utils module, since each integration test is compiled independently.
-
-use test_utils::base_config;
-use ui_test::run_tests;
+#![expect(
+    clippy::disallowed_macros,
+    reason = "`bevy_lint`'s macros are intended for lints, not tests"
+)]
 
 mod test_utils;
 
+use std::path::{Path, PathBuf};
+
+use ui_test::{CommandBuilder, Config, dependencies::DependencyBuilder, run_tests};
+
+// This is set by Cargo to the absolute path of `bevy_lint_driver`.
+const DRIVER_PATH: &str = env!("CARGO_BIN_EXE_bevy_lint_driver");
+
 fn main() {
-    let config = base_config("ui").unwrap();
+    let driver_path = Path::new(DRIVER_PATH);
+
+    assert!(
+        driver_path.is_file(),
+        "`bevy_lint_driver` could not be found at {}, make sure to build it with `cargo build -p bevy_lint --bin bevy_lint_driver`",
+        driver_path.display(),
+    );
+
+    let mut config = Config {
+        // We need to specify the host tuple manually, because if we don't then `ui_test` will try
+        // running `bevy_lint_driver -vV` to discover the host and promptly error because it
+        // doesn't realize `bevy_lint_driver` expects its first argument to be the path to `rustc`.
+        // If `ui_test` ran `bevy_lint_driver rustc -vV` everything would work, but it's not smart
+        // enough to do that.
+        host: Some(test_utils::host_tuple()),
+        program: CommandBuilder {
+            // We don't need `rustup run` here because we're already using the correct toolchain
+            // due to `rust-toolchain.toml`.
+            program: driver_path.into(),
+            args: vec![
+                // `bevy_lint_driver` expects the first argument to be the path to `rustc`.
+                "rustc".into(),
+                // This is required so that `ui_test` can parse warnings and errors.
+                "--error-format=json".into(),
+            ],
+            out_dir_flag: Some("--out-dir".into()),
+            input_file_flag: None,
+            envs: Vec::new(),
+            cfg_flag: Some("--print=cfg".into()),
+        },
+        out_dir: PathBuf::from("../target/ui"),
+        ..Config::rustc(Path::new("tests/ui"))
+    };
+
+    // Give UI tests access to all crate dependencies in the `dependencies` folder. This lets UI
+    // tests import `bevy`.
+    let revisioned = config.comment_defaults.base();
+    revisioned.set_custom(
+        "dependencies",
+        DependencyBuilder {
+            crate_manifest_path: PathBuf::from("tests/dependencies/Cargo.toml"),
+            ..Default::default()
+        },
+    );
+
     run_tests(config).unwrap();
 }