diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..3a6f7a0c --- /dev/null +++ b/.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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb02da7a..7190ab9a 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/Cargo.lock b/Cargo.lock index 28ac2ba9..62a65749 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -824,7 +824,6 @@ dependencies = [ "clippy_utils", "pico-args", "serde", - "serde_json", "toml", "ui_test", ] @@ -2696,6 +2695,15 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f9fbbcab51052fe104eb5e5d351cf728d30a5be1fe14d9be8a3b097481fb97de" +[[package]] +name = "lint-ui-dependencies" +version = "0.0.0" +dependencies = [ + "bevy", + "bevy_ecs", + "bevy_reflect", +] + [[package]] name = "linux-raw-sys" version = "0.11.0" diff --git a/Cargo.toml b/Cargo.toml index 0f129660..2700eb53 100644 --- a/Cargo.toml +++ b/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()`. diff --git a/bevy_lint/Cargo.toml b/bevy_lint/Cargo.toml index 47a82e51..5493aab1 100644 --- a/bevy_lint/Cargo.toml +++ b/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 diff --git a/bevy_lint/tests/dependencies/Cargo.toml b/bevy_lint/tests/dependencies/Cargo.toml new file mode 100644 index 00000000..b4808069 --- /dev/null +++ b/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 = "*" diff --git a/bevy_lint/tests/dependencies/src/lib.rs b/bevy_lint/tests/dependencies/src/lib.rs new file mode 100644 index 00000000..4653cff5 --- /dev/null +++ b/bevy_lint/tests/dependencies/src/lib.rs @@ -0,0 +1 @@ +//! This file is intentionally blank. :) diff --git a/bevy_lint/tests/test_utils/mod.rs b/bevy_lint/tests/test_utils/mod.rs index e7048d9a..d96c1b55 100644 --- a/bevy_lint/tests/test_utils/mod.rs +++ b/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 { - 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 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 { - // `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::(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() } diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.stderr b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.stderr index e9ee6af4..a48e266c 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.stderr +++ b/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 diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.toml b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.toml index 99ce600b..217fa32a 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/fail/Cargo.toml +++ b/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 diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/pass/Cargo.toml b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/pass/Cargo.toml index 8e3bd433..cd84e590 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/long_version_format/pass/Cargo.toml +++ b/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 diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.stderr b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.stderr index fadaf6c5..dd3b96a8 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.stderr +++ b/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 diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.toml b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.toml index 8509627f..6d8324d3 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/fail/Cargo.toml +++ b/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 diff --git a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/pass/Cargo.toml b/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/pass/Cargo.toml index ef9d53bb..accd764f 100644 --- a/bevy_lint/tests/ui-cargo/duplicate_bevy_dependencies/short_version_format/pass/Cargo.toml +++ b/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 diff --git a/bevy_lint/tests/ui.rs b/bevy_lint/tests/ui.rs index 5bf5a25f..f949bace 100644 --- a/bevy_lint/tests/ui.rs +++ b/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(); } diff --git a/bevy_lint/tests/ui_cargo.rs b/bevy_lint/tests/ui_cargo.rs index 82620203..41161084 100644 --- a/bevy_lint/tests/ui_cargo.rs +++ b/bevy_lint/tests/ui_cargo.rs @@ -1,54 +1,80 @@ -// 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. +#![expect( + clippy::disallowed_macros, + reason = "`bevy_lint`'s macros are intended for lints, not tests" +)] -use std::env; +mod test_utils; -use test_utils::base_config; -use ui_test::{CommandBuilder, status_emitter}; +use std::{ + env, + path::{Path, PathBuf}, +}; + +use ui_test::{ + Args, CommandBuilder, Config, Format, OptWithLine, + status_emitter::{self, StatusEmitter}, +}; + +// This is set by Cargo to the absolute paths of `bevy_lint` and `bevy_lint_driver`. +const LINTER_PATH: &str = env!("CARGO_BIN_EXE_bevy_lint"); -mod test_utils; -/// This [`Config`] will run the `bevy_lint` command for all paths that end in `Cargo.toml` -/// # Example: -/// ```sh -/// bevy_lint" "--quiet" "--target-dir" -/// "../target/ui/0/tests/ui-cargo/duplicate_bevy_dependencies/fail" "--manifest-path" -/// "tests/ui-cargo/duplicate_bevy_dependencies/fail/Cargo.toml"``` fn main() { - let mut config = base_config("ui-cargo").unwrap(); + let linter_path = Path::new(LINTER_PATH); + + assert!( + linter_path.is_file(), + "`bevy_lint` could not be found at {}, make sure to build it with `cargo build -p bevy_lint --bin bevy_lint`", + linter_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 -vV` to discover the host and promptly error because `bevy_lint` + // doesn't recognize the `-vV` flag. + host: Some(test_utils::host_tuple()), + program: CommandBuilder { + program: linter_path.into(), + args: vec!["--color=never".into(), "--quiet".into()], + out_dir_flag: Some("--target-dir".into()), + input_file_flag: Some("--manifest-path".into()), + envs: Vec::new(), + cfg_flag: None, + }, + out_dir: PathBuf::from("../target/ui"), + ..Config::cargo(Path::new("tests/ui-cargo")) + }; - let defaults = config.comment_defaults.base(); - // The driver returns a '101' on error. - // This allows for any status code to be considered a success. - defaults.exit_status = None.into(); + // We haven't found a way to get error annotations like `#~v ERROR: msg` to work, so we disable + // the requirement for them. + config.comment_defaults.base().require_annotations = None.into(); - defaults.require_annotations = None.into(); + // Create the `#@exit-status: CODE` annotation. This can be used to ensure a UI test exits with + // a specific exit code (e.g. `bevy_lint` exits with code 101 when a denied lint is found). + config + .custom_comments + .insert("exit-status", |parser, args, _span| { + parser.exit_status = OptWithLine::new( + args.content + .parse() + .expect("expected `i32` as input for `exit-status`"), + args.span, + ); + }); - // This sets the '--manifest-path' flag - config.program.input_file_flag = CommandBuilder::cargo().input_file_flag; - config.program.out_dir_flag = CommandBuilder::cargo().out_dir_flag; - // Do not print cargo log messages - config.program.args = vec!["--quiet".into(), "--color".into(), "never".into()]; + let args = Args::test().unwrap(); - let current_exe_path = env::current_exe().unwrap(); - let deps_path = current_exe_path.parent().unwrap(); - let profile_path = deps_path.parent().unwrap(); + if let Format::Pretty = args.format { + println!( + "Compiler: {}", + config.program.display().to_string().replace('\\', "/") + ); + } - // Specify the binary to use when executing tests with this `Config` - config.program.program = profile_path.join(if cfg!(windows) { - "bevy_lint_driver.exe" - } else { - "bevy_lint_driver" - }); + let name = config.root_dir.display().to_string().replace('\\', "/"); - config.program.program.set_file_name(if cfg!(windows) { - "bevy_lint.exe" - } else { - "bevy_lint" - }); + let emitter: Box = args.format.into(); - // this clears the default `--edition` flag - config.comment_defaults.base().custom.clear(); + config.with_args(&args); // Run this `Config` for all paths that end with `Cargo.toml` resulting // only in the `Cargo` lints. @@ -59,7 +85,7 @@ fn main() { .then(|| ui_test::default_any_file_filter(path, config)) }, |_config, _file_contents| {}, - status_emitter::Text::verbose(), + (emitter, status_emitter::Gha { name, group: true }), ) .unwrap(); }