From bf684354f5c8287d6d4a0ced58e70d40e06ce807 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Wed, 13 Dec 2023 20:46:21 -0800 Subject: [PATCH] Inform users that `crate` and `srcs` are mutually exclusive (#1650) This should avoid confusion in cases where users have defined tests with both `rust_test.crate` and `rust_test.srcs` set and later find `rust_test.srcs` are actually unused and not being tested. relates to https://github.com/bazelbuild/rules_rust/issues/2324 --- .../no_cargo_manifests/BUILD.bazel | 1 - .../vendor_local_pkgs/BUILD.bazel | 1 - .../vendor_remote_pkgs/BUILD.bazel | 1 - .../vendor_remote_pkgs/BUILD.bazel | 1 - rust/private/rust.bzl | 6 ++++++ rust/settings/BUILD.bazel | 17 +++++++++++----- rust/toolchain.bzl | 4 ++++ .../test_with_srcs/BUILD.bazel | 20 ------------------- .../test_with_srcs/src/extra.rs | 4 ---- .../test_with_srcs/src/lib.rs | 18 ----------------- test/out_dir_in_tests/BUILD.bazel | 5 +---- 11 files changed, 23 insertions(+), 55 deletions(-) delete mode 100644 test/inline_test_with_deps/test_with_srcs/BUILD.bazel delete mode 100644 test/inline_test_with_deps/test_with_srcs/src/extra.rs delete mode 100644 test/inline_test_with_deps/test_with_srcs/src/lib.rs diff --git a/examples/crate_universe/no_cargo_manifests/BUILD.bazel b/examples/crate_universe/no_cargo_manifests/BUILD.bazel index 2691eb2661..3dc2664f9b 100644 --- a/examples/crate_universe/no_cargo_manifests/BUILD.bazel +++ b/examples/crate_universe/no_cargo_manifests/BUILD.bazel @@ -19,7 +19,6 @@ rust_binary( rust_test( name = "unit_test", - srcs = glob(["**/*.rs"]), crate = ":no_cargo_manifests", edition = "2018", ) diff --git a/examples/crate_universe/vendor_local_pkgs/BUILD.bazel b/examples/crate_universe/vendor_local_pkgs/BUILD.bazel index 195809b0b8..a4b39473fd 100644 --- a/examples/crate_universe/vendor_local_pkgs/BUILD.bazel +++ b/examples/crate_universe/vendor_local_pkgs/BUILD.bazel @@ -69,7 +69,6 @@ rust_binary( rust_test( name = "unit_test", - srcs = glob(["**/*.rs"]), crate = ":vendor_local", edition = "2018", ) diff --git a/examples/crate_universe/vendor_remote_pkgs/BUILD.bazel b/examples/crate_universe/vendor_remote_pkgs/BUILD.bazel index 63d12f49fd..d5b4e170f8 100644 --- a/examples/crate_universe/vendor_remote_pkgs/BUILD.bazel +++ b/examples/crate_universe/vendor_remote_pkgs/BUILD.bazel @@ -75,7 +75,6 @@ rust_binary( rust_test( name = "unit_test", - srcs = glob(["**/*.rs"]), crate = ":vendor_remote", edition = "2018", ) diff --git a/examples/crate_universe_unnamed/vendor_remote_pkgs/BUILD.bazel b/examples/crate_universe_unnamed/vendor_remote_pkgs/BUILD.bazel index 747d3343ce..271bbd8c54 100644 --- a/examples/crate_universe_unnamed/vendor_remote_pkgs/BUILD.bazel +++ b/examples/crate_universe_unnamed/vendor_remote_pkgs/BUILD.bazel @@ -70,7 +70,6 @@ rust_binary( rust_test( name = "unit_test", - srcs = glob(["**/*.rs"]), crate = ":vendor_remote", edition = "2018", ) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 4147bf59a3..724e044322 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -283,6 +283,12 @@ def _rust_test_impl(ctx): deps = transform_deps(ctx.attr.deps) proc_macro_deps = transform_deps(ctx.attr.proc_macro_deps + get_import_macro_deps(ctx)) + if toolchain._incompatible_test_attr_crate_and_srcs_mutually_exclusive: + if ctx.attr.crate and ctx.attr.srcs: + fail("rust_test.crate and rust_test.srcs are mutually exclusive. Update {} to use only one of these attributes".format( + ctx.label, + )) + if ctx.attr.crate: # Target is building the crate in `test` config crate = ctx.attr.crate[rust_common.crate_info] if rust_common.crate_info in ctx.attr.crate else ctx.attr.crate[rust_common.test_crate_info].crate diff --git a/rust/settings/BUILD.bazel b/rust/settings/BUILD.bazel index 40b7d3120e..f1ba202552 100644 --- a/rust/settings/BUILD.bazel +++ b/rust/settings/BUILD.bazel @@ -4,6 +4,11 @@ load(":incompatible.bzl", "incompatible_flag") package(default_visibility = ["//visibility:public"]) +bzl_library( + name = "bzl_lib", + srcs = glob(["**/*.bzl"]), +) + # A flag controlling whether to rename first-party crates such that their names # encode the Bazel package and target name, instead of just the target name. # @@ -60,14 +65,16 @@ bool_flag( build_setting_default = False, ) -bzl_library( - name = "bzl_lib", - srcs = glob(["**/*.bzl"]), -) - # A flag to set rustc --sysroot flag to the sysroot generated by rust_toolchain incompatible_flag( name = "experimental_toolchain_generated_sysroot", build_setting_default = True, issue = "https://github.com/bazelbuild/rules_rust/issues/2039", ) + +# A flag to make `rust_test.crate` and `rust_test.srcs` mutually exclusive. +incompatible_flag( + name = "incompatible_test_attr_crate_and_srcs_mutually_exclusive", + build_setting_default = True, + issue = "https://github.com/bazelbuild/rules_rust/issues/2324", +) diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index 66f6b0e52c..f2cf3c7468 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -643,6 +643,7 @@ def _rust_toolchain_impl(ctx): _experimental_use_global_allocator = experimental_use_global_allocator, _experimental_use_coverage_metadata_files = ctx.attr._experimental_use_coverage_metadata_files[BuildSettingInfo].value, _experimental_toolchain_generated_sysroot = ctx.attr._experimental_toolchain_generated_sysroot[IncompatibleFlagInfo].enabled, + _incompatible_test_attr_crate_and_srcs_mutually_exclusive = ctx.attr._incompatible_test_attr_crate_and_srcs_mutually_exclusive[IncompatibleFlagInfo].enabled, _no_std = no_std, ) return [ @@ -806,6 +807,9 @@ rust_toolchain = rule( "This flag is only relevant when used together with --@rules_rust//rust/settings:experimental_use_global_allocator." ), ), + "_incompatible_test_attr_crate_and_srcs_mutually_exclusive": attr.label( + default = Label("//rust/settings:incompatible_test_attr_crate_and_srcs_mutually_exclusive"), + ), "_no_std": attr.label( default = Label("//:no_std"), ), diff --git a/test/inline_test_with_deps/test_with_srcs/BUILD.bazel b/test/inline_test_with_deps/test_with_srcs/BUILD.bazel deleted file mode 100644 index 64d628e512..0000000000 --- a/test/inline_test_with_deps/test_with_srcs/BUILD.bazel +++ /dev/null @@ -1,20 +0,0 @@ -load("//rust:defs.bzl", "rust_library", "rust_test") - -package(default_visibility = ["//visibility:public"]) - -rust_library( - name = "inline", - srcs = ["src/lib.rs"], - edition = "2018", - # Not all source files are included in this this target (`extra.rs`) - # and as a result, rustfmt complains about a missing module. Do not - # run rustfmt to avoid this issue. - tags = ["norustfmt"], -) - -rust_test( - name = "inline_test", - srcs = ["src/extra.rs"], - crate = ":inline", - deps = ["//test/inline_test_with_deps/dep"], -) diff --git a/test/inline_test_with_deps/test_with_srcs/src/extra.rs b/test/inline_test_with_deps/test_with_srcs/src/extra.rs deleted file mode 100644 index b72dd58c2e..0000000000 --- a/test/inline_test_with_deps/test_with_srcs/src/extra.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg(test)] -pub(crate) fn extra_test_fn() -> u32 { - 100 -} diff --git a/test/inline_test_with_deps/test_with_srcs/src/lib.rs b/test/inline_test_with_deps/test_with_srcs/src/lib.rs deleted file mode 100644 index da59d95e48..0000000000 --- a/test/inline_test_with_deps/test_with_srcs/src/lib.rs +++ /dev/null @@ -1,18 +0,0 @@ -#[allow(dead_code)] -fn multiply(val: u32) -> u32 { - val * 100 -} - -#[cfg(test)] -mod extra; - -#[cfg(test)] -mod tests { - use super::{extra, multiply}; - use dep::example_test_dep_fn; - - #[test] - fn test() { - assert_eq!(extra::extra_test_fn(), multiply(example_test_dep_fn())); - } -} diff --git a/test/out_dir_in_tests/BUILD.bazel b/test/out_dir_in_tests/BUILD.bazel index c46bae9d19..496f5aae0b 100644 --- a/test/out_dir_in_tests/BUILD.bazel +++ b/test/out_dir_in_tests/BUILD.bazel @@ -30,9 +30,6 @@ rust_doc_test( rust_test_suite( name = "suite", srcs = glob(["tests/**"]), - # Add the 'crate' argument, which will be passed as a kwarg - # to the underlying rust_test rules. This will make OUT_DIR - # available when compiling integration tests. - crate = ":demo_lib", edition = "2018", + deps = [":build_script"], )