From f750a3f842d684d4de4edc5ae6ee3bf92d47a9bd Mon Sep 17 00:00:00 2001 From: chansuke Date: Sun, 2 Aug 2020 00:45:11 +0900 Subject: [PATCH] Move `is_test_module_or_function` to utils --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/unwrap.rs | 52 ++++++++++++++++++---------- clippy_lints/src/utils/mod.rs | 5 +++ clippy_lints/src/wildcard_imports.rs | 6 +--- tests/ui/unwrap.rs | 16 --------- tests/ui/unwrap.stderr | 22 +++++++----- 6 files changed, 54 insertions(+), 49 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 81864340f1a5..71d3f772184f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -988,7 +988,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box map_unit_fn::MapUnit); store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default()); store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd); - store.register_late_pass(|| box unwrap::Unwrap); + store.register_late_pass(|| box unwrap::Unwrap::new()); store.register_late_pass(|| box duration_subsec::DurationSubsec); store.register_late_pass(|| box default_trait_access::DefaultTraitAccess); store.register_late_pass(|| box indexing_slicing::IndexingSlicing); diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index 7008589bcab3..f87fddfc477e 100644 --- a/clippy_lints/src/unwrap.rs +++ b/clippy_lints/src/unwrap.rs @@ -1,15 +1,15 @@ use crate::utils::{ - differing_macro_contexts, higher::if_block, is_type_diagnostic_item, span_lint_and_then, - usage::is_potentially_mutated, + differing_macro_contexts, higher::if_block, is_test_module_or_function, is_type_diagnostic_item, + span_lint_and_then, usage::is_potentially_mutated, }; use if_chain::if_chain; use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor}; -use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Item, ItemKind, Path, QPath, UnOp}; +use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Item, Path, QPath, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::Ty; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; declare_clippy_lint! { @@ -207,12 +207,23 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> { } } -declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]); +#[derive(Default)] +pub struct Unwrap { + test_modules_deep: u32, +} + +impl Unwrap { + pub fn new() -> Self { + Self { test_modules_deep: 0 } + } +} + +impl_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]); impl<'tcx> LateLintPass<'tcx> for Unwrap { - fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + fn check_item(&mut self, _: &LateContext<'_>, item: &Item<'_>) { if is_test_module_or_function(item) { - return; + self.test_modules_deep = self.test_modules_deep.saturating_add(1); } } fn check_fn( @@ -224,19 +235,24 @@ impl<'tcx> LateLintPass<'tcx> for Unwrap { span: Span, fn_id: HirId, ) { - if span.from_expansion() { - return; + if_chain! { + if !span.from_expansion(); + if !self.check_test_module(); + if let mut v = UnwrappableVariablesVisitor { cx, unwrappables: Vec::new(), }; + then { + walk_fn(&mut v, kind, decl, body.id(), span, fn_id); + } + } + } + fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { + if is_test_module_or_function(item) { + self.test_modules_deep = self.test_modules_deep.saturating_sub(1); } - - let mut v = UnwrappableVariablesVisitor { - cx, - unwrappables: Vec::new(), - }; - - walk_fn(&mut v, kind, decl, body.id(), span, fn_id); } } -fn is_test_module_or_function(item: &Item<'_>) -> bool { - matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") +impl Unwrap { + fn check_test_module(&self) -> bool { + self.test_modules_deep > 0 + } } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 4b6749808990..648dd097e32e 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1407,6 +1407,11 @@ pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { } } +// Check if item is #[test]ing code. +pub fn is_test_module_or_function(item: &Item<'_>) -> bool { + matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") +} + #[macro_export] macro_rules! unwrap_cargo_metadata { ($cx: ident, $lint: ident, $deps: expr) => {{ diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index e7eb7c2e9802..8686ca3d1231 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -1,4 +1,4 @@ -use crate::utils::{in_macro, snippet, snippet_with_applicability, span_lint_and_sugg}; +use crate::utils::{in_macro, is_test_module_or_function, snippet, snippet_with_applicability, span_lint_and_sugg}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{ @@ -208,7 +208,3 @@ fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool { fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool { segments.len() == 1 && segments[0].ident.as_str() == "super" } - -fn is_test_module_or_function(item: &Item<'_>) -> bool { - matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") -} diff --git a/tests/ui/unwrap.rs b/tests/ui/unwrap.rs index beccccd08aea..a4a3cd1d3797 100644 --- a/tests/ui/unwrap.rs +++ b/tests/ui/unwrap.rs @@ -10,23 +10,7 @@ fn unwrap_result() { let _ = res.unwrap(); } -// Should not get linted. -#[test] -fn with_test() { - let opt = Some(0); - let _ = opt.unwrap(); -} - -// Should not get linted. -#[cfg(test)] -fn with_cfg_test() { - let res: Result = Ok(0); - let _ = res.unwrap(); -} - fn main() { unwrap_option(); unwrap_result(); - with_test(); - with_cfg_test(); } diff --git a/tests/ui/unwrap.stderr b/tests/ui/unwrap.stderr index bd05819f8dfb..4f0858005f6e 100644 --- a/tests/ui/unwrap.stderr +++ b/tests/ui/unwrap.stderr @@ -1,15 +1,19 @@ -error[E0425]: cannot find function `with_test` in this scope - --> $DIR/unwrap.rs:30:5 +error: used `unwrap()` on `an Option` value + --> $DIR/unwrap.rs:5:13 | -LL | with_test(); - | ^^^^^^^^^ not found in this scope +LL | let _ = opt.unwrap(); + | ^^^^^^^^^^^^ + | + = note: `-D clippy::unwrap-used` implied by `-D warnings` + = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message -error[E0425]: cannot find function `with_cfg_test` in this scope - --> $DIR/unwrap.rs:31:5 +error: used `unwrap()` on `a Result` value + --> $DIR/unwrap.rs:10:13 + | +LL | let _ = res.unwrap(); + | ^^^^^^^^^^^^ | -LL | with_cfg_test(); - | ^^^^^^^^^^^^^ not found in this scope + = help: if you don't want to handle the `Err` case gracefully, consider using `expect()` to provide a better panic message error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0425`.