Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d45f6db
Move some tests to more reasonable directories
c410-f3r Sep 20, 2022
8b9a1f1
Remove tuple candidate, nothing special about it
compiler-errors Oct 7, 2022
293f662
Make tests capture the error printed by a Result return
dtolnay Oct 7, 2022
fa380a8
Start uplifting `clippy::for_loops_over_fallibles`
WaffleLapkin Jul 17, 2022
d030ba5
Use structured suggestions for `for_loop_over_fallibles` lint
WaffleLapkin Jul 18, 2022
21ec99b
`for_loop_over_fallibles`: Suggest removing `.next()`
WaffleLapkin Jul 24, 2022
5dcfdbf
`for_loop_over_fallibles`: suggest `while let` loop
WaffleLapkin Jul 24, 2022
b2975ee
`for_loop_over_fallibles`: suggest using `?` in some cases
WaffleLapkin Jul 24, 2022
dd842ff
`for_loop_over_fallibles`: remove duplication from the message
WaffleLapkin Jul 24, 2022
7308564
Add a test for the `for_loop_over_fallibles` lint
WaffleLapkin Jul 24, 2022
23a7674
`for_loop_over_fallibles`: fix suggestion for "remove `.next()`" case
WaffleLapkin Jul 24, 2022
8ca57b5
`for_loop_over_fallibles`: don't use `MachineApplicable`
WaffleLapkin Jul 24, 2022
0250f02
allow or avoid for loops over option in compiler and tests
WaffleLapkin Jul 26, 2022
75ae20a
allow `for_loop_over_fallibles` in a `core` test
WaffleLapkin Jul 26, 2022
b9b2059
Edit documentation for `for_loop_over_fallibles` lint
WaffleLapkin Aug 14, 2022
6766113
remove an infinite loop
WaffleLapkin Aug 15, 2022
98e0c4d
fix `for_loop_over_fallibles` lint docs
WaffleLapkin Aug 18, 2022
9c64bb1
Fix clippy tests that trigger `for_loop_over_fallibles` lint
WaffleLapkin Aug 25, 2022
7434b9f
fixup lint name
WaffleLapkin Oct 7, 2022
40f36fa
adopt to new rustc lint api
WaffleLapkin Oct 7, 2022
5347c81
deprecate `clippy::for_loops_over_fallibles`
WaffleLapkin Oct 7, 2022
9d4edff
adopt to building infcx
WaffleLapkin Oct 7, 2022
e828ce5
Skip chained OpaqueCast when building captures.
cjgillot Oct 9, 2022
d3bd6be
Rename AssocItemKind::TyAlias to AssocItemKind::Type
compiler-errors Oct 10, 2022
7e16f9f
Rollup merge of #99696 - WaffleLapkin:uplift, r=fee1-dead
Dylan-DPC Oct 10, 2022
0c17324
Rollup merge of #102055 - c410-f3r:moar-errors, r=petrochenkov
Dylan-DPC Oct 10, 2022
58d533d
Rollup merge of #102786 - compiler-errors:no-tuple-candidate, r=lcnr
Dylan-DPC Oct 10, 2022
302bf31
Rollup merge of #102794 - dtolnay:termination, r=thomcc
Dylan-DPC Oct 10, 2022
5a09b72
Rollup merge of #102853 - cjgillot:skip-opaque-cast, r=jackh726
Dylan-DPC Oct 10, 2022
81b9d0b
Rollup merge of #102868 - compiler-errors:rename-assoc-tyalias-to-ty,…
Dylan-DPC Oct 10, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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.
  • Loading branch information
WaffleLapkin committed Oct 9, 2022
commit fa380a82a5886768d0509222dc0d49db231a3403
99 changes: 99 additions & 0 deletions compiler/rustc_lint/src/for_loop_over_fallibles.rs
Original file line number Diff line number Diff line change
@@ -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
}

}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ mod early;
mod enum_intrinsics_non_enums;
mod errors;
mod expect;
mod for_loop_over_fallibles;
pub mod hidden_unicode_codepoints;
mod internal;
mod late;
Expand Down Expand Up @@ -86,6 +87,7 @@ use rustc_span::Span;
use array_into_iter::ArrayIntoIter;
use builtin::*;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
use for_loop_over_fallibles::*;
use hidden_unicode_codepoints::*;
use internal::*;
use let_underscore::*;
Expand Down Expand Up @@ -188,6 +190,7 @@ macro_rules! late_lint_mod_passes {
$macro!(
$args,
[
ForLoopOverFallibles: ForLoopOverFallibles,
HardwiredLints: HardwiredLints,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
Expand Down