Skip to content

Commit

Permalink
Auto merge of rust-lang#10391 - ldm0:ldm0_fix_unwrap_in_tests, r=xFre…
Browse files Browse the repository at this point in the history
…dnet

Fix test function checker in `unwrap_used`, `expect_used`

After rust-lang#9686 , `unwrap` and `expect` in integration tests and raw test functions won't be allowed.

fixes rust-lang#10011
fixes rust-lang#10238
fixes rust-lang#10264

---

changelog: Fix: [`expect_used`], [`unwrap_used`], [`dbg_macro`], [`print_stdout`], [`print_stderr`]: No longer lint in test functions, if the related configuration is set
[rust-lang#10391](rust-lang/rust-clippy#10391)
<!-- changelog_checked -->
  • Loading branch information
bors committed Feb 24, 2023
2 parents 659112c + 84ceca8 commit b528cc9
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 14 deletions.
8 changes: 4 additions & 4 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -472,31 +472,31 @@ The maximum size of a file included via `include_bytes!()` or `include_str!()`,


### allow-expect-in-tests
Whether `expect` should be allowed within `#[cfg(test)]`
Whether `expect` should be allowed in test functions or `#[cfg(test)]`

**Default Value:** `false` (`bool`)

* [expect_used](https://rust-lang.github.io/rust-clippy/master/index.html#expect_used)


### allow-unwrap-in-tests
Whether `unwrap` should be allowed in test cfg
Whether `unwrap` should be allowed in test functions or `#[cfg(test)]`

**Default Value:** `false` (`bool`)

* [unwrap_used](https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used)


### allow-dbg-in-tests
Whether `dbg!` should be allowed in test functions
Whether `dbg!` should be allowed in test functions or `#[cfg(test)]`

**Default Value:** `false` (`bool`)

* [dbg_macro](https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro)


### allow-print-in-tests
Whether print macros (ex. `println!`) should be allowed in test functions
Whether print macros (ex. `println!`) should be allowed in test functions or `#[cfg(test)]`

**Default Value:** `false` (`bool`)

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/expect_used.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_in_cfg_test;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_in_cfg_test, is_in_test_function};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
Expand All @@ -27,7 +27,7 @@ pub(super) fn check(

let method = if is_err { "expect_err" } else { "expect" };

if allow_expect_in_tests && is_in_cfg_test(cx.tcx, expr.hir_id) {
if allow_expect_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/unwrap_used.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_in_cfg_test, is_lint_allowed};
use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
Expand All @@ -27,7 +27,7 @@ pub(super) fn check(

let method_suffix = if is_err { "_err" } else { "" };

if allow_unwrap_in_tests && is_in_cfg_test(cx.tcx, expr.hir_id) {
if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
return;
}

Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,19 +419,19 @@ define_Conf! {
(max_include_file_size: u64 = 1_000_000),
/// Lint: EXPECT_USED.
///
/// Whether `expect` should be allowed within `#[cfg(test)]`
/// Whether `expect` should be allowed in test functions or `#[cfg(test)]`
(allow_expect_in_tests: bool = false),
/// Lint: UNWRAP_USED.
///
/// Whether `unwrap` should be allowed in test cfg
/// Whether `unwrap` should be allowed in test functions or `#[cfg(test)]`
(allow_unwrap_in_tests: bool = false),
/// Lint: DBG_MACRO.
///
/// Whether `dbg!` should be allowed in test functions
/// Whether `dbg!` should be allowed in test functions or `#[cfg(test)]`
(allow_dbg_in_tests: bool = false),
/// Lint: PRINT_STDOUT, PRINT_STDERR.
///
/// Whether print macros (ex. `println!`) should be allowed in test functions
/// Whether print macros (ex. `println!`) should be allowed in test functions or `#[cfg(test)]`
(allow_print_in_tests: bool = false),
/// Lint: RESULT_LARGE_ERR.
///
Expand Down
12 changes: 12 additions & 0 deletions tests/ui-toml/expect_used/expect_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ fn main() {
expect_result();
}

#[test]
fn test_expect_option() {
let opt = Some(0);
let _ = opt.expect("");
}

#[test]
fn test_expect_result() {
let res: Result<u8, ()> = Ok(0);
let _ = res.expect("");
}

#[cfg(test)]
mod issue9612 {
// should not lint in `#[cfg(test)]` modules
Expand Down
6 changes: 6 additions & 0 deletions tests/ui-toml/unwrap_used/unwrap_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ fn main() {
}
}

#[test]
fn test() {
let boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]);
let _ = boxed_slice.get(1).unwrap();
}

#[cfg(test)]
mod issue9612 {
// should not lint in `#[cfg(test)]` modules
Expand Down
10 changes: 8 additions & 2 deletions tests/ui-toml/unwrap_used/unwrap_used.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,16 @@ LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/unwrap_used.rs:84:17
--> $DIR/unwrap_used.rs:72:13
|
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&boxed_slice[1]`

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/unwrap_used.rs:90:17
|
LL | let _ = Box::new([0]).get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&Box::new([0])[1]`

error: aborting due to 27 previous errors
error: aborting due to 28 previous errors

0 comments on commit b528cc9

Please sign in to comment.