|
1 | 1 | use clippy_utils::diagnostics::span_lint_hir_and_then; |
2 | 2 | use clippy_utils::res::MaybeResPath; |
3 | | -use clippy_utils::source::snippet; |
| 3 | +use clippy_utils::source::snippet_with_context; |
4 | 4 | use clippy_utils::visitors::is_local_used; |
5 | 5 | use rustc_errors::Applicability; |
6 | 6 | use rustc_hir as hir; |
@@ -59,80 +59,88 @@ declare_lint_pass!(LetIfSeq => [USELESS_LET_IF_SEQ]); |
59 | 59 | impl<'tcx> LateLintPass<'tcx> for LetIfSeq { |
60 | 60 | fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { |
61 | 61 | for [stmt, next] in block.stmts.array_windows::<2>() { |
62 | | - if let hir::StmtKind::Let(local) = stmt.kind |
63 | | - && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind |
64 | | - && let hir::StmtKind::Expr(if_) = next.kind |
65 | | - && let hir::ExprKind::If(cond, then, else_) = if_.kind |
66 | | - && !is_local_used(cx, cond, canonical_id) |
67 | | - && let hir::ExprKind::Block(then, _) = then.kind |
68 | | - && let Some(value) = check_assign(cx, canonical_id, then) |
69 | | - && !is_local_used(cx, value, canonical_id) |
70 | | - { |
71 | | - let span = stmt.span.to(if_.span); |
| 62 | + if let hir::StmtKind::Expr(if_) = next.kind { |
| 63 | + check_block_inner(cx, stmt, if_); |
| 64 | + } |
| 65 | + } |
72 | 66 |
|
73 | | - let has_interior_mutability = !cx |
74 | | - .typeck_results() |
75 | | - .node_type(canonical_id) |
76 | | - .is_freeze(cx.tcx, cx.typing_env()); |
77 | | - if has_interior_mutability { |
78 | | - return; |
79 | | - } |
| 67 | + if let Some(expr) = block.expr |
| 68 | + && let Some(stmt) = block.stmts.last() |
| 69 | + { |
| 70 | + check_block_inner(cx, stmt, expr); |
| 71 | + } |
| 72 | + } |
| 73 | +} |
| 74 | + |
| 75 | +fn check_block_inner<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>, if_: &'tcx hir::Expr<'tcx>) { |
| 76 | + if let hir::StmtKind::Let(local) = stmt.kind |
| 77 | + && let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind |
| 78 | + && let hir::ExprKind::If(cond, then, else_) = if_.kind |
| 79 | + && !is_local_used(cx, cond, canonical_id) |
| 80 | + && let hir::ExprKind::Block(then, _) = then.kind |
| 81 | + && let Some(value) = check_assign(cx, canonical_id, then) |
| 82 | + && !is_local_used(cx, value, canonical_id) |
| 83 | + { |
| 84 | + let span = stmt.span.to(if_.span); |
80 | 85 |
|
81 | | - let (default_multi_stmts, default) = if let Some(else_) = else_ { |
82 | | - if let hir::ExprKind::Block(else_, _) = else_.kind { |
83 | | - if let Some(default) = check_assign(cx, canonical_id, else_) { |
84 | | - (else_.stmts.len() > 1, default) |
85 | | - } else if let Some(default) = local.init { |
86 | | - (true, default) |
87 | | - } else { |
88 | | - continue; |
89 | | - } |
90 | | - } else { |
91 | | - continue; |
92 | | - } |
| 86 | + let has_interior_mutability = !cx |
| 87 | + .typeck_results() |
| 88 | + .node_type(canonical_id) |
| 89 | + .is_freeze(cx.tcx, cx.typing_env()); |
| 90 | + if has_interior_mutability { |
| 91 | + return; |
| 92 | + } |
| 93 | + |
| 94 | + let (default_multi_stmts, default) = if let Some(else_) = else_ { |
| 95 | + if let hir::ExprKind::Block(else_, _) = else_.kind { |
| 96 | + if let Some(default) = check_assign(cx, canonical_id, else_) { |
| 97 | + (else_.stmts.len() > 1, default) |
93 | 98 | } else if let Some(default) = local.init { |
94 | | - (false, default) |
| 99 | + (true, default) |
95 | 100 | } else { |
96 | | - continue; |
97 | | - }; |
| 101 | + return; |
| 102 | + } |
| 103 | + } else { |
| 104 | + return; |
| 105 | + } |
| 106 | + } else if let Some(default) = local.init { |
| 107 | + (false, default) |
| 108 | + } else { |
| 109 | + return; |
| 110 | + }; |
98 | 111 |
|
99 | | - let mutability = match mode { |
100 | | - BindingMode(_, Mutability::Mut) => "<mut> ", |
101 | | - _ => "", |
102 | | - }; |
| 112 | + let mutability = match mode { |
| 113 | + BindingMode(_, Mutability::Mut) => "<mut> ", |
| 114 | + _ => "", |
| 115 | + }; |
103 | 116 |
|
104 | | - // FIXME: this should not suggest `mut` if we can detect that the variable is not |
105 | | - // use mutably after the `if` |
| 117 | + // FIXME: this should not suggest `mut` if we can detect that the variable is not |
| 118 | + // use mutably after the `if` |
106 | 119 |
|
107 | | - let sug = format!( |
108 | | - "let {mutability}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};", |
109 | | - name=ident.name, |
110 | | - cond=snippet(cx, cond.span, "_"), |
111 | | - then=if then.stmts.len() > 1 { " ..;" } else { "" }, |
112 | | - else=if default_multi_stmts { " ..;" } else { "" }, |
113 | | - value=snippet(cx, value.span, "<value>"), |
114 | | - default=snippet(cx, default.span, "<default>"), |
115 | | - ); |
116 | | - span_lint_hir_and_then( |
117 | | - cx, |
118 | | - USELESS_LET_IF_SEQ, |
119 | | - local.hir_id, |
120 | | - span, |
121 | | - "`if _ { .. } else { .. }` is an expression", |
122 | | - |diag| { |
123 | | - diag.span_suggestion( |
124 | | - span, |
125 | | - "it is more idiomatic to write", |
126 | | - sug, |
127 | | - Applicability::HasPlaceholders, |
128 | | - ); |
129 | | - if !mutability.is_empty() { |
130 | | - diag.note("you might not need `mut` at all"); |
131 | | - } |
132 | | - }, |
133 | | - ); |
134 | | - } |
135 | | - } |
| 120 | + let mut applicability = Applicability::HasPlaceholders; |
| 121 | + let (cond_snip, _) = snippet_with_context(cx, cond.span, if_.span.ctxt(), "_", &mut applicability); |
| 122 | + let (value_snip, _) = snippet_with_context(cx, value.span, if_.span.ctxt(), "<value>", &mut applicability); |
| 123 | + let (default_snip, _) = |
| 124 | + snippet_with_context(cx, default.span, if_.span.ctxt(), "<default>", &mut applicability); |
| 125 | + let sug = format!( |
| 126 | + "let {mutability}{name} = if {cond_snip} {{{then} {value_snip} }} else {{{else} {default_snip} }};", |
| 127 | + name=ident.name, |
| 128 | + then=if then.stmts.len() > 1 { " ..;" } else { "" }, |
| 129 | + else=if default_multi_stmts { " ..;" } else { "" }, |
| 130 | + ); |
| 131 | + span_lint_hir_and_then( |
| 132 | + cx, |
| 133 | + USELESS_LET_IF_SEQ, |
| 134 | + local.hir_id, |
| 135 | + span, |
| 136 | + "`if _ { .. } else { .. }` is an expression", |
| 137 | + |diag| { |
| 138 | + diag.span_suggestion(span, "it is more idiomatic to write", sug, applicability); |
| 139 | + if !mutability.is_empty() { |
| 140 | + diag.note("you might not need `mut` at all"); |
| 141 | + } |
| 142 | + }, |
| 143 | + ); |
136 | 144 | } |
137 | 145 | } |
138 | 146 |
|
|
0 commit comments