diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index bf92ddb33701c..577523a1d5a32 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -3,7 +3,7 @@ use crate::deriving::generic::*; use crate::errors; use core::ops::ControlFlow; use rustc_ast as ast; -use rustc_ast::visit::walk_list; +use rustc_ast::visit::visit_opt; use rustc_ast::{attr, EnumDef, VariantData}; use rustc_expand::base::{Annotatable, DummyResult, ExtCtxt}; use rustc_span::symbol::Ident; @@ -224,7 +224,7 @@ impl<'a, 'b> rustc_ast::visit::Visitor<'a> for DetectNonVariantDefaultAttr<'a, ' self.visit_ident(v.ident); self.visit_vis(&v.vis); self.visit_variant_data(&v.data); - walk_list!(self, visit_anon_const, &v.disr_expr); + visit_opt!(self, visit_anon_const, &v.disr_expr); for attr in &v.attrs { rustc_ast::visit::walk_attribute(self, attr); } diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index 4540310937d02..e51f95eed021b 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -6,7 +6,7 @@ //! //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/borrow_check.html -use rustc_ast::visit::walk_list; +use rustc_ast::visit::visit_opt; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -168,7 +168,7 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => visitor.visit_stmt(statement), } } - walk_list!(visitor, visit_expr, &blk.expr); + visit_opt!(visitor, visit_expr, &blk.expr); } visitor.cx = prev_cx; diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 5180fce2eb378..8c9ad5b6c6b7a 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -215,7 +215,7 @@ lint_expectation = this lint expectation is unfulfilled .rationale = {$rationale} lint_for_loops_over_fallibles = - for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement + for loop over {$article} `{$ref_prefix}{$ty}`. This is more readably written as an `if let` statement .suggestion = consider using `if let` to clear intent .remove_next = to iterate over `{$recv_snip}` remove the call to `next` .use_while_let = to check pattern in a loop use `while let` diff --git a/compiler/rustc_lint/src/for_loops_over_fallibles.rs b/compiler/rustc_lint/src/for_loops_over_fallibles.rs index a6876d8aae793..b05f5e7638b4e 100644 --- a/compiler/rustc_lint/src/for_loops_over_fallibles.rs +++ b/compiler/rustc_lint/src/for_loops_over_fallibles.rs @@ -52,14 +52,27 @@ impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles { let ty = cx.typeck_results().expr_ty(arg); - let &ty::Adt(adt, args) = ty.kind() else { return }; + let (adt, args, ref_mutability) = match ty.kind() { + &ty::Adt(adt, args) => (adt, args, None), + &ty::Ref(_, ty, mutability) => match ty.kind() { + &ty::Adt(adt, args) => (adt, args, Some(mutability)), + _ => return, + }, + _ => return, + }; let (article, ty, var) = match adt.did() { + did if cx.tcx.is_diagnostic_item(sym::Option, did) && ref_mutability.is_some() => ("a", "Option", "Some"), 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 ref_prefix = match ref_mutability { + None => "", + Some(ref_mutability) => ref_mutability.ref_prefix_str(), + }; + let sub = if let Some(recv) = extract_iterator_next_call(cx, arg) && let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span) { @@ -85,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles { cx.emit_span_lint( FOR_LOOPS_OVER_FALLIBLES, arg.span, - ForLoopsOverFalliblesDiag { article, ty, sub, question_mark, suggestion }, + ForLoopsOverFalliblesDiag { article, ref_prefix, ty, sub, question_mark, suggestion }, ); } } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 7efa5245baa20..382532b17880a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -613,6 +613,7 @@ pub enum PtrNullChecksDiag<'a> { #[diag(lint_for_loops_over_fallibles)] pub struct ForLoopsOverFalliblesDiag<'a> { pub article: &'static str, + pub ref_prefix: &'static str, pub ty: &'static str, #[subdiagnostic] pub sub: ForLoopsOverFalliblesLoopSub<'a>, diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 3fc719394bf9e..d948354bc4959 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -925,7 +925,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for subpattern in prefix.iter() { self.visit_primary_bindings(subpattern, pattern_user_ty.clone().index(), f); } - for subpattern in slice { + if let Some(subpattern) = slice { self.visit_primary_bindings( subpattern, pattern_user_ty.clone().subslice(from, to), diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 322f2922f9253..796e3e38aebfb 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -12,7 +12,7 @@ use crate::{Module, ModuleOrUniformRoot, NameBinding, ParentScope, PathResult}; use crate::{ResolutionError, Resolver, Segment, UseError}; use rustc_ast::ptr::P; -use rustc_ast::visit::{walk_list, AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor}; +use rustc_ast::visit::{visit_opt, walk_list, AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor}; use rustc_ast::*; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::{codes::*, Applicability, DiagArgValue, IntoDiagArg, StashKey}; @@ -3286,7 +3286,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { fn resolve_local(&mut self, local: &'ast Local) { debug!("resolving local ({:?})", local); // Resolve the type. - walk_list!(self, visit_ty, &local.ty); + visit_opt!(self, visit_ty, &local.ty); // Resolve the initializer. if let Some((init, els)) = local.kind.init_else_opt() { @@ -3485,8 +3485,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { fn resolve_arm(&mut self, arm: &'ast Arm) { self.with_rib(ValueNS, RibKind::Normal, |this| { this.resolve_pattern_top(&arm.pat, PatternSource::Match); - walk_list!(this, visit_expr, &arm.guard); - walk_list!(this, visit_expr, &arm.body); + visit_opt!(this, visit_expr, &arm.guard); + visit_opt!(this, visit_expr, &arm.body); }); } diff --git a/library/core/tests/result.rs b/library/core/tests/result.rs index a47ef7aa1ebc6..00a6fd75b4f46 100644 --- a/library/core/tests/result.rs +++ b/library/core/tests/result.rs @@ -170,6 +170,7 @@ pub fn test_iter() { } #[test] +#[allow(for_loops_over_fallibles)] pub fn test_iter_mut() { let mut ok: Result = Ok(100); for loc in ok.iter_mut() { diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs index 10726b9842080..29e8809e5bd66 100644 --- a/src/tools/compiletest/src/json.rs +++ b/src/tools/compiletest/src/json.rs @@ -282,7 +282,7 @@ fn push_expected_errors( // Add notes for the backtrace for span in primary_spans { - for frame in &span.expansion { + if let Some(frame) = &span.expansion { push_backtrace(expected_errors, frame, file_name); } } @@ -315,7 +315,7 @@ fn push_backtrace( }); } - for previous_expansion in &expansion.span.expansion { + if let Some(previous_expansion) = &expansion.span.expansion { push_backtrace(expected_errors, previous_expansion, file_name); } } diff --git a/tests/ui/lint/for_loop_over_fallibles.rs b/tests/ui/lint/for_loop_over_fallibles.rs index 52c3b8f2aae5f..89ac20c352178 100644 --- a/tests/ui/lint/for_loop_over_fallibles.rs +++ b/tests/ui/lint/for_loop_over_fallibles.rs @@ -41,3 +41,25 @@ fn _returns_result() -> Result<(), ()> { Ok(()) } + +fn _by_ref() { + // Shared refs + for _ in &Some(1) {} + //~^ WARN for loop over a `&Option`. This is more readably written as an `if let` statement + //~| HELP to check pattern in a loop use `while let` + //~| HELP consider using `if let` to clear intent + for _ in &Ok::<_, ()>(1) {} + //~^ WARN for loop over a `&Result`. This is more readably written as an `if let` statement + //~| HELP to check pattern in a loop use `while let` + //~| HELP consider using `if let` to clear intent + + // Mutable refs + for _ in &mut Some(1) {} + //~^ WARN for loop over a `&mut Option`. This is more readably written as an `if let` statement + //~| HELP to check pattern in a loop use `while let` + //~| HELP consider using `if let` to clear intent + for _ in &mut Ok::<_, ()>(1) {} + //~^ WARN for loop over a `&mut Result`. This is more readably written as an `if let` statement + //~| HELP to check pattern in a loop use `while let` + //~| HELP consider using `if let` to clear intent +} diff --git a/tests/ui/lint/for_loop_over_fallibles.stderr b/tests/ui/lint/for_loop_over_fallibles.stderr index 96efdf85c4901..f695de082572a 100644 --- a/tests/ui/lint/for_loop_over_fallibles.stderr +++ b/tests/ui/lint/for_loop_over_fallibles.stderr @@ -97,5 +97,65 @@ help: consider using `if let` to clear intent LL | if let Ok(_) = Ok::<_, ()>([0; 0]) {} | ~~~~~~~~~~ ~~~ -warning: 6 warnings emitted +warning: for loop over a `&Option`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:47:14 + | +LL | for _ in &Some(1) {} + | ^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Some(_) = &Some(1) {} + | ~~~~~~~~~~~~~~~ ~~~ +help: consider using `if let` to clear intent + | +LL | if let Some(_) = &Some(1) {} + | ~~~~~~~~~~~~ ~~~ + +warning: for loop over a `&Result`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:51:14 + | +LL | for _ in &Ok::<_, ()>(1) {} + | ^^^^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Ok(_) = &Ok::<_, ()>(1) {} + | ~~~~~~~~~~~~~ ~~~ +help: consider using `if let` to clear intent + | +LL | if let Ok(_) = &Ok::<_, ()>(1) {} + | ~~~~~~~~~~ ~~~ + +warning: for loop over a `&mut Option`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:57:14 + | +LL | for _ in &mut Some(1) {} + | ^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Some(_) = &mut Some(1) {} + | ~~~~~~~~~~~~~~~ ~~~ +help: consider using `if let` to clear intent + | +LL | if let Some(_) = &mut Some(1) {} + | ~~~~~~~~~~~~ ~~~ + +warning: for loop over a `&mut Result`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:61:14 + | +LL | for _ in &mut Ok::<_, ()>(1) {} + | ^^^^^^^^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Ok(_) = &mut Ok::<_, ()>(1) {} + | ~~~~~~~~~~~~~ ~~~ +help: consider using `if let` to clear intent + | +LL | if let Ok(_) = &mut Ok::<_, ()>(1) {} + | ~~~~~~~~~~ ~~~ + +warning: 10 warnings emitted