Allow `clippy::too_many_arguments`

[dariocurr]

Hi, this is Dario from hiop.

I have an async fn test function that uses 8 parameters. I cannot #[allow(clippy::too_many_arguments)] probably because the Error originated from a macro called here.

Is there any way to allow this without changing other macro configurations?

If a fix is needed, I am more than happy to contribute to this awesome project.

[la10736]

Hi Dario, can you post a simple example that's expose the issue?

Anyway I guess that to annotate the test module by #[allow(clippy::too_many_arguments)] should work... let me know

Il giorno mer 13 dic 2023 alle ore 11:01 Dario Curreri < @.***> ha scritto:

Hi, this is Dario from hiop.

I have an async fn test function that uses 8 parameters. I cannot #[allow(clippy::too_many_arguments)] probably because the Error originated from a macro called here.

Is there any way to allow this without changing other macro configurations?

If a fix is needed, I am more than happy to contribute to this awesome project.

— Reply to this email directly, view it on GitHub https://github.com/la10736/rstest/issues/226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5Y34LGZGOQLU6Y7R4LHBLYJF4IHAVCNFSM6AAAAABAS337EOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGAZTSMZVHA3DANA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[dariocurr]

Put it at module level solved the issue. By the way I provide you the example:

#[allow(clippy::too_many_arguments)] // this works
mod tests {
    use rstest::rstest;

    #[allow(clippy::too_many_arguments)] // this does not work
    #[rstest]
    #[case(1, 2, 3, 4, 5, 6, 7, 8)]
    fn test_fn(
        #[case] _a: u8,
        #[case] _b: u8,
        #[case] _c: u8,
        #[case] _d: u8,
        #[case] _e: u8,
        #[case] _f: u8,
        #[case] _g: u8,
        #[case] _h: u8,
    ) {
    }
}

[la10736]

Ok... According to https://docs.rs/rstest/latest/rstest/attr.rstest.html#use-specific-case-attributes you should move the annotation before the function instead of put it before #[rstest].

[dariocurr]

Sorry, I should have mentioned before that I tried it already. As you can check:

//#[allow(clippy::too_many_arguments)] // this works
mod tests {
    use rstest::rstest;

    #[allow(clippy::too_many_arguments)] // this does not work
    #[rstest]
    #[allow(clippy::too_many_arguments)] // this does not work
    #[case(1, 2, 3, 4, 5, 6, 7, 8)]
    #[allow(clippy::too_many_arguments)] // this does not work
    fn test_fn(
        #[case] _a: u8,
        #[case] _b: u8,
        #[case] _c: u8,
        #[case] _d: u8,
        #[case] _e: u8,
        #[case] _f: u8,
        #[case] _g: u8,
        #[case] _h: u8,
    ) {
    }
}

[la10736]

Wired!!!! On my side it works like a charm in all cases

damico@miklap:~/dev_random/rstest_226$ cargo test && cat src/main.rs && cargo clippy && rustup show | tail
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests src/main.rs (target/debug/deps/rstest_226-a0447c32f017b59d)

running 1 test
test tests::test_fn::case_1 ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

fn main() {
    println!("Hello, world!");
}

mod tests {
    use rstest::rstest;

    #[allow(clippy::too_many_arguments)] // this does not work
    #[rstest]
    #[allow(clippy::too_many_arguments)] // this does not work
    #[case(1, 2, 3, 4, 5, 6, 7, 8)]
    #[allow(clippy::too_many_arguments)] // this does not work
    fn test_fn(
        #[case] _a: u8,
        #[case] _b: u8,
        #[case] _c: u8,
        #[case] _d: u8,
        #[case] _e: u8,
        #[case] _f: u8,
        #[case] _g: u8,
        #[case] _h: u8,
    ) {
    }
}
    Checking rstest_226 v0.1.0 (/home/mdamico/dev_random/rstest_226)
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.74.1 (a28077b28 2023-12-04)

[dariocurr]

Then I guess it's a rust-analyzer bug

Moreover, If i run cargo clippy -- --deny warnings, everything works fine

[jrray]

Although this is marked closed, I am also getting this clippy lint and when expanding the macro I see that when my test function is generated it doesn't have any attributes, as would be expected looking at line 437 here:

https://github.com/la10736/rstest/blob/3d6f05fcfcd686d290869cd49ae39a10eaaf25d0/rstest_macros/src/render/mod.rs#L435-L444

My test code looks like:

#[allow(clippy::too_many_arguments)]
#[rstest]
// cases
#[tokio::test]
async fn test_with_too_many_args(..., #[values(...)]) { ... }

After macro expansion I see:

#[cfg(test)]
async fn test_with_too_many_args(...) { // <-- too many args here but no allow attribute
    ...
}
#[cfg(test)]
mod test_with_too_many_args {
     use super::*;
     mod case_1_foo {
        use super::*;
        #[allow(clippy::too_many_arguments)]  // <-- attribute shows up here but doesn't help
        ...

Moving the #[allow(clippy::too_many_arguments)] to different places in the fn definition doesn't change the outcome.

Putting the attribute on a nested mod as a workaround does work.

[la10736]

I open it again. You're right. I tried my example again and I noted that I forgot --all-targets.

What I should try to do is assume that all allow and deny attributes that I found should go just before the original function and not before the wrapped. That's quite wiring because I cannot distinguish the attributes before #[rstest] and the ones just before the first case: allow and deny will become a special attributes that will follow a different path that the other ones and become independent by the position.

[lmmx]

It seems like this is maybe being noticed again, I wanted to mention I came across it when using rstest with 5 case parameters in a function :

#[rstest]
#[case::nonexistent_pattern(
    Some(vec!["definitely_nonexistent_pattern_xyz123".to_string()]),
    false,
    Some("No matching tests to analyze"),
    "Pattern that doesn't match any test",
    None
)]
#[case::error_handling(
    Some(vec!["tests::test_foo".to_string()]),
    false,
    None,
    "Error handling test with invalid output directory",
    Some(true)
)]
#[case::zero_coverage(
   Some(vec!["tests::test_not_bar".to_string()]),
    true,
    None,
    "Test with zero coverage",
    None
)]
#[case::mixed_tests(
    Some(vec![
        "tests::test_foo".to_string(),
        "tests::test_not_bar".to_string(),
    ]),
    true,
    None,
    "Mixed tests with and without coverage",
    None
)]
#[allow(clippy::too_many_arguments)]
fn test_execute_analyze_command(
    #[case] tests: Option<Vec<String>>,
    #[case] expect_ok: bool,
    #[case] error_substring: Option<&str>,
   #[case] description: &str,
   #[case] create_file_instead_of_dir: Option<bool>,
   temp_dir: TempDir,
   output_dir: PathBuf,
    report_path: PathBuf,
    demo_path: PathBuf,
    original_dir: PathBuf,
) {

I put this at the top of the module

#![allow(clippy::too_many_arguments)]

as this seems to be the suggestion above