Start uplifting clippy::for_loops_over_fallibles
I refactored the code:
- Removed handling of methods, as it felt entirely unnecessary
- Removed clippy utils (obviously...)
- Used some shiny compiler features
(let-else is very handy for lints 👀)
- I also renamed the lint to `for_loop_over_fallibles` (note: no `s`).
I'm not sure what's the naming convention here, so maybe I'm wrong.
This commit is contained in:
parent
c0784109da
commit
fa380a82a5
2 changed files with 102 additions and 0 deletions
99
compiler/rustc_lint/src/for_loop_over_fallibles.rs
Normal file
99
compiler/rustc_lint/src/for_loop_over_fallibles.rs
Normal file
|
@ -0,0 +1,99 @@
|
||||||
|
use crate::{LateContext, LateLintPass, LintContext};
|
||||||
|
|
||||||
|
use hir::{Expr, Pat};
|
||||||
|
use rustc_hir as hir;
|
||||||
|
use rustc_middle::ty;
|
||||||
|
use rustc_span::sym;
|
||||||
|
|
||||||
|
declare_lint! {
|
||||||
|
/// ### What it does
|
||||||
|
///
|
||||||
|
/// Checks for `for` loops over `Option` or `Result` values.
|
||||||
|
///
|
||||||
|
/// ### Why is this bad?
|
||||||
|
/// Readability. This is more clearly expressed as an `if
|
||||||
|
/// let`.
|
||||||
|
///
|
||||||
|
/// ### Example
|
||||||
|
///
|
||||||
|
/// ```rust
|
||||||
|
/// # let opt = Some(1);
|
||||||
|
/// # let res: Result<i32, std::io::Error> = Ok(1);
|
||||||
|
/// for x in opt {
|
||||||
|
/// // ..
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// for x in &res {
|
||||||
|
/// // ..
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// for x in res.iter() {
|
||||||
|
/// // ..
|
||||||
|
/// }
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```rust
|
||||||
|
/// # let opt = Some(1);
|
||||||
|
/// # let res: Result<i32, std::io::Error> = Ok(1);
|
||||||
|
/// if let Some(x) = opt {
|
||||||
|
/// // ..
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// if let Ok(x) = res {
|
||||||
|
/// // ..
|
||||||
|
/// }
|
||||||
|
/// ```
|
||||||
|
pub FOR_LOOP_OVER_FALLIBLES,
|
||||||
|
Warn,
|
||||||
|
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
|
||||||
|
}
|
||||||
|
|
||||||
|
declare_lint_pass!(ForLoopOverFallibles => [FOR_LOOP_OVER_FALLIBLES]);
|
||||||
|
|
||||||
|
impl<'tcx> LateLintPass<'tcx> for ForLoopOverFallibles {
|
||||||
|
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||||
|
let Some((pat, arg)) = extract_for_loop(expr) else { return };
|
||||||
|
|
||||||
|
let ty = cx.typeck_results().expr_ty(arg);
|
||||||
|
|
||||||
|
let ty::Adt(adt, _) = ty.kind() else { return };
|
||||||
|
|
||||||
|
let (article, ty, var) = match adt.did() {
|
||||||
|
did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
|
||||||
|
did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
|
||||||
|
_ => return,
|
||||||
|
};
|
||||||
|
|
||||||
|
let Ok(pat_snip) = cx.sess().source_map().span_to_snippet(pat.span) else { return };
|
||||||
|
let Ok(arg_snip) = cx.sess().source_map().span_to_snippet(arg.span) else { return };
|
||||||
|
|
||||||
|
let help_string = format!(
|
||||||
|
"consider replacing `for {pat_snip} in {arg_snip}` with `if let {var}({pat_snip}) = {arg_snip}`"
|
||||||
|
);
|
||||||
|
let msg = format!(
|
||||||
|
"for loop over `{arg_snip}`, which is {article} `{ty}`. This is more readably written as an `if let` statement",
|
||||||
|
);
|
||||||
|
|
||||||
|
cx.struct_span_lint(FOR_LOOP_OVER_FALLIBLES, arg.span, |diag| {
|
||||||
|
diag.build(msg).help(help_string).emit()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
|
||||||
|
if let hir::ExprKind::DropTemps(e) = expr.kind
|
||||||
|
&& let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
|
||||||
|
&& let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
|
||||||
|
&& let hir::ExprKind::Loop(block, ..) = arm.body.kind
|
||||||
|
&& let [stmt] = block.stmts
|
||||||
|
&& let hir::StmtKind::Expr(e) = stmt.kind
|
||||||
|
&& let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
|
||||||
|
&& let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
|
||||||
|
{
|
||||||
|
Some((field.pat, arg))
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -52,6 +52,7 @@ mod early;
|
||||||
mod enum_intrinsics_non_enums;
|
mod enum_intrinsics_non_enums;
|
||||||
mod errors;
|
mod errors;
|
||||||
mod expect;
|
mod expect;
|
||||||
|
mod for_loop_over_fallibles;
|
||||||
pub mod hidden_unicode_codepoints;
|
pub mod hidden_unicode_codepoints;
|
||||||
mod internal;
|
mod internal;
|
||||||
mod late;
|
mod late;
|
||||||
|
@ -86,6 +87,7 @@ use rustc_span::Span;
|
||||||
use array_into_iter::ArrayIntoIter;
|
use array_into_iter::ArrayIntoIter;
|
||||||
use builtin::*;
|
use builtin::*;
|
||||||
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
|
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
|
||||||
|
use for_loop_over_fallibles::*;
|
||||||
use hidden_unicode_codepoints::*;
|
use hidden_unicode_codepoints::*;
|
||||||
use internal::*;
|
use internal::*;
|
||||||
use let_underscore::*;
|
use let_underscore::*;
|
||||||
|
@ -188,6 +190,7 @@ macro_rules! late_lint_mod_passes {
|
||||||
$macro!(
|
$macro!(
|
||||||
$args,
|
$args,
|
||||||
[
|
[
|
||||||
|
ForLoopOverFallibles: ForLoopOverFallibles,
|
||||||
HardwiredLints: HardwiredLints,
|
HardwiredLints: HardwiredLints,
|
||||||
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
|
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
|
||||||
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
|
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue