Move is_test_module_or_function to utils

Author: chansuke

clippy_lints/src/lib.rs

@@ -992,7 +992,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);

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
+    }
 }

clippy_lints/src/utils/mod.rs

@@ -1416,6 +1416,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) => {{

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")
-}

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<u8, ()> = Ok(0);
-    let _ = res.unwrap();
-}
-
 fn main() {
     unwrap_option();
     unwrap_result();
-    with_test();
-    with_cfg_test();
 }

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`.