clippy_lints/src/unnecessary_wrap.rs

   1 use crate::utils::{
   2     is_type_diagnostic_item, match_qpath, 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                     diag.multipart_suggestion(
 106                         "factor this out to",
 107                         suggs.into_iter().chain({
 108                             let inner_ty = return_ty(cx, hir_id)
 109                                 .walk()
 110                                 .skip(1) // skip `std::option::Option` or `std::result::Result`
 111                                 .take(1) // take the first outermost inner type
 112                                 .filter_map(|inner| match inner.unpack() {
 113                                     GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()),
 114                                     _ => None,
 115                                 });
 116                             inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty))
 117                         }).collect(),
 118                         Applicability::MachineApplicable,
 119                     );
 120                 },
 121             );
 122         }
 123     }
 124 }
 125
 126 // code below is copied from `bind_instead_of_map`
 127
 128 fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir Expr<'hir>, callback: F) -> bool
 129 where
 130     F: FnMut(&'hir Expr<'hir>) -> bool,
 131 {
 132     struct RetFinder<F> {
 133         in_stmt: bool,
 134         failed: bool,
 135         cb: F,
 136     }
 137
 138     struct WithStmtGuarg<'a, F> {
 139         val: &'a mut RetFinder<F>,
 140         prev_in_stmt: bool,
 141     }
 142
 143     impl<F> RetFinder<F> {
 144         fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
 145             let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
 146             WithStmtGuarg {
 147                 val: self,
 148                 prev_in_stmt,
 149             }
 150         }
 151     }
 152
 153     impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
 154         type Target = RetFinder<F>;
 155
 156         fn deref(&self) -> &Self::Target {
 157             self.val
 158         }
 159     }
 160
 161     impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
 162         fn deref_mut(&mut self) -> &mut Self::Target {
 163             self.val
 164         }
 165     }
 166
 167     impl<F> Drop for WithStmtGuarg<'_, F> {
 168         fn drop(&mut self) {
 169             self.val.in_stmt = self.prev_in_stmt;
 170         }
 171     }
 172
 173     impl<'hir, F: FnMut(&'hir Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> {
 174         type Map = Map<'hir>;
 175
 176         fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
 177             intravisit::NestedVisitorMap::None
 178         }
 179
 180         fn visit_stmt(&mut self, stmt: &'hir Stmt<'_>) {
 181             intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt)
 182         }
 183
 184         fn visit_expr(&mut self, expr: &'hir Expr<'_>) {
 185             if self.failed {
 186                 return;
 187             }
 188             if self.in_stmt {
 189                 match expr.kind {
 190                     ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr),
 191                     _ => intravisit::walk_expr(self, expr),
 192                 }
 193             } else {
 194                 match expr.kind {
 195                     ExprKind::Match(cond, arms, _) => {
 196                         self.inside_stmt(true).visit_expr(cond);
 197                         for arm in arms {
 198                             self.visit_expr(arm.body);
 199                         }
 200                     },
 201                     ExprKind::Block(..) => intravisit::walk_expr(self, expr),
 202                     ExprKind::Ret(Some(expr)) => self.visit_expr(expr),
 203                     _ => self.failed |= !(self.cb)(expr),
 204                 }
 205             }
 206         }
 207     }
 208
 209     !contains_try(expr) && {
 210         let mut ret_finder = RetFinder {
 211             in_stmt: false,
 212             failed: false,
 213             cb: callback,
 214         };
 215         ret_finder.visit_expr(expr);
 216         !ret_finder.failed
 217     }
 218 }
 219
 220 /// returns `true` if expr contains match expr desugared from try
 221 fn contains_try(expr: &Expr<'_>) -> bool {
 222     struct TryFinder {
 223         found: bool,
 224     }
 225
 226     impl<'hir> intravisit::Visitor<'hir> for TryFinder {
 227         type Map = Map<'hir>;
 228
 229         fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
 230             intravisit::NestedVisitorMap::None
 231         }
 232
 233         fn visit_expr(&mut self, expr: &'hir Expr<'hir>) {
 234             if self.found {
 235                 return;
 236             }
 237             match expr.kind {
 238                 ExprKind::Match(_, _, MatchSource::TryDesugar) => self.found = true,
 239                 _ => intravisit::walk_expr(self, expr),
 240             }
 241         }
 242     }
 243
 244     let mut visitor = TryFinder { found: false };
 245     visitor.visit_expr(expr);
 246     visitor.found
 247 }