Skip to content

Commit

Permalink
Optionally allow expect and unwrap in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed May 8, 2022
1 parent bdfea1c commit 597f61b
Show file tree
Hide file tree
Showing 12 changed files with 543 additions and 181 deletions.
11 changes: 10 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,17 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});

let avoid_breaking_exported_api = conf.avoid_breaking_exported_api;
let allow_expect_in_tests = conf.allow_expect_in_tests;
let allow_unwrap_in_tests = conf.allow_unwrap_in_tests;
store.register_late_pass(move || Box::new(approx_const::ApproxConstant::new(msrv)));
store.register_late_pass(move || Box::new(methods::Methods::new(avoid_breaking_exported_api, msrv)));
store.register_late_pass(move || {
Box::new(methods::Methods::new(
avoid_breaking_exported_api,
msrv,
allow_expect_in_tests,
allow_unwrap_in_tests,
))
});
store.register_late_pass(move || Box::new(matches::Matches::new(msrv)));
store.register_early_pass(move || Box::new(manual_non_exhaustive::ManualNonExhaustiveStruct::new(msrv)));
store.register_late_pass(move || Box::new(manual_non_exhaustive::ManualNonExhaustiveEnum::new(msrv)));
Expand Down
7 changes: 6 additions & 1 deletion clippy_lints/src/methods/expect_used.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_in_test_function;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir as hir;
use rustc_lint::LateContext;
Expand All @@ -7,7 +8,7 @@ use rustc_span::sym;
use super::EXPECT_USED;

/// lint use of `expect()` for `Option`s and `Result`s
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, allow_expect_in_tests: bool) {
let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();

let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) {
Expand All @@ -18,6 +19,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
None
};

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

if let Some((lint, kind, none_value)) = mess {
span_lint_and_help(
cx,
Expand Down
368 changes: 191 additions & 177 deletions clippy_lints/src/methods/mod.rs

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion clippy_lints/src/methods/unwrap_used.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_in_test_function;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir as hir;
use rustc_lint::LateContext;
Expand All @@ -7,7 +8,7 @@ use rustc_span::sym;
use super::UNWRAP_USED;

/// lint use of `unwrap()` for `Option`s and `Result`s
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, allow_unwrap_in_tests: bool) {
let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs();

let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) {
Expand All @@ -18,6 +19,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
None
};

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

if let Some((lint, kind, none_value)) = mess {
span_lint_and_help(
cx,
Expand Down
8 changes: 8 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,14 @@ define_Conf! {
///
/// The maximum size of a file included via `include_bytes!()` or `include_str!()`, in bytes
(max_include_file_size: u64 = 1_000_000),
/// Lint: EXPECT_USED.
///
/// Whether `expect` should be allowed in test functions
(allow_expect_in_tests: bool = false),
/// Lint: UNWRAP_USED.
///
/// Whether `unwrap` should be allowed in test functions
(allow_unwrap_in_tests: bool = false),
}

/// Search for the configuration file.
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/expect_used/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-expect-in-tests = true
29 changes: 29 additions & 0 deletions tests/ui-toml/expect_used/expect_used.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// compile-flags: --test
#![warn(clippy::expect_used)]

fn expect_option() {
let opt = Some(0);
let _ = opt.expect("");
}

fn expect_result() {
let res: Result<u8, ()> = Ok(0);
let _ = res.expect("");
}

fn main() {
expect_option();
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("");
}
19 changes: 19 additions & 0 deletions tests/ui-toml/expect_used/expect_used.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: used `expect()` on `an Option` value
--> $DIR/expect_used.rs:6:13
|
LL | let _ = opt.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::expect-used` implied by `-D warnings`
= help: if this value is an `None`, it will panic

error: used `expect()` on `a Result` value
--> $DIR/expect_used.rs:11:13
|
LL | let _ = res.expect("");
| ^^^^^^^^^^^^^^
|
= help: if this value is an `Err`, it will panic

error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `await-holding-invalid-types`, `max-include-file-size`, `third-party` at line 5 column 1
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `await-holding-invalid-types`, `max-include-file-size`, `allow-expect-in-tests`, `allow-unwrap-in-tests`, `third-party` at line 5 column 1

error: aborting due to previous error

1 change: 1 addition & 0 deletions tests/ui-toml/unwrap_used/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-unwrap-in-tests = true
74 changes: 74 additions & 0 deletions tests/ui-toml/unwrap_used/unwrap_used.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// compile-flags: --test

#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
#![warn(clippy::unwrap_used)]
#![deny(clippy::get_unwrap)]

use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::VecDeque;
use std::iter::FromIterator;

struct GetFalsePositive {
arr: [u32; 3],
}

impl GetFalsePositive {
fn get(&self, pos: usize) -> Option<&u32> {
self.arr.get(pos)
}
fn get_mut(&mut self, pos: usize) -> Option<&mut u32> {
self.arr.get_mut(pos)
}
}

fn main() {
let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]);
let mut some_slice = &mut [0, 1, 2, 3];
let mut some_vec = vec![0, 1, 2, 3];
let mut some_vecdeque: VecDeque<_> = some_vec.iter().cloned().collect();
let mut some_hashmap: HashMap<u8, char> = HashMap::from_iter(vec![(1, 'a'), (2, 'b')]);
let mut some_btreemap: BTreeMap<u8, char> = BTreeMap::from_iter(vec![(1, 'a'), (2, 'b')]);
let mut false_positive = GetFalsePositive { arr: [0, 1, 2] };

{
// Test `get().unwrap()`
let _ = boxed_slice.get(1).unwrap();
let _ = some_slice.get(0).unwrap();
let _ = some_vec.get(0).unwrap();
let _ = some_vecdeque.get(0).unwrap();
let _ = some_hashmap.get(&1).unwrap();
let _ = some_btreemap.get(&1).unwrap();
#[allow(clippy::unwrap_used)]
let _ = false_positive.get(0).unwrap();
// Test with deref
let _: u8 = *boxed_slice.get(1).unwrap();
}

{
// Test `get_mut().unwrap()`
*boxed_slice.get_mut(0).unwrap() = 1;
*some_slice.get_mut(0).unwrap() = 1;
*some_vec.get_mut(0).unwrap() = 1;
*some_vecdeque.get_mut(0).unwrap() = 1;
// Check false positives
#[allow(clippy::unwrap_used)]
{
*some_hashmap.get_mut(&1).unwrap() = 'b';
*some_btreemap.get_mut(&1).unwrap() = 'b';
*false_positive.get_mut(0).unwrap() = 1;
}
}

{
// Test `get().unwrap().foo()` and `get_mut().unwrap().bar()`
let _ = some_vec.get(0..1).unwrap().to_vec();
let _ = some_vec.get_mut(0..1).unwrap().to_vec();
}
}

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

0 comments on commit 597f61b

Please sign in to comment.