clippy_lints/src/unnecessary_wrap.rs

   1 use crate::utils::{
   2     is_type_diagnostic_item, match_qpath, multispan_sugg_with_applicability, paths, return_ty, snippet,
   3     span_lint_and_then,
   4 };
   5 use if_chain::if_chain;
   6 use rustc_errors::Applicability;
   7 use rustc_hir::intravisit::{FnKind, Visitor};
   8 use rustc_hir::*;
   9 use rustc_lint::{LateContext, LateLintPass};
  10 use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind};
  11 use rustc_session::{declare_lint_pass, declare_tool_lint};
  12 use rustc_span::Span;
  13
  14 declare_clippy_lint! {
  15     /// **What it does:** Checks for private functions that only return `Ok` or `Some`.
  16     ///
  17     /// **Why is this bad?** It is not meaningful to wrap values when no `None` or `Err` is returned.
  18     ///
  19     /// **Known problems:** Since this lint changes function type signature, you may need to
  20     /// adjust some code at callee side.
  21     ///
  22     /// **Example:**
  23     ///
  24     /// ```rust
  25     /// fn get_cool_number(a: bool, b: bool) -> Option<i32> {
  26     ///     if a && b {
  27     ///         return Some(50);
  28     ///     }
  29     ///     if a {
  30     ///         Some(0)
  31     ///     } else {
  32     ///         Some(10)
  33     ///     }
  34     /// }
  35     /// ```
  36     /// Use instead:
  37     /// ```rust
  38     /// fn get_cool_number(a: bool, b: bool) -> i32 {
  39     ///     if a && b {
  40     ///         return 50;
  41     ///     }
  42     ///     if a {
  43     ///         0
  44     ///     } else {
  45     ///         10
  46     ///     }
  47     /// }
  48     /// ```
  49     pub UNNECESSARY_WRAP,
  50     complexity,
  51     "functions that only return `Ok` or `Some`"
  52 }
  53
  54 declare_lint_pass!(UnnecessaryWrap => [UNNECESSARY_WRAP]);
  55
  56 impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap {
  57     fn check_fn(
  58         &mut self,
  59         cx: &LateContext<'tcx>,
  60         fn_kind: FnKind<'tcx>,
  61         fn_decl: &FnDecl<'tcx>,
  62         body: &Body<'tcx>,
  63         span: Span,
  64         hir_id: HirId,
  65     ) {
  66         if_chain! {
  67             if let FnKind::ItemFn(.., visibility, _) = fn_kind;
  68             if visibility.node.is_pub();
  69             then {
  70                 return;
  71             }
  72         }
  73
  74         let path = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) {
  75             &paths::OPTION_SOME
  76         } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) {
  77             &paths::RESULT_OK
  78         } else {
  79             return;
  80         };
  81
  82         let mut suggs = Vec::new();
  83         let can_sugg = find_all_ret_expressions(cx, &body.value, |ret_expr| {
  84             if_chain! {
  85                 if let ExprKind::Call(ref func, ref args) = ret_expr.kind;
  86                 if let ExprKind::Path(ref qpath) = func.kind;
  87                 if match_qpath(qpath, path);
  88                 if args.len() == 1;
  89                 then {
  90                     suggs.push((ret_expr.span, snippet(cx, args[0].span.source_callsite(), "..").to_string()));
  91                     true
  92                 } else {
  93                     false
  94                 }
  95             }
  96         });
  97
  98         if can_sugg {
  99             span_lint_and_then(
 100                 cx,
 101                 UNNECESSARY_WRAP,
 102                 span,
 103                 "this function returns unnecessarily wrapping data",
 104                 move |diag| {
 105                     multispan_sugg_with_applicability(
 106                         diag,
 107                         "factor this out to",
 108                         Applicability::MachineApplicable,
 109                         suggs.into_iter().chain({
 110                             let inner_ty = return_ty(cx, hir_id)
 111                                 .walk()
 112                                 .skip(1) // skip `std::option::Option` or `std::result::Result`
 113                                 .take(1) // take the first outermost inner type
 114                                 .filter_map(|inner| match inner.unpack() {
 115                                     GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()),
 116                                     _ => None,
 117                                 });
 118                             inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty))
 119                         }),
 120                     );
 121                 },
 122             );
 123         }
 124     }
 125 }
 126
 127 // code below is copied from `bind_instead_of_map`
 128
 129 fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir Expr<'hir>, callback: F) -> bool
 130 where
 131     F: FnMut(&'hir Expr<'hir>) -> bool,
 132 {
 133     struct RetFinder<F> {
 134         in_stmt: bool,
 135         failed: bool,
 136         cb: F,
 137     }
 138
 139     struct WithStmtGuarg<'a, F> {
 140         val: &'a mut RetFinder<F>,
 141         prev_in_stmt: bool,
 142     }
 143
 144     impl<F> RetFinder<F> {
 145         fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
 146             let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
 147             WithStmtGuarg {
 148                 val: self,
 149                 prev_in_stmt,
 150             }
 151         }
 152     }
 153
 154     impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
 155         type Target = RetFinder<F>;
 156
 157         fn deref(&self) -> &Self::Target {
 158             self.val
 159         }
 160     }
 161
 162     impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
 163         fn deref_mut(&mut self) -> &mut Self::Target {
 164             self.val
 165         }
 166     }
 167
 168     impl<F> Drop for WithStmtGuarg<'_, F> {
 169         fn drop(&mut self) {
 170             self.val.in_stmt = self.prev_in_stmt;
 171         }
 172     }
 173
 174     impl<'hir, F: FnMut(&'hir Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> {
 175         type Map = Map<'hir>;
 176
 177         fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
 178             intravisit::NestedVisitorMap::None
 179         }
 180
 181         fn visit_stmt(&mut self, stmt: &'hir Stmt<'_>) {
 182             intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt)
 183         }
 184
 185         fn visit_expr(&mut self, expr: &'hir Expr<'_>) {
 186             if self.failed {
 187                 return;
 188             }
 189             if self.in_stmt {
 190                 match expr.kind {
 191                     ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr),
 192                     _ => intravisit::walk_expr(self, expr),
 193                 }
 194             } else {
 195                 match expr.kind {
 196                     ExprKind::Match(cond, arms, _) => {
 197                         self.inside_stmt(true).visit_expr(cond);
 198                         for arm in arms {
 199                             self.visit_expr(arm.body);
 200                         }
 201                     },
 202                     ExprKind::Block(..) => intravisit::walk_expr(self, expr),
 203                     ExprKind::Ret(Some(expr)) => self.visit_expr(expr),
 204                     _ => self.failed |= !(self.cb)(expr),
 205                 }
 206             }
 207         }
 208     }
 209
 210     !contains_try(expr) && {
 211         let mut ret_finder = RetFinder {
 212             in_stmt: false,
 213             failed: false,
 214             cb: callback,
 215         };
 216         ret_finder.visit_expr(expr);
 217         !ret_finder.failed
 218     }
 219 }
 220
 221 /// returns `true` if expr contains match expr desugared from try
 222 fn contains_try(expr: &Expr<'_>) -> bool {
 223     struct TryFinder {
 224         found: bool,
 225     }
 226
 227     impl<'hir> intravisit::Visitor<'hir> for TryFinder {
 228         type Map = Map<'hir>;
 229
 230         fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
 231             intravisit::NestedVisitorMap::None
 232         }
 233
 234         fn visit_expr(&mut self, expr: &'hir Expr<'hir>) {
 235             if self.found {
 236                 return;
 237             }
 238             match expr.kind {
 239                 ExprKind::Match(_, _, MatchSource::TryDesugar) => self.found = true,
 240                 _ => intravisit::walk_expr(self, expr),
 241             }
 242         }
 243     }
 244
 245     let mut visitor = TryFinder { found: false };
 246     visitor.visit_expr(expr);
 247     visitor.found
 248 }