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
/// 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
}
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>> {
/// 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
})
}
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])?;
};
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 {