ast/hir: Rename field-related structures

[rust.git] / clippy_lints / src / option_if_let_else.rs

diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
index 4a3eb9c983a11617114d4231a2b58e74133b1279..9ef0d267b0b20b49b691ae5bfdf49c637fb357c8 100644 (file)
--- a/clippy_lints/src/option_if_let_else.rs
+++ b/clippy_lints/src/option_if_let_else.rs
@@ -5,22 +5,21 @@
 use if_chain::if_chain;
 
 use rustc_errors::Applicability;
-use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, PatKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
 
 declare_clippy_lint! {
     /// **What it does:**
-    /// Lints usage of  `if let Some(v) = ... { y } else { x }` which is more
+    /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more
     /// idiomatically done with `Option::map_or` (if the else bit is a pure
     /// expression) or `Option::map_or_else` (if the else bit is an impure
-    /// expresion).
+    /// expression).
     ///
     /// **Why is this bad?**
     /// Using the dedicated functions of the Option type is clearer and
-    /// more concise than an if let expression.
+    /// more concise than an `if let` expression.
     ///
     /// **Known problems:**
     /// This lint uses a deliberately conservative metric for checking
@@ -67,8 +66,8 @@
 /// Returns true iff the given expression is the result of calling `Result::ok`
 fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
     if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
-        path.ident.name.to_ident_string() == "ok"
-            && is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym!(result_type))
+        path.ident.name.as_str() == "ok"
+            && is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym::result_type)
     } else {
         false
     }
@@ -84,53 +83,6 @@ struct OptionIfLetElseOccurence {
     wrap_braces: bool,
 }
 
-struct ReturnBreakContinueMacroVisitor {
-    seen_return_break_continue: bool,
-}
-
-impl ReturnBreakContinueMacroVisitor {
-    fn new() -> ReturnBreakContinueMacroVisitor {
-        ReturnBreakContinueMacroVisitor {
-            seen_return_break_continue: false,
-        }
-    }
-}
-
-impl<'tcx> Visitor<'tcx> for ReturnBreakContinueMacroVisitor {
-    type Map = Map<'tcx>;
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
-
-    fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
-        if self.seen_return_break_continue {
-            // No need to look farther if we've already seen one of them
-            return;
-        }
-        match &ex.kind {
-            ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => {
-                self.seen_return_break_continue = true;
-            },
-            // Something special could be done here to handle while or for loop
-            // desugaring, as this will detect a break if there's a while loop
-            // or a for loop inside the expression.
-            _ => {
-                if utils::in_macro(ex.span) {
-                    self.seen_return_break_continue = true;
-                } else {
-                    rustc_hir::intravisit::walk_expr(self, ex);
-                }
-            },
-        }
-    }
-}
-
-fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
-    let mut recursive_visitor = ReturnBreakContinueMacroVisitor::new();
-    recursive_visitor.visit_expr(expression);
-    recursive_visitor.seen_return_break_continue
-}
-
 /// Extracts the body of a given arm. If the arm contains only an expression,
 /// then it returns the expression. Otherwise, it returns the entire block
 fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> {
@@ -157,25 +109,30 @@ fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> {
 /// it in curly braces. Otherwise, we don't.
 fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     utils::get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| {
+        let mut should_wrap = false;
+
         if let Some(Expr {
             kind:
                 ExprKind::Match(
                     _,
                     arms,
-                    MatchSource::IfDesugar {
-                        contains_else_clause: true,
-                    }
-                    | MatchSource::IfLetDesugar {
+                    MatchSource::IfLetDesugar {
                         contains_else_clause: true,
                     },
                 ),
             ..
         }) = parent.expr
         {
-            expr.hir_id == arms[1].body.hir_id
-        } else {
-            false
+            should_wrap = expr.hir_id == arms[1].body.hir_id;
+        } else if let Some(Expr {
+            kind: ExprKind::If(_, _, Some(else_clause)),
+            ..
+        }) = parent.expr
+        {
+            should_wrap = expr.hir_id == else_clause.hir_id;
         }
+
+        should_wrap
     })
 }
 
@@ -208,8 +165,8 @@ fn detect_option_if_let_else<'tcx>(
         if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;
         if utils::match_qpath(struct_qpath, &paths::OPTION_SOME);
         if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind;
-        if !contains_return_break_continue_macro(arms[0].body);
-        if !contains_return_break_continue_macro(arms[1].body);
+        if !utils::usage::contains_return_break_continue_macro(arms[0].body);
+        if !utils::usage::contains_return_break_continue_macro(arms[1].body);
         then {
             let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
             let some_body = extract_body_from_arm(&arms[0])?;
@@ -224,7 +181,7 @@ fn detect_option_if_let_else<'tcx>(
             };
             let cond_expr = match &cond_expr.kind {
                 // Pointer dereferencing happens automatically, so we can omit it in the suggestion
-                ExprKind::Unary(UnOp::UnDeref, expr) | ExprKind::AddrOf(_, _, expr) => expr,
+                ExprKind::Unary(UnOp::Deref, expr) | ExprKind::AddrOf(_, _, expr) => expr,
                 _ => cond_expr,
             };
             Some(OptionIfLetElseOccurence {