|
| 1 | +use clippy_utils::diagnostics::span_lint_and_then; |
| 2 | +use clippy_utils::higher::IfLetOrMatch; |
| 3 | +use clippy_utils::source::snippet_opt; |
| 4 | +use clippy_utils::ty::is_type_diagnostic_item; |
| 5 | +use clippy_utils::visitors::{for_each_expr, Descend}; |
| 6 | +use clippy_utils::{meets_msrv, msrvs, peel_blocks}; |
| 7 | +use if_chain::if_chain; |
| 8 | +use rustc_data_structures::fx::FxHashSet; |
| 9 | +use rustc_errors::Applicability; |
| 10 | +use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, Stmt, StmtKind}; |
| 11 | +use rustc_lint::{LateContext, LateLintPass, LintContext}; |
| 12 | +use rustc_middle::lint::in_external_macro; |
| 13 | +use rustc_semver::RustcVersion; |
| 14 | +use rustc_session::{declare_tool_lint, impl_lint_pass}; |
| 15 | +use rustc_span::symbol::sym; |
| 16 | +use rustc_span::Span; |
| 17 | +use serde::Deserialize; |
| 18 | +use std::ops::ControlFlow; |
| 19 | + |
| 20 | +declare_clippy_lint! { |
| 21 | + /// ### What it does |
| 22 | + /// |
| 23 | + /// Warn of cases where `let...else` could be used |
| 24 | + /// |
| 25 | + /// ### Why is this bad? |
| 26 | + /// |
| 27 | + /// `let...else` provides a standard construct for this pattern |
| 28 | + /// that people can easily recognize. It's also more compact. |
| 29 | + /// |
| 30 | + /// ### Example |
| 31 | + /// |
| 32 | + /// ```rust |
| 33 | + /// # let w = Some(0); |
| 34 | + /// let v = if let Some(v) = w { v } else { return }; |
| 35 | + /// ``` |
| 36 | + /// |
| 37 | + /// Could be written: |
| 38 | + /// |
| 39 | + /// ```rust |
| 40 | + /// # #![feature(let_else)] |
| 41 | + /// # fn main () { |
| 42 | + /// # let w = Some(0); |
| 43 | + /// let Some(v) = w else { return }; |
| 44 | + /// # } |
| 45 | + /// ``` |
| 46 | + #[clippy::version = "1.67.0"] |
| 47 | + pub MANUAL_LET_ELSE, |
| 48 | + pedantic, |
| 49 | + "manual implementation of a let...else statement" |
| 50 | +} |
| 51 | + |
| 52 | +pub struct ManualLetElse { |
| 53 | + msrv: Option<RustcVersion>, |
| 54 | + matches_behaviour: MatchLintBehaviour, |
| 55 | +} |
| 56 | + |
| 57 | +impl ManualLetElse { |
| 58 | + #[must_use] |
| 59 | + pub fn new(msrv: Option<RustcVersion>, matches_behaviour: MatchLintBehaviour) -> Self { |
| 60 | + Self { |
| 61 | + msrv, |
| 62 | + matches_behaviour, |
| 63 | + } |
| 64 | + } |
| 65 | +} |
| 66 | + |
| 67 | +impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]); |
| 68 | + |
| 69 | +impl<'tcx> LateLintPass<'tcx> for ManualLetElse { |
| 70 | + fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) { |
| 71 | + let if_let_or_match = if_chain! { |
| 72 | + if meets_msrv(self.msrv, msrvs::LET_ELSE); |
| 73 | + if !in_external_macro(cx.sess(), stmt.span); |
| 74 | + if let StmtKind::Local(local) = stmt.kind; |
| 75 | + if let Some(init) = local.init; |
| 76 | + if local.els.is_none(); |
| 77 | + if local.ty.is_none(); |
| 78 | + if init.span.ctxt() == stmt.span.ctxt(); |
| 79 | + if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init); |
| 80 | + then { |
| 81 | + if_let_or_match |
| 82 | + } else { |
| 83 | + return; |
| 84 | + } |
| 85 | + }; |
| 86 | + |
| 87 | + match if_let_or_match { |
| 88 | + IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! { |
| 89 | + if expr_is_simple_identity(let_pat, if_then); |
| 90 | + if let Some(if_else) = if_else; |
| 91 | + if expr_diverges(cx, if_else); |
| 92 | + then { |
| 93 | + emit_manual_let_else(cx, stmt.span, if_let_expr, let_pat, if_else); |
| 94 | + } |
| 95 | + }, |
| 96 | + IfLetOrMatch::Match(match_expr, arms, source) => { |
| 97 | + if self.matches_behaviour == MatchLintBehaviour::Never { |
| 98 | + return; |
| 99 | + } |
| 100 | + if source != MatchSource::Normal { |
| 101 | + return; |
| 102 | + } |
| 103 | + // Any other number than two arms doesn't (neccessarily) |
| 104 | + // have a trivial mapping to let else. |
| 105 | + if arms.len() != 2 { |
| 106 | + return; |
| 107 | + } |
| 108 | + // Guards don't give us an easy mapping either |
| 109 | + if arms.iter().any(|arm| arm.guard.is_some()) { |
| 110 | + return; |
| 111 | + } |
| 112 | + let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes; |
| 113 | + let diverging_arm_opt = arms |
| 114 | + .iter() |
| 115 | + .enumerate() |
| 116 | + .find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types)); |
| 117 | + let Some((idx, diverging_arm)) = diverging_arm_opt else { return; }; |
| 118 | + let pat_arm = &arms[1 - idx]; |
| 119 | + if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) { |
| 120 | + return; |
| 121 | + } |
| 122 | + |
| 123 | + emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body); |
| 124 | + }, |
| 125 | + } |
| 126 | + } |
| 127 | + |
| 128 | + extract_msrv_attr!(LateContext); |
| 129 | +} |
| 130 | + |
| 131 | +fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: &Pat<'_>, else_body: &Expr<'_>) { |
| 132 | + span_lint_and_then( |
| 133 | + cx, |
| 134 | + MANUAL_LET_ELSE, |
| 135 | + span, |
| 136 | + "this could be rewritten as `let...else`", |
| 137 | + |diag| { |
| 138 | + // This is far from perfect, for example there needs to be: |
| 139 | + // * mut additions for the bindings |
| 140 | + // * renamings of the bindings |
| 141 | + // * unused binding collision detection with existing ones |
| 142 | + // * putting patterns with at the top level | inside () |
| 143 | + // for this to be machine applicable. |
| 144 | + let app = Applicability::HasPlaceholders; |
| 145 | + |
| 146 | + if let Some(sn_pat) = snippet_opt(cx, pat.span) && |
| 147 | + let Some(sn_expr) = snippet_opt(cx, expr.span) && |
| 148 | + let Some(sn_else) = snippet_opt(cx, else_body.span) |
| 149 | + { |
| 150 | + let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) { |
| 151 | + sn_else |
| 152 | + } else { |
| 153 | + format!("{{ {sn_else} }}") |
| 154 | + }; |
| 155 | + let sugg = format!("let {sn_pat} = {sn_expr} else {else_bl};"); |
| 156 | + diag.span_suggestion(span, "consider writing", sugg, app); |
| 157 | + } |
| 158 | + }, |
| 159 | + ); |
| 160 | +} |
| 161 | + |
| 162 | +fn expr_diverges(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool { |
| 163 | + fn is_never(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool { |
| 164 | + if let Some(ty) = cx.typeck_results().expr_ty_opt(expr) { |
| 165 | + return ty.is_never(); |
| 166 | + } |
| 167 | + false |
| 168 | + } |
| 169 | + // We can't just call is_never on expr and be done, because the type system |
| 170 | + // sometimes coerces the ! type to something different before we can get |
| 171 | + // our hands on it. So instead, we do a manual search. We do fall back to |
| 172 | + // is_never in some places when there is no better alternative. |
| 173 | + for_each_expr(expr, |ex| { |
| 174 | + match ex.kind { |
| 175 | + ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => ControlFlow::Break(()), |
| 176 | + ExprKind::Call(call, _) => { |
| 177 | + if is_never(cx, ex) || is_never(cx, call) { |
| 178 | + return ControlFlow::Break(()); |
| 179 | + } |
| 180 | + ControlFlow::Continue(Descend::Yes) |
| 181 | + }, |
| 182 | + ExprKind::MethodCall(..) => { |
| 183 | + if is_never(cx, ex) { |
| 184 | + return ControlFlow::Break(()); |
| 185 | + } |
| 186 | + ControlFlow::Continue(Descend::Yes) |
| 187 | + }, |
| 188 | + ExprKind::If(if_expr, if_then, if_else) => { |
| 189 | + let else_diverges = if_else.map_or(false, |ex| expr_diverges(cx, ex)); |
| 190 | + let diverges = expr_diverges(cx, if_expr) || (else_diverges && expr_diverges(cx, if_then)); |
| 191 | + if diverges { |
| 192 | + return ControlFlow::Break(()); |
| 193 | + } |
| 194 | + ControlFlow::Continue(Descend::No) |
| 195 | + }, |
| 196 | + ExprKind::Match(match_expr, match_arms, _) => { |
| 197 | + let diverges = expr_diverges(cx, match_expr) |
| 198 | + || match_arms.iter().all(|arm| { |
| 199 | + let guard_diverges = arm.guard.as_ref().map_or(false, |g| expr_diverges(cx, g.body())); |
| 200 | + guard_diverges || expr_diverges(cx, arm.body) |
| 201 | + }); |
| 202 | + if diverges { |
| 203 | + return ControlFlow::Break(()); |
| 204 | + } |
| 205 | + ControlFlow::Continue(Descend::No) |
| 206 | + }, |
| 207 | + |
| 208 | + // Don't continue into loops or labeled blocks, as they are breakable, |
| 209 | + // and we'd have to start checking labels. |
| 210 | + ExprKind::Block(_, Some(_)) | ExprKind::Loop(..) => ControlFlow::Continue(Descend::No), |
| 211 | + |
| 212 | + // Default: descend |
| 213 | + _ => ControlFlow::Continue(Descend::Yes), |
| 214 | + } |
| 215 | + }) |
| 216 | + .is_some() |
| 217 | +} |
| 218 | + |
| 219 | +fn pat_allowed_for_else(cx: &LateContext<'_>, pat: &'_ Pat<'_>, check_types: bool) -> bool { |
| 220 | + // Check whether the pattern contains any bindings, as the |
| 221 | + // binding might potentially be used in the body. |
| 222 | + // TODO: only look for *used* bindings. |
| 223 | + let mut has_bindings = false; |
| 224 | + pat.each_binding_or_first(&mut |_, _, _, _| has_bindings = true); |
| 225 | + if has_bindings { |
| 226 | + return false; |
| 227 | + } |
| 228 | + |
| 229 | + // If we shouldn't check the types, exit early. |
| 230 | + if !check_types { |
| 231 | + return true; |
| 232 | + } |
| 233 | + |
| 234 | + // Check whether any possibly "unknown" patterns are included, |
| 235 | + // because users might not know which values some enum has. |
| 236 | + // Well-known enums are excepted, as we assume people know them. |
| 237 | + // We do a deep check, to be able to disallow Err(En::Foo(_)) |
| 238 | + // for usage of the En::Foo variant, as we disallow En::Foo(_), |
| 239 | + // but we allow Err(_). |
| 240 | + let typeck_results = cx.typeck_results(); |
| 241 | + let mut has_disallowed = false; |
| 242 | + pat.walk_always(|pat| { |
| 243 | + // Only do the check if the type is "spelled out" in the pattern |
| 244 | + if !matches!( |
| 245 | + pat.kind, |
| 246 | + PatKind::Struct(..) | PatKind::TupleStruct(..) | PatKind::Path(..) |
| 247 | + ) { |
| 248 | + return; |
| 249 | + }; |
| 250 | + let ty = typeck_results.pat_ty(pat); |
| 251 | + // Option and Result are allowed, everything else isn't. |
| 252 | + if !(is_type_diagnostic_item(cx, ty, sym::Option) || is_type_diagnostic_item(cx, ty, sym::Result)) { |
| 253 | + has_disallowed = true; |
| 254 | + } |
| 255 | + }); |
| 256 | + !has_disallowed |
| 257 | +} |
| 258 | + |
| 259 | +/// Checks if the passed block is a simple identity referring to bindings created by the pattern |
| 260 | +fn expr_is_simple_identity(pat: &'_ Pat<'_>, expr: &'_ Expr<'_>) -> bool { |
| 261 | + // We support patterns with multiple bindings and tuples, like: |
| 262 | + // let ... = if let (Some(foo), bar) = g() { (foo, bar) } else { ... } |
| 263 | + let peeled = peel_blocks(expr); |
| 264 | + let paths = match peeled.kind { |
| 265 | + ExprKind::Tup(exprs) | ExprKind::Array(exprs) => exprs, |
| 266 | + ExprKind::Path(_) => std::slice::from_ref(peeled), |
| 267 | + _ => return false, |
| 268 | + }; |
| 269 | + let mut pat_bindings = FxHashSet::default(); |
| 270 | + pat.each_binding_or_first(&mut |_ann, _hir_id, _sp, ident| { |
| 271 | + pat_bindings.insert(ident); |
| 272 | + }); |
| 273 | + if pat_bindings.len() < paths.len() { |
| 274 | + return false; |
| 275 | + } |
| 276 | + for path in paths { |
| 277 | + if_chain! { |
| 278 | + if let ExprKind::Path(QPath::Resolved(_ty, path)) = path.kind; |
| 279 | + if let [path_seg] = path.segments; |
| 280 | + then { |
| 281 | + if !pat_bindings.remove(&path_seg.ident) { |
| 282 | + return false; |
| 283 | + } |
| 284 | + } else { |
| 285 | + return false; |
| 286 | + } |
| 287 | + } |
| 288 | + } |
| 289 | + true |
| 290 | +} |
| 291 | + |
| 292 | +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Deserialize)] |
| 293 | +pub enum MatchLintBehaviour { |
| 294 | + AllTypes, |
| 295 | + WellKnownTypes, |
| 296 | + Never, |
| 297 | +} |
0 commit comments