|
| 1 | +use super::{contains_return, BIND_INSTEAD_OF_MAP}; |
| 2 | +use crate::utils::{ |
| 3 | + in_macro, match_qpath, match_type, method_calls, multispan_sugg_with_applicability, paths, remove_blocks, snippet, |
| 4 | + snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then, |
| 5 | +}; |
| 6 | +use if_chain::if_chain; |
| 7 | +use rustc_errors::Applicability; |
| 8 | +use rustc_hir as hir; |
| 9 | +use rustc_hir::intravisit::{self, Visitor}; |
| 10 | +use rustc_lint::LateContext; |
| 11 | +use rustc_middle::hir::map::Map; |
| 12 | +use rustc_span::Span; |
| 13 | + |
| 14 | +pub(crate) struct OptionAndThenSome; |
| 15 | +impl BindInsteadOfMap for OptionAndThenSome { |
| 16 | + const TYPE_NAME: &'static str = "Option"; |
| 17 | + const TYPE_QPATH: &'static [&'static str] = &paths::OPTION; |
| 18 | + |
| 19 | + const BAD_METHOD_NAME: &'static str = "and_then"; |
| 20 | + const BAD_VARIANT_NAME: &'static str = "Some"; |
| 21 | + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::OPTION_SOME; |
| 22 | + |
| 23 | + const GOOD_METHOD_NAME: &'static str = "map"; |
| 24 | +} |
| 25 | + |
| 26 | +pub(crate) struct ResultAndThenOk; |
| 27 | +impl BindInsteadOfMap for ResultAndThenOk { |
| 28 | + const TYPE_NAME: &'static str = "Result"; |
| 29 | + const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; |
| 30 | + |
| 31 | + const BAD_METHOD_NAME: &'static str = "and_then"; |
| 32 | + const BAD_VARIANT_NAME: &'static str = "Ok"; |
| 33 | + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_OK; |
| 34 | + |
| 35 | + const GOOD_METHOD_NAME: &'static str = "map"; |
| 36 | +} |
| 37 | + |
| 38 | +pub(crate) struct ResultOrElseErrInfo; |
| 39 | +impl BindInsteadOfMap for ResultOrElseErrInfo { |
| 40 | + const TYPE_NAME: &'static str = "Result"; |
| 41 | + const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; |
| 42 | + |
| 43 | + const BAD_METHOD_NAME: &'static str = "or_else"; |
| 44 | + const BAD_VARIANT_NAME: &'static str = "Err"; |
| 45 | + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_ERR; |
| 46 | + |
| 47 | + const GOOD_METHOD_NAME: &'static str = "map_err"; |
| 48 | +} |
| 49 | + |
| 50 | +pub(crate) trait BindInsteadOfMap { |
| 51 | + const TYPE_NAME: &'static str; |
| 52 | + const TYPE_QPATH: &'static [&'static str]; |
| 53 | + |
| 54 | + const BAD_METHOD_NAME: &'static str; |
| 55 | + const BAD_VARIANT_NAME: &'static str; |
| 56 | + const BAD_VARIANT_QPATH: &'static [&'static str]; |
| 57 | + |
| 58 | + const GOOD_METHOD_NAME: &'static str; |
| 59 | + |
| 60 | + fn no_op_msg() -> String { |
| 61 | + format!( |
| 62 | + "using `{}.{}({})`, which is a no-op", |
| 63 | + Self::TYPE_NAME, |
| 64 | + Self::BAD_METHOD_NAME, |
| 65 | + Self::BAD_VARIANT_NAME |
| 66 | + ) |
| 67 | + } |
| 68 | + |
| 69 | + fn lint_msg() -> String { |
| 70 | + format!( |
| 71 | + "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`", |
| 72 | + Self::TYPE_NAME, |
| 73 | + Self::BAD_METHOD_NAME, |
| 74 | + Self::BAD_VARIANT_NAME, |
| 75 | + Self::GOOD_METHOD_NAME |
| 76 | + ) |
| 77 | + } |
| 78 | + |
| 79 | + fn lint_closure_autofixable( |
| 80 | + cx: &LateContext<'_, '_>, |
| 81 | + expr: &hir::Expr<'_>, |
| 82 | + args: &[hir::Expr<'_>], |
| 83 | + closure_expr: &hir::Expr<'_>, |
| 84 | + closure_args_span: Span, |
| 85 | + ) -> bool { |
| 86 | + if_chain! { |
| 87 | + if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.kind; |
| 88 | + if let hir::ExprKind::Path(ref qpath) = some_expr.kind; |
| 89 | + if match_qpath(qpath, Self::BAD_VARIANT_QPATH); |
| 90 | + if some_args.len() == 1; |
| 91 | + then { |
| 92 | + let inner_expr = &some_args[0]; |
| 93 | + |
| 94 | + if contains_return(inner_expr) { |
| 95 | + return false; |
| 96 | + } |
| 97 | + |
| 98 | + let some_inner_snip = if inner_expr.span.from_expansion() { |
| 99 | + snippet_with_macro_callsite(cx, inner_expr.span, "_") |
| 100 | + } else { |
| 101 | + snippet(cx, inner_expr.span, "_") |
| 102 | + }; |
| 103 | + |
| 104 | + let closure_args_snip = snippet(cx, closure_args_span, ".."); |
| 105 | + let option_snip = snippet(cx, args[0].span, ".."); |
| 106 | + let note = format!("{}.{}({} {})", option_snip, Self::GOOD_METHOD_NAME, closure_args_snip, some_inner_snip); |
| 107 | + span_lint_and_sugg( |
| 108 | + cx, |
| 109 | + BIND_INSTEAD_OF_MAP, |
| 110 | + expr.span, |
| 111 | + Self::lint_msg().as_ref(), |
| 112 | + "try this", |
| 113 | + note, |
| 114 | + Applicability::MachineApplicable, |
| 115 | + ); |
| 116 | + true |
| 117 | + } else { |
| 118 | + false |
| 119 | + } |
| 120 | + } |
| 121 | + } |
| 122 | + |
| 123 | + fn lint_closure(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) { |
| 124 | + let mut suggs = Vec::new(); |
| 125 | + let can_sugg = find_all_ret_expressions(cx, closure_expr, |ret_expr| { |
| 126 | + if_chain! { |
| 127 | + if !in_macro(ret_expr.span); |
| 128 | + if let hir::ExprKind::Call(ref func_path, ref args) = ret_expr.kind; |
| 129 | + if let hir::ExprKind::Path(ref qpath) = func_path.kind; |
| 130 | + if match_qpath(qpath, Self::BAD_VARIANT_QPATH); |
| 131 | + if args.len() == 1; |
| 132 | + if !contains_return(&args[0]); |
| 133 | + then { |
| 134 | + suggs.push((ret_expr.span, args[0].span.source_callsite())); |
| 135 | + true |
| 136 | + } else { |
| 137 | + false |
| 138 | + } |
| 139 | + } |
| 140 | + }); |
| 141 | + |
| 142 | + if can_sugg { |
| 143 | + span_lint_and_then(cx, BIND_INSTEAD_OF_MAP, expr.span, Self::lint_msg().as_ref(), |diag| { |
| 144 | + multispan_sugg_with_applicability( |
| 145 | + diag, |
| 146 | + "try this", |
| 147 | + Applicability::MachineApplicable, |
| 148 | + std::iter::once((*method_calls(expr, 1).2.get(0).unwrap(), Self::GOOD_METHOD_NAME.into())).chain( |
| 149 | + suggs |
| 150 | + .into_iter() |
| 151 | + .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())), |
| 152 | + ), |
| 153 | + ) |
| 154 | + }); |
| 155 | + } |
| 156 | + } |
| 157 | + |
| 158 | + /// Lint use of `_.and_then(|x| Some(y))` for `Option`s |
| 159 | + fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { |
| 160 | + if !match_type(cx, cx.tables.expr_ty(&args[0]), Self::TYPE_QPATH) { |
| 161 | + return; |
| 162 | + } |
| 163 | + |
| 164 | + match args[1].kind { |
| 165 | + hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => { |
| 166 | + let closure_body = cx.tcx.hir().body(body_id); |
| 167 | + let closure_expr = remove_blocks(&closure_body.value); |
| 168 | + |
| 169 | + if !Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) { |
| 170 | + Self::lint_closure(cx, expr, closure_expr); |
| 171 | + } |
| 172 | + }, |
| 173 | + // `_.and_then(Some)` case, which is no-op. |
| 174 | + hir::ExprKind::Path(ref qpath) if match_qpath(qpath, Self::BAD_VARIANT_QPATH) => { |
| 175 | + span_lint_and_sugg( |
| 176 | + cx, |
| 177 | + BIND_INSTEAD_OF_MAP, |
| 178 | + expr.span, |
| 179 | + Self::no_op_msg().as_ref(), |
| 180 | + "use the expression directly", |
| 181 | + snippet(cx, args[0].span, "..").into(), |
| 182 | + Applicability::MachineApplicable, |
| 183 | + ); |
| 184 | + }, |
| 185 | + _ => {}, |
| 186 | + } |
| 187 | + } |
| 188 | +} |
| 189 | + |
| 190 | +/// returns `true` if expr contains match expr desugared from try |
| 191 | +fn contains_try(expr: &hir::Expr<'_>) -> bool { |
| 192 | + struct TryFinder { |
| 193 | + found: bool, |
| 194 | + } |
| 195 | + |
| 196 | + impl<'hir> intravisit::Visitor<'hir> for TryFinder { |
| 197 | + type Map = Map<'hir>; |
| 198 | + |
| 199 | + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { |
| 200 | + intravisit::NestedVisitorMap::None |
| 201 | + } |
| 202 | + |
| 203 | + fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) { |
| 204 | + if self.found { |
| 205 | + return; |
| 206 | + } |
| 207 | + match expr.kind { |
| 208 | + hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true, |
| 209 | + _ => intravisit::walk_expr(self, expr), |
| 210 | + } |
| 211 | + } |
| 212 | + } |
| 213 | + |
| 214 | + let mut visitor = TryFinder { found: false }; |
| 215 | + visitor.visit_expr(expr); |
| 216 | + visitor.found |
| 217 | +} |
| 218 | + |
| 219 | +fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_, '_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool |
| 220 | +where |
| 221 | + F: FnMut(&'hir hir::Expr<'hir>) -> bool, |
| 222 | +{ |
| 223 | + struct RetFinder<F> { |
| 224 | + in_stmt: bool, |
| 225 | + failed: bool, |
| 226 | + cb: F, |
| 227 | + } |
| 228 | + |
| 229 | + struct WithStmtGuarg<'a, F> { |
| 230 | + val: &'a mut RetFinder<F>, |
| 231 | + prev_in_stmt: bool, |
| 232 | + } |
| 233 | + |
| 234 | + impl<F> RetFinder<F> { |
| 235 | + fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { |
| 236 | + let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); |
| 237 | + WithStmtGuarg { |
| 238 | + val: self, |
| 239 | + prev_in_stmt, |
| 240 | + } |
| 241 | + } |
| 242 | + } |
| 243 | + |
| 244 | + impl<F> std::ops::Deref for WithStmtGuarg<'_, F> { |
| 245 | + type Target = RetFinder<F>; |
| 246 | + |
| 247 | + fn deref(&self) -> &Self::Target { |
| 248 | + self.val |
| 249 | + } |
| 250 | + } |
| 251 | + |
| 252 | + impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> { |
| 253 | + fn deref_mut(&mut self) -> &mut Self::Target { |
| 254 | + self.val |
| 255 | + } |
| 256 | + } |
| 257 | + |
| 258 | + impl<F> Drop for WithStmtGuarg<'_, F> { |
| 259 | + fn drop(&mut self) { |
| 260 | + self.val.in_stmt = self.prev_in_stmt; |
| 261 | + } |
| 262 | + } |
| 263 | + |
| 264 | + impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> { |
| 265 | + type Map = Map<'hir>; |
| 266 | + |
| 267 | + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { |
| 268 | + intravisit::NestedVisitorMap::None |
| 269 | + } |
| 270 | + |
| 271 | + fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) { |
| 272 | + intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) |
| 273 | + } |
| 274 | + |
| 275 | + fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) { |
| 276 | + if self.failed { |
| 277 | + return; |
| 278 | + } |
| 279 | + if self.in_stmt { |
| 280 | + match expr.kind { |
| 281 | + hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), |
| 282 | + _ => intravisit::walk_expr(self, expr), |
| 283 | + } |
| 284 | + } else { |
| 285 | + match expr.kind { |
| 286 | + hir::ExprKind::Match(cond, arms, _) => { |
| 287 | + self.inside_stmt(true).visit_expr(cond); |
| 288 | + for arm in arms { |
| 289 | + self.visit_expr(arm.body); |
| 290 | + } |
| 291 | + }, |
| 292 | + hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr), |
| 293 | + hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr), |
| 294 | + _ => self.failed |= !(self.cb)(expr), |
| 295 | + } |
| 296 | + } |
| 297 | + } |
| 298 | + } |
| 299 | + |
| 300 | + !contains_try(expr) && { |
| 301 | + let mut ret_finder = RetFinder { |
| 302 | + in_stmt: false, |
| 303 | + failed: false, |
| 304 | + cb: callback, |
| 305 | + }; |
| 306 | + ret_finder.visit_expr(expr); |
| 307 | + !ret_finder.failed |
| 308 | + } |
| 309 | +} |
0 commit comments