On nightlies and betas, cargo +nightly clippy creates items_after_test_module warnings

[ilyagr]

Example:

warning: items were found after the testing module
    --> lib/tests/test_revset.rs:2277:1
     |
2277 |   #[test_case(false ; "local backend")]
     |   ^------------------------------------
     |   |
     |  _in this procedural macro expansion
     | |
2278 | | #[test_case(true ; "git backend")]
2279 | | fn test_evaluate_expression_conflict(use_git: bool) {
2280 | |     let settings = testutils::user_settings();
...    |
2488 | |     assert_eq!(resolve_prefix("a"), PrefixResolution::Ambi...
2489 | | }
     | |_^
     |
     = help: move the items to before the testing module was defined
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module
     = note: `#[warn(clippy::items_after_test_module)]` on by default
     = note: this warning originates in the attribute macro `test_case` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `jujutsu-lib` (test "test_revset") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 12.46s
Wed 17 May 2023 12:22:39 PM PDT
[Finished running. Exit status: 0]

when running cargo +nightly clippy on https://github.com/martinvonz/jj/tree/e3beb82d5fea50a7d55d515237fa800c30526c26.

This regards the test at https://github.com/martinvonz/jj/blob/e3beb82d5fea50a7d55d515237fa800c30526c26/lib/tests/test_revset.rs.

This is with

$ rustc --version
rustc 1.71.0-nightly (e77366b57 2023-05-16)
$  cargo +nightly clippy --version
clippy 0.1.71 (e77366b 2023-05-16)

I don't have a minimal example nor have I 100% concluded this is a bug in test-case, but I'm filing the issue as is in case the cause would be obvious to the maintainer.

[ilyagr]

The error now shows up on the beta clippy (1.71), but not yet on stable (1.70).

[luke-biel]

I'm gonna look at this asap. Probably we can just add a simple #[allow for now.

[luke-biel]

For now I can confirm that with romeval of

test-case/crates/test-case-macros/src/lib.rs

Line 81 in ea03c58

#[cfg(test)]

line the error disappears, but I'll need to double check whether I can safely remove it without introducing a breaking change into the lib. As for tests - they seem to pass fine.

[ilyagr]

This seems like great progress to me -- if you don't figure out anything else, you can try to put something like https://github.com/martinvonz/jj/pull/1686/files before that line. I'm not sure if there's a way to make that snippet more elegant.

[blyxyas]

I'm going to open a PR to rust-lang/rust-clippy that should fix this, don't worry ❤️

[ilyagr]

I think this should be fixed in all Rust versions thanks to rust-lang/rust-clippy#10992 and rust-lang/rust#113417 (thank you!). If you get hit by it, rustup update should fix it.

I guess I'll close it, though I haven't completely verified the fix.

[ilyagr]

Apparently this bug is still present in clippy 1.71 stable :( Not sure why. It should be fixed in 1.72.

#126 (comment)

[blyxyas]

The fix was backported to the beta toolchain on the rust-lang/rust@8cde275 (#113417) commit, merged on Jul 7 (being beta branched from master on July 7). So the backport has been on the beta toolchain for about 11 days.

It will be on stable on Aug 24 (the day 1.72 is released). Until that day, users will have to use a beta/nightly toolchain. I may be incorrect, but maybe there's a way to allow this lint on build.rs until that stable version comes around.

[brettcannon]

FYI I just updated to to clippy 0.1.72 (5680fa18 2023-08-23) which comes with Rust 1.72 and I'm still getting the warning for https://github.com/brettcannon/python-launcher/blob/main/tests/main_tests.rs (I'm going to work around it by disabling the check as the file is integration tests).