Auto merge of rust-lang#6397 - matsujika:fix-6384, r=flip1995

Fix a false positive in `unnecessary_wraps`

Fix rust-lang#6384

changelog: Fix FP in `unnecessary_wraps` that happens when `Call` expr has `Return` expr inside

Author: bors

clippy_lints/src/methods/mod.rs

@@ -14,10 +14,8 @@ use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
-use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{TraitItem, TraitItemKind};
 use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
-use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::{self, TraitRef, Ty, TyS};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -28,11 +26,12 @@ use crate::consts::{constant, Constant};
 use crate::utils::eager_or_lazy::is_lazyness_candidate;
 use crate::utils::usage::mutated_variables;
 use crate::utils::{
-    contains_ty, get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, in_macro,
-    is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath,
-    match_trait_method, match_type, match_var, meets_msrv, method_calls, method_chain_args, paths, remove_blocks,
-    return_ty, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
-    span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, walk_ptrs_ty_depth, SpanlessEq,
+    contains_return, contains_ty, get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher,
+    implements_trait, in_macro, is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment,
+    match_def_path, match_qpath, match_trait_method, match_type, match_var, meets_msrv, method_calls,
+    method_chain_args, paths, remove_blocks, return_ty, single_segment_path, snippet, snippet_with_applicability,
+    snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
+    walk_ptrs_ty_depth, SpanlessEq,
 };
 use semver::{Version, VersionReq};
 
@@ -3877,36 +3876,6 @@ fn is_bool(ty: &hir::Ty<'_>) -> bool {
     }
 }
 
-// Returns `true` if `expr` contains a return expression
-fn contains_return(expr: &hir::Expr<'_>) -> bool {
-    struct RetCallFinder {
-        found: bool,
-    }
-
-    impl<'tcx> intravisit::Visitor<'tcx> for RetCallFinder {
-        type Map = Map<'tcx>;
-
-        fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
-            if self.found {
-                return;
-            }
-            if let hir::ExprKind::Ret(..) = &expr.kind {
-                self.found = true;
-            } else {
-                intravisit::walk_expr(self, expr);
-            }
-        }
-
-        fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
-            intravisit::NestedVisitorMap::None
-        }
-    }
-
-    let mut visitor = RetCallFinder { found: false };
-    visitor.visit_expr(expr);
-    visitor.found
-}
-
 fn check_pointer_offset(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
     if_chain! {
         if args.len() == 2;

clippy_lints/src/unnecessary_wraps.rs

@@ -1,5 +1,5 @@
 use crate::utils::{
-    in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then,
+    contains_return, in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then,
     visitors::find_all_ret_expressions,
 };
 use if_chain::if_chain;
@@ -95,6 +95,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps {
                 if let ExprKind::Path(ref qpath) = func.kind;
                 if match_qpath(qpath, path);
                 if args.len() == 1;
+                if !contains_return(&args[0]);
                 then {
                     suggs.push((ret_expr.span, snippet(cx, args[0].span.source_callsite(), "..").to_string()));
                     true

clippy_lints/src/utils/mod.rs

@@ -572,6 +572,36 @@ pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool {
     cn.result
 }
 
+/// Returns `true` if `expr` contains a return expression
+pub fn contains_return(expr: &hir::Expr<'_>) -> bool {
+    struct RetCallFinder {
+        found: bool,
+    }
+
+    impl<'tcx> hir::intravisit::Visitor<'tcx> for RetCallFinder {
+        type Map = Map<'tcx>;
+
+        fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
+            if self.found {
+                return;
+            }
+            if let hir::ExprKind::Ret(..) = &expr.kind {
+                self.found = true;
+            } else {
+                hir::intravisit::walk_expr(self, expr);
+            }
+        }
+
+        fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap<Self::Map> {
+            hir::intravisit::NestedVisitorMap::None
+        }
+    }
+
+    let mut visitor = RetCallFinder { found: false };
+    visitor.visit_expr(expr);
+    visitor.found
+}
+
 /// Converts a span to a code snippet if available, otherwise use default.
 ///
 /// This is useful if you want to provide suggestions for your lint or more generally, if you want

tests/ui/unnecessary_wraps.rs

@@ -109,6 +109,13 @@ impl B for A {
     }
 }
 
+fn issue_6384(s: &str) -> Option<&str> {
+    Some(match s {
+        "a" => "A",
+        _ => return None,
+    })
+}
+
 fn main() {
     // method calls are not linted
     func1(true, true);