unwrap_used/expect_used fire in test module function regardless of allow-unwrap-in-tests/allow-expect-in-tests

[Firionus]

Summary

Functions inside modules with #[cfg(test)] that are themselves not #[test] functions are linted for unwrap_used and expect_used even when clippy.toml sets allow-unwrap-in-tests = true and allow-expect-in-tests = true.

Lint Name

unwrap_used & expect_used

Reproducer

I tried this code:

main.rs

#![warn(clippy::unwrap_used, clippy::expect_used)]

fn main() {
    // warns as expected
    let _a: u8 = 0.try_into().unwrap();
    let _a: u8 = 1.try_into().expect("");
}

#[cfg(test)]
mod tests {
    #[test]
    fn test_fn() {
        // doesn't warn, as expected
        let _a: u8 = 2.try_into().unwrap();
        let _a: u8 = 3.try_into().expect("");

        util();
    }

    fn util() {
        // BUG: warns even though it is in test module
        let _a: u8 = 4.try_into().unwrap();
        let _a: u8 = 5.try_into().expect("");
    }
}

clippy.toml

allow-unwrap-in-tests = true
allow-expect-in-tests = true

I saw this happen:

$ cargo clippy --tests
warning: used `unwrap()` on `a Result` value
 --> src/main.rs:5:18
  |
5 |     let _a: u8 = 0.try_into().unwrap();
  |                  ^^^^^^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::unwrap_used, clippy::expect_used)]
  |         ^^^^^^^^^^^^^^^^^^^
  = help: if you don't want to handle the `Err` case gracefully, consider using `expect()` to provide a better panic message
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used

warning: used `expect()` on `a Result` value
 --> src/main.rs:6:18
  |
6 |     let _a: u8 = 1.try_into().expect("");
  |                  ^^^^^^^^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:30
  |
1 | #![warn(clippy::unwrap_used, clippy::expect_used)]
  |                              ^^^^^^^^^^^^^^^^^^^
  = help: if this value is an `Err`, it will panic
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_used

warning: used `unwrap()` on `a Result` value
  --> src/main.rs:22:22
   |
22 |         let _a: u8 = 4.try_into().unwrap();
   |                      ^^^^^^^^^^^^^^^^^^^^^
   |
   = help: if you don't want to handle the `Err` case gracefully, consider using `expect()` to provide a better panic message
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used

warning: used `expect()` on `a Result` value
  --> src/main.rs:23:22
   |
23 |         let _a: u8 = 5.try_into().expect("");
   |                      ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: if this value is an `Err`, it will panic
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_used

warning: `rust-analyzer-clippy-toml-bug` (bin "rust-analyzer-clippy-toml-bug" test) generated 4 warnings

I expected to see this happen: Lints should not fire for lines 22 and 23, since unwrap and expect should be allowed in tests and these lines are part of a test module.

Version

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: x86_64-unknown-linux-gnu
release: 1.64.0
LLVM version: 14.0.6

Additional Labels

config feature was introduced in #8802

[kraktus]

It’s unclear if this is undesired. At least it’s consistent with the doc.

allow-unwrap-in-tests: bool: Whether unwrap should be allowed in test functions (defaults to false)

[Firionus]

My expected behavior was considered desirable in the original issue:

This will not work properly if test module contains helper functions that do unwrap() and #[test] function calls this helper function rather than unwrap(). Such lints should be disabled for the whole test module.

(taken from #1015 (comment))

What I want to enforce in my code is that it does not panic under any expected condition. Since I do not consider test modules to be part of my normal library code, I don’t think this lint should apply there.

[kraktus]

@rustbot claim