diff options
| author | bors <bors@rust-lang.org> | 2022-10-10 09:12:06 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-10 09:12:06 +0000 |
| commit | 691aeaad6c9ee3483f2851695379b348b8a92a43 (patch) | |
| tree | 644a0f96f905d8e219e89cb7c06f1272e6731d56 | |
| parent | 8dfb40722da197e77a33a19eb9d3fd6512831341 (diff) | |
| parent | 81b9d0b1d11d5a43c9d5ba1791040bbb597b7cdf (diff) | |
| download | rust-691aeaad6c9ee3483f2851695379b348b8a92a43.tar.gz rust-691aeaad6c9ee3483f2851695379b348b8a92a43.zip | |
Auto merge of #102875 - Dylan-DPC:rollup-zwcq8h9, r=Dylan-DPC
Rollup of 6 pull requests Successful merges: - #99696 (Uplift `clippy::for_loops_over_fallibles` lint into rustc) - #102055 (Move some tests to more reasonable directories) - #102786 (Remove tuple candidate, nothing special about it) - #102794 (Make tests capture the error printed by a Result return) - #102853 (Skip chained OpaqueCast when building captures.) - #102868 (Rename `AssocItemKind::TyAlias` to `AssocItemKind::Type`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
67 files changed, 478 insertions, 621 deletions
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 0efde1e7b21..340302766d2 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2953,7 +2953,7 @@ pub enum AssocItemKind { /// An associated function. Fn(Box<Fn>), /// An associated type. - TyAlias(Box<TyAlias>), + Type(Box<TyAlias>), /// A macro expanding to associated items. MacCall(P<MacCall>), } @@ -2963,7 +2963,7 @@ impl AssocItemKind { match *self { Self::Const(defaultness, ..) | Self::Fn(box Fn { defaultness, .. }) - | Self::TyAlias(box TyAlias { defaultness, .. }) => defaultness, + | Self::Type(box TyAlias { defaultness, .. }) => defaultness, Self::MacCall(..) => Defaultness::Final, } } @@ -2974,7 +2974,7 @@ impl From<AssocItemKind> for ItemKind { match assoc_item_kind { AssocItemKind::Const(a, b, c) => ItemKind::Const(a, b, c), AssocItemKind::Fn(fn_kind) => ItemKind::Fn(fn_kind), - AssocItemKind::TyAlias(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind), + AssocItemKind::Type(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind), AssocItemKind::MacCall(a) => ItemKind::MacCall(a), } } @@ -2987,7 +2987,7 @@ impl TryFrom<ItemKind> for AssocItemKind { Ok(match item_kind { ItemKind::Const(a, b, c) => AssocItemKind::Const(a, b, c), ItemKind::Fn(fn_kind) => AssocItemKind::Fn(fn_kind), - ItemKind::TyAlias(ty_alias_kind) => AssocItemKind::TyAlias(ty_alias_kind), + ItemKind::TyAlias(ty_kind) => AssocItemKind::Type(ty_kind), ItemKind::MacCall(a) => AssocItemKind::MacCall(a), _ => return Err(item_kind), }) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index ad68d6e755e..25022a02f4b 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1106,7 +1106,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>( visit_fn_sig(sig, visitor); visit_opt(body, |body| visitor.visit_block(body)); } - AssocItemKind::TyAlias(box TyAlias { + AssocItemKind::Type(box TyAlias { defaultness, generics, where_clauses, diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index a71e055a4b3..e752cc7dc2d 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -244,14 +244,12 @@ pub trait Visitor<'ast>: Sized { #[macro_export] macro_rules! walk_list { - ($visitor: expr, $method: ident, $list: expr) => { - for elem in $list { - $visitor.$method(elem) - } - }; - ($visitor: expr, $method: ident, $list: expr, $($extra_args: expr),*) => { - for elem in $list { - $visitor.$method(elem, $($extra_args,)*) + ($visitor: expr, $method: ident, $list: expr $(, $($extra_args: expr),* )?) => { + { + #[cfg_attr(not(bootstrap), allow(for_loops_over_fallibles))] + for elem in $list { + $visitor.$method(elem $(, $($extra_args,)* )?) + } } } } @@ -685,7 +683,7 @@ pub fn walk_assoc_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a AssocItem, let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), ident, sig, vis, generics, body.as_deref()); visitor.visit_fn(kind, span, id); } - AssocItemKind::TyAlias(box TyAlias { generics, bounds, ty, .. }) => { + AssocItemKind::Type(box TyAlias { generics, bounds, ty, .. }) => { visitor.visit_generics(generics); walk_list!(visitor, visit_param_bound, bounds, BoundKind::Bound); walk_list!(visitor, visit_ty, ty); diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 347e735fadf..56d71aaa7d3 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -804,7 +804,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ); (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true) } - AssocItemKind::TyAlias(box TyAlias { + AssocItemKind::Type(box TyAlias { ref generics, where_clauses, ref bounds, @@ -850,7 +850,7 @@ impl<'hir> LoweringContext<'_, 'hir> { fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef { let kind = match &i.kind { AssocItemKind::Const(..) => hir::AssocItemKind::Const, - AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type, + AssocItemKind::Type(..) => hir::AssocItemKind::Type, AssocItemKind::Fn(box Fn { sig, .. }) => { hir::AssocItemKind::Fn { has_self: sig.decl.has_self() } } @@ -898,7 +898,7 @@ impl<'hir> LoweringContext<'_, 'hir> { (generics, hir::ImplItemKind::Fn(sig, body_id)) } - AssocItemKind::TyAlias(box TyAlias { generics, where_clauses, ty, .. }) => { + AssocItemKind::Type(box TyAlias { generics, where_clauses, ty, .. }) => { let mut generics = generics.clone(); add_ty_alias_where_clause(&mut generics, *where_clauses, false); self.lower_generics( @@ -941,7 +941,7 @@ impl<'hir> LoweringContext<'_, 'hir> { span: self.lower_span(i.span), kind: match &i.kind { AssocItemKind::Const(..) => hir::AssocItemKind::Const, - AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type, + AssocItemKind::Type(..) => hir::AssocItemKind::Type, AssocItemKind::Fn(box Fn { sig, .. }) => { hir::AssocItemKind::Fn { has_self: sig.decl.has_self() } } diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index ecf74c76020..1a4c60087c3 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -1556,7 +1556,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { }); } } - AssocItemKind::TyAlias(box TyAlias { + AssocItemKind::Type(box TyAlias { generics, where_clauses, where_predicates_split, @@ -1595,7 +1595,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } match item.kind { - AssocItemKind::TyAlias(box TyAlias { ref generics, ref bounds, ref ty, .. }) + AssocItemKind::Type(box TyAlias { ref generics, ref bounds, ref ty, .. }) if ctxt == AssocCtxt::Trait => { self.visit_vis(&item.vis); diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index 0b033e9384c..0f11c176652 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -517,7 +517,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) { let is_fn = match i.kind { ast::AssocItemKind::Fn(_) => true, - ast::AssocItemKind::TyAlias(box ast::TyAlias { ref ty, .. }) => { + ast::AssocItemKind::Type(box ast::TyAlias { ref ty, .. }) => { if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) { gate_feature_post!( &self, diff --git a/compiler/rustc_ast_pretty/src/pprust/state/item.rs b/compiler/rustc_ast_pretty/src/pprust/state/item.rs index 54bac29a6ce..159853c9e24 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/item.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/item.rs @@ -516,7 +516,7 @@ impl<'a> State<'a> { ast::AssocItemKind::Const(def, ty, body) => { self.print_item_const(ident, None, ty, body.as_deref(), vis, *def); } - ast::AssocItemKind::TyAlias(box ast::TyAlias { + ast::AssocItemKind::Type(box ast::TyAlias { defaultness, generics, where_clauses, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index a794fdf2bba..16ee3aa89bb 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -566,7 +566,7 @@ impl<'a> TraitDef<'a> { tokens: None, }, attrs: ast::AttrVec::new(), - kind: ast::AssocItemKind::TyAlias(Box::new(ast::TyAlias { + kind: ast::AssocItemKind::Type(Box::new(ast::TyAlias { defaultness: ast::Defaultness::Final, generics: Generics::default(), where_clauses: ( diff --git a/compiler/rustc_lint/src/for_loops_over_fallibles.rs b/compiler/rustc_lint/src/for_loops_over_fallibles.rs new file mode 100644 index 00000000000..ed8d424e0c6 --- /dev/null +++ b/compiler/rustc_lint/src/for_loops_over_fallibles.rs @@ -0,0 +1,183 @@ +use crate::{LateContext, LateLintPass, LintContext}; + +use hir::{Expr, Pat}; +use rustc_errors::{Applicability, DelayDm}; +use rustc_hir as hir; +use rustc_infer::traits::TraitEngine; +use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause}; +use rustc_middle::ty::{self, List}; +use rustc_span::{sym, Span}; +use rustc_trait_selection::traits::TraitEngineExt; + +declare_lint! { + /// The `for_loops_over_fallibles` lint checks for `for` loops over `Option` or `Result` values. + /// + /// ### Example + /// + /// ```rust + /// let opt = Some(1); + /// for x in opt { /* ... */} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop. + /// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`) + /// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed + /// via `if let`. + /// + /// `for` loop can also be accidentally written with the intention to call a function multiple times, + /// while the function returns `Some(_)`, in these cases `while let` loop should be used instead. + /// + /// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to + /// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)` + /// to optionally add a value to an iterator. + pub FOR_LOOPS_OVER_FALLIBLES, + Warn, + "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`" +} + +declare_lint_pass!(ForLoopsOverFallibles => [FOR_LOOPS_OVER_FALLIBLES]); + +impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles { + 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, substs) = 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 msg = DelayDm(|| { + format!( + "for loop over {article} `{ty}`. This is more readably written as an `if let` statement", + ) + }); + + cx.struct_span_lint(FOR_LOOPS_OVER_FALLIBLES, arg.span, msg, |lint| { + if let Some(recv) = extract_iterator_next_call(cx, arg) + && let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span) + { + lint.span_suggestion( + recv.span.between(arg.span.shrink_to_hi()), + format!("to iterate over `{recv_snip}` remove the call to `next`"), + ".by_ref()", + Applicability::MaybeIncorrect + ); + } else { + lint.multipart_suggestion_verbose( + format!("to check pattern in a loop use `while let`"), + vec![ + // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts + (expr.span.with_hi(pat.span.lo()), format!("while let {var}(")), + (pat.span.between(arg.span), format!(") = ")), + ], + Applicability::MaybeIncorrect + ); + } + + if suggest_question_mark(cx, adt, substs, expr.span) { + lint.span_suggestion( + arg.span.shrink_to_hi(), + "consider unwrapping the `Result` with `?` to iterate over its contents", + "?", + Applicability::MaybeIncorrect, + ); + } + + lint.multipart_suggestion_verbose( + "consider using `if let` to clear intent", + vec![ + // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts + (expr.span.with_hi(pat.span.lo()), format!("if let {var}(")), + (pat.span.between(arg.span), format!(") = ")), + ], + Applicability::MaybeIncorrect, + ) + }) + } +} + +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 + } +} + +fn extract_iterator_next_call<'tcx>( + cx: &LateContext<'_>, + expr: &Expr<'tcx>, +) -> Option<&'tcx Expr<'tcx>> { + // This won't work for `Iterator::next(iter)`, is this an issue? + if let hir::ExprKind::MethodCall(_, recv, _, _) = expr.kind + && cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn() + { + Some(recv) + } else { + return None + } +} + +fn suggest_question_mark<'tcx>( + cx: &LateContext<'tcx>, + adt: ty::AdtDef<'tcx>, + substs: &List<ty::GenericArg<'tcx>>, + span: Span, +) -> bool { + let Some(body_id) = cx.enclosing_body else { return false }; + let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false }; + + if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) { + return false; + } + + // Check that the function/closure/constant we are in has a `Result` type. + // Otherwise suggesting using `?` may not be a good idea. + { + let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value); + let ty::Adt(ret_adt, ..) = ty.kind() else { return false }; + if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) { + return false; + } + } + + let ty = substs.type_at(0); + let infcx = cx.tcx.infer_ctxt().build(); + let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx); + + let cause = ObligationCause::new( + span, + body_id.hir_id, + rustc_infer::traits::ObligationCauseCode::MiscObligation, + ); + fulfill_cx.register_bound( + &infcx, + ty::ParamEnv::empty(), + // Erase any region vids from the type, which may not be resolved + infcx.tcx.erase_regions(ty), + into_iterator_did, + cause, + ); + + // Select all, including ambiguous predicates + let errors = fulfill_cx.select_all_or_error(&infcx); + + errors.is_empty() +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 070cccd141b..fee6e080c4f 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -52,6 +52,7 @@ mod early; mod enum_intrinsics_non_enums; mod errors; mod expect; +mod for_loops_over_fallibles; pub mod hidden_unicode_codepoints; mod internal; mod late; @@ -86,6 +87,7 @@ use rustc_span::Span; use array_into_iter::ArrayIntoIter; use builtin::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; +use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; use internal::*; use let_underscore::*; @@ -188,6 +190,7 @@ macro_rules! late_lint_mod_passes { $macro!( $args, [ + ForLoopsOverFallibles: ForLoopsOverFallibles, HardwiredLints: HardwiredLints, ImproperCTypesDeclarations: ImproperCTypesDeclarations, ImproperCTypesDefinitions: ImproperCTypesDefinitions, diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 9f800e9c8c9..6b32e78b910 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -187,7 +187,7 @@ impl EarlyLintPass for NonCamelCaseTypes { } fn check_trait_item(&mut self, cx: &EarlyContext<'_>, it: &ast::AssocItem) { - if let ast::AssocItemKind::TyAlias(..) = it.kind { + if let ast::AssocItemKind::Type(..) = it.kind { self.check_case(cx, "associated type", &it.ident); } } diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 68a7af0b8c8..a635e0463e5 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -664,10 +664,6 @@ pub enum ImplSource<'tcx, N> { /// ImplSource for a `const Drop` implementation. ConstDestruct(ImplSourceConstDestructData<N>), - - /// ImplSource for a `std::marker::Tuple` implementation. - /// This has no nested predicates ever, so no data. - Tuple, } impl<'tcx, N> ImplSource<'tcx, N> { @@ -682,8 +678,7 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::Object(d) => d.nested, ImplSource::FnPointer(d) => d.nested, ImplSource::DiscriminantKind(ImplSourceDiscriminantKindData) - | ImplSource::Pointee(ImplSourcePointeeData) - | ImplSource::Tuple => Vec::new(), + | ImplSource::Pointee(ImplSourcePointeeData) => vec![], ImplSource::TraitAlias(d) => d.nested, ImplSource::TraitUpcasting(d) => d.nested, ImplSource::ConstDestruct(i) => i.nested, @@ -701,8 +696,7 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::Object(d) => &d.nested, ImplSource::FnPointer(d) => &d.nested, ImplSource::DiscriminantKind(ImplSourceDiscriminantKindData) - | ImplSource::Pointee(ImplSourcePointeeData) - | ImplSource::Tuple => &[], + | ImplSource::Pointee(ImplSourcePointeeData) => &[], ImplSource::TraitAlias(d) => &d.nested, ImplSource::TraitUpcasting(d) => &d.nested, ImplSource::ConstDestruct(i) => &i.nested, @@ -769,7 +763,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { nested: i.nested.into_iter().map(f).collect(), }) } - ImplSource::Tuple => ImplSource::Tuple, } } } diff --git a/compiler/rustc_middle/src/traits/select.rs b/compiler/rustc_middle/src/traits/select.rs index 11758599cff..85ead3171e7 100644 --- a/compiler/rustc_middle/src/traits/select.rs +++ b/compiler/rustc_middle/src/traits/select.rs @@ -161,9 +161,6 @@ pub enum SelectionCandidate<'tcx> { /// Implementation of `const Destruct`, optionally from a custom `impl const Drop`. ConstDestructCandidate(Option<DefId>), - - /// Witnesses the fact that a type is a tuple. - TupleCandidate, } /// The result of trait evaluation. The order is important diff --git a/compiler/rustc_middle/src/traits/structural_impls.rs b/compiler/rustc_middle/src/traits/structural_impls.rs index c526344e1f2..7fbd57ac735 100644 --- a/compiler/rustc_middle/src/traits/structural_impls.rs +++ b/compiler/rustc_middle/src/traits/structural_impls.rs @@ -34,8 +34,6 @@ impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSource<'tcx, N> { super::ImplSource::TraitUpcasting(ref d) => write!(f, "{:?}", d), super::ImplSource::ConstDestruct(ref d) => write!(f, "{:?}", d), - - super::ImplSource::Tuple => write!(f, "ImplSource::Tuple"), } } } diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index 4d9e9b7c473..396782d45d2 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -17,6 +17,7 @@ use rustc_target::abi::VariantIdx; use rustc_index::vec::Idx; +use std::assert_matches::assert_matches; use std::iter; /// The "outermost" place that holds this value. @@ -232,22 +233,20 @@ fn strip_prefix<'tcx>( projections: Vec<PlaceElem<'tcx>>, prefix_projections: &[HirProjection<'tcx>], ) -> impl Iterator<Item = PlaceElem<'tcx>> { - let mut iter = projections.into_iter(); - let mut next = || match iter.next()? { + let mut iter = projections + .into_iter() // Filter out opaque casts, they are unnecessary in the prefix. - ProjectionElem::OpaqueCast(..) => iter.next(), - other => Some(other), - }; + .filter(|elem| !matches!(elem, ProjectionElem::OpaqueCast(..))); for projection in prefix_projections { match projection.kind { HirProjectionKind::Deref => { - assert!(matches!(next(), Some(ProjectionElem::Deref))); + assert_matches!(iter.next(), Some(ProjectionElem::Deref)); } HirProjectionKind::Field(..) => { if base_ty.is_enum() { - assert!(matches!(next(), Some(ProjectionElem::Downcast(..)))); + assert_matches!(iter.next(), Some(ProjectionElem::Downcast(..))); } - assert!(matches!(next(), Some(ProjectionElem::Field(..)))); + assert_matches!(iter.next(), Some(ProjectionElem::Field(..))); } HirProjectionKind::Index | HirProjectionKind::Subslice => { bug!("unexpected projection kind: {:?}", projection); diff --git a/compiler/rustc_mir_build/src/lib.rs b/compiler/rustc_mir_build/src/lib.rs index 8236b1528c0..b53bd3d0710 100644 --- a/compiler/rustc_mir_build/src/lib.rs +++ b/compiler/rustc_mir_build/src/lib.rs @@ -2,6 +2,7 @@ //! //! This crate also contains the match exhaustiveness and usefulness checking. #![allow(rustc::potential_query_instability)] +#![feature(assert_matches)] #![feature(box_patterns)] #![feature(control_flow_enum)] #![feature(if_let_guard)] diff --git a/compiler/rustc_passes/src/hir_stats.rs b/compiler/rustc_passes/src/hir_stats.rs index b413d78b38d..33220fd2b39 100644 --- a/compiler/rustc_passes/src/hir_stats.rs +++ b/compiler/rustc_passes/src/hir_stats.rs @@ -620,7 +620,7 @@ impl<'v> ast_visit::Visitor<'v> for StatCollector<'v> { fn visit_assoc_item(&mut self, i: &'v ast::AssocItem, ctxt: ast_visit::AssocCtxt) { record_variants!( (self, i, i.kind, Id::None, ast, AssocItem, AssocItemKind), - [Const, Fn, TyAlias, MacCall] + [Const, Fn, Type, MacCall] ); ast_visit::walk_assoc_item(self, i, ctxt); } diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 29aab416ae9..c3d87b5b6af 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -1425,7 +1425,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { } (DefKind::AssocFn, ValueNS) } - AssocItemKind::TyAlias(..) => (DefKind::AssocTy, TypeNS), + AssocItemKind::Type(..) => (DefKind::AssocTy, TypeNS), AssocItemKind::MacCall(_) => bug!(), // handled above }; diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index 38a3c9dd71a..d36e0f61d91 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -239,7 +239,7 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> { fn visit_assoc_item(&mut self, i: &'a AssocItem, ctxt: visit::AssocCtxt) { let def_data = match &i.kind { AssocItemKind::Fn(..) | AssocItemKind::Const(..) => DefPathData::ValueNs(i.ident.name), - AssocItemKind::TyAlias(..) => DefPathData::TypeNs(i.ident.name), + AssocItemKind::Type(..) => DefPathData::TypeNs(i.ident.name), AssocItemKind::MacCall(..) => return self.visit_macro_invoc(i.id), }; diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 431507e8e0f..989a827ef7d 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2498,7 +2498,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { AssocItemKind::Fn(box Fn { generics, .. }) => { walk_assoc_item(self, generics, LifetimeBinderKind::Function, item); } - AssocItemKind::TyAlias(box TyAlias { generics, .. }) => self + AssocItemKind::Type(box TyAlias { generics, .. }) => self .with_lifetime_rib(LifetimeRibKind::AnonymousReportError, |this| { walk_assoc_item(this, generics, LifetimeBinderKind::Item, item) }), @@ -2694,8 +2694,8 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { }, ); } - AssocItemKind::TyAlias(box TyAlias { generics, .. }) => { - debug!("resolve_implementation AssocItemKind::TyAlias"); + AssocItemKind::Type(box TyAlias { generics, .. }) => { + debug!("resolve_implementation AssocItemKind::Type"); // We also need a new scope for the impl item type parameters. self.with_generic_param_rib( &generics.params, @@ -2770,7 +2770,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { let res = binding.res(); let Res::Def(def_kind, _) = res else { bug!() }; match (def_kind, kind) { - (DefKind::AssocTy, AssocItemKind::TyAlias(..)) + (DefKind::AssocTy, AssocItemKind::Type(..)) | (DefKind::AssocFn, AssocItemKind::Fn(..)) | (DefKind::AssocConst, AssocItemKind::Const(..)) => { self.r.record_partial_res(id, PartialRes::new(res)); @@ -2784,7 +2784,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { let (code, kind) = match kind { AssocItemKind::Const(..) => (rustc_errors::error_code!(E0323), "const"), AssocItemKind::Fn(..) => (rustc_errors::error_code!(E0324), "method"), - AssocItemKind::TyAlias(..) => (rustc_errors::error_code!(E0325), "type"), + AssocItemKind::Type(..) => (rustc_errors::error_code!(E0325), "type"), AssocItemKind::MacCall(..) => span_bug!(span, "unexpanded macro"), }; let trait_path = path_names_to_string(path); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2d339a4d070..c05f89a6575 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1482,7 +1482,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { .filter(|(_, res)| match (kind, res) { (AssocItemKind::Const(..), Res::Def(DefKind::AssocConst, _)) => true, (AssocItemKind::Fn(_), Res::Def(DefKind::AssocFn, _)) => true, - (AssocItemKind::TyAlias(..), Res::Def(DefKind::AssocTy, _)) => true, + (AssocItemKind::Type(..), Res::Def(DefKind::AssocTy, _)) => true, _ => false, }) .map(|(key, _)| key.ident.name) @@ -1546,7 +1546,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { AssocSuggestion::MethodWithSelf } ast::AssocItemKind::Fn(..) => AssocSuggestion::AssocFn, - ast::AssocItemKind::TyAlias(..) => AssocSuggestion::AssocType, + ast::AssocItemKind::Type(..) => AssocSuggestion::AssocType, ast::AssocItemKind::MacCall(_) => continue, }); } diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 4a2fde2cb4b..e1485079487 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1750,8 +1750,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( super::ImplSource::AutoImpl(..) | super::ImplSource::Builtin(..) | super::ImplSource::TraitUpcasting(_) - | super::ImplSource::ConstDestruct(_) - | super::ImplSource::Tuple => { + | super::ImplSource::ConstDestruct(_) => { // These traits have no associated types. selcx.tcx().sess.delay_span_bug( obligation.cause.span, @@ -1829,8 +1828,7 @@ fn confirm_select_candidate<'cx, 'tcx>( | super::ImplSource::Builtin(..) | super::ImplSource::TraitUpcasting(_) | super::ImplSource::TraitAlias(..) - | super::ImplSource::ConstDestruct(_) - | super::ImplSource::Tuple => { + | super::ImplSource::ConstDestruct(_) => { // we don't create Select candidates with this kind of resolution span_bug!( obligation.cause.span, diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index f6c0930eb5f..4c5bc333961 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -1021,7 +1021,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let self_ty = self.infcx().shallow_resolve(obligation.self_ty().skip_binder()); match self_ty.kind() { ty::Tuple(_) => { - candidates.vec.push(TupleCandidate); + candidates.vec.push(BuiltinCandidate { has_nested: false }); } ty::Infer(ty::TyVar(_)) => { candidates.ambiguous = true; diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 98c99e9ad4f..ed22058c646 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -126,8 +126,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let data = self.confirm_const_destruct_candidate(obligation, def_id)?; ImplSource::ConstDestruct(data) } - - TupleCandidate => ImplSource::Tuple, }; if !obligation.predicate.is_const_if_const() { diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 3799c9a491d..3f445f9ca46 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1562,7 +1562,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { }; // (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`, - // `DiscriminantKindCandidate`, `ConstDestructCandidate`, and `TupleCandidate` + // `DiscriminantKindCandidate`, `ConstDestructCandidate` // to anything else. // // This is a fix for #53123 and prevents winnowing from accidentally extending the @@ -1583,8 +1583,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { BuiltinCandidate { has_nested: false } | DiscriminantKindCandidate | PointeeCandidate - | ConstDestructCandidate(_) - | TupleCandidate, + | ConstDestructCandidate(_), _, ) => true, ( @@ -1592,8 +1591,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { BuiltinCandidate { has_nested: false } | DiscriminantKindCandidate | PointeeCandidate - | ConstDestructCandidate(_) - | TupleCandidate, + | ConstDestructCandidate(_), ) => false, (ParamCandidate(other), ParamCandidate(victim)) => { diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 5ed40515d8a..416c1ec510b 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -267,8 +267,7 @@ fn resolve_associated_item<'tcx>( | traits::ImplSource::DiscriminantKind(..) | traits::ImplSource::Pointee(..) | traits::ImplSource::TraitUpcasting(_) - | traits::ImplSource::ConstDestruct(_) - | traits::ImplSource::Tuple => None, + | traits::ImplSource::ConstDestruct(_) => None, }) } diff --git a/library/core/tests/option.rs b/library/core/tests/option.rs index 9f5e537dcef..f36f7c26806 100644 --- a/library/core/tests/option.rs +++ b/library/core/tests/option.rs @@ -57,6 +57,7 @@ fn test_get_resource() { } #[test] +#[cfg_attr(not(bootstrap), allow(for_loops_over_fallibles))] fn test_option_dance() { let x = Some(()); let mut y = Some(5); diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index eeace2c43c4..cceef539b90 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -262,6 +262,7 @@ use crate::sys_common::memchr; #[stable(feature = "bufwriter_into_parts", since = "1.56.0")] pub use self::buffered::WriterPanicked; +pub(crate) use self::stdio::attempt_print_to_stderr; #[unstable(feature = "internal_output_capture", issue = "none")] #[doc(no_inline, hidden)] pub use self::stdio::set_output_capture; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 2dc12a18a8a..4ccb2bf3231 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -999,7 +999,18 @@ fn print_to<T>(args: fmt::Arguments<'_>, global_s: fn() -> T, label: &str) where T: Write, { - if OUTPUT_CAPTURE_USED.load(Ordering::Relaxed) + if print_to_buffer_if_capture_used(args) { + // Successfully wrote to capture buffer. + return; + } + + if let Err(e) = global_s().write_fmt(args) { + panic!("failed printing to {label}: {e}"); + } +} + +fn print_to_buffer_if_capture_used(args: fmt::Arguments<'_>) -> bool { + OUTPUT_CAPTURE_USED.load(Ordering::Relaxed) && OUTPUT_CAPTURE.try_with(|s| { // Note that we completely remove a local sink to write to in case // our printing recursively panics/prints, so the recursive @@ -1009,14 +1020,19 @@ where s.set(Some(w)); }) }) == Ok(Some(())) - { - // Successfully wrote to capture buffer. +} + +/// Used by impl Termination for Result to print error after `main` or a test +/// has returned. Should avoid panicking, although we can't help it if one of +/// the Display impls inside args decides to. +pub(crate) fn attempt_print_to_stderr(args: fmt::Arguments<'_>) { + if print_to_buffer_if_capture_used(args) { return; } - if let Err(e) = global_s().write_fmt(args) { - panic!("failed printing to {label}: {e}"); - } + // Ignore error if the write fails, for example because stderr is already + // closed. There is not much point panicking at this point. + let _ = stderr().write_fmt(args); } #[unstable( diff --git a/library/std/src/process.rs b/library/std/src/process.rs index d91d4fa64ca..e08ea8f9a5f 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -2200,9 +2200,7 @@ impl<T: Termination, E: fmt::Debug> Termination for Result<T, E> { match self { Ok(val) => val.report(), Err(err) => { - // Ignore error if the write fails, for example because stderr is - // already closed. There is not much point panicking at this point. - let _ = writeln!(io::stderr(), "Error: {err:?}"); + io::attempt_print_to_stderr(format_args_nl!("Error: {err:?}")); ExitCode::FAILURE } } diff --git a/src/test/ui/issues/issue-73541-3.rs b/src/test/ui/async-await/issue-73541-3.rs index 02ca02da8ed..02ca02da8ed 100644 --- a/src/test/ui/issues/issue-73541-3.rs +++ b/src/test/ui/async-await/issue-73541-3.rs diff --git a/src/test/ui/issues/issue-73541-3.stderr b/src/test/ui/async-await/issue-73541-3.stderr index 53487aaca99..53487aaca99 100644 --- a/src/test/ui/issues/issue-73541-3.stderr +++ b/src/test/ui/async-await/issue-73541-3.stderr diff --git a/src/test/ui/issues/issue-73541.rs b/src/test/ui/async-await/issue-73541.rs index 399a07cd3fc..399a07cd3fc 100644 --- a/src/test/ui/issues/issue-73541.rs +++ b/src/test/ui/async-await/issue-73541.rs diff --git a/src/test/ui/issues/issue-73541.stderr b/src/test/ui/async-await/issue-73541.stderr index 4bb466ff16c..4bb466ff16c 100644 --- a/src/test/ui/issues/issue-73541.stderr +++ b/src/test/ui/async-await/issue-73541.stderr diff --git a/src/test/ui/closures/issue-102089-multiple-opaque-cast.rs b/src/test/ui/closures/issue-102089-multiple-opaque-cast.rs new file mode 100644 index 00000000000..043bf06a1f5 --- /dev/null +++ b/src/test/ui/closures/issue-102089-multiple-opaque-cast.rs @@ -0,0 +1,17 @@ +// edition:2021 +// check-pass + +pub struct Example<'a, T> { + a: T, + b: &'a T, +} + +impl<'a, T> Example<'a, T> { + pub fn error_trying_to_destructure_self_in_closure(self) { + let closure = || { + let Self { a, b } = self; + }; + } +} + +fn main() {} diff --git a/src/test/ui/drop/dropck_legal_cycles.rs b/src/test/ui/drop/dropck_legal_cycles.rs index 27a599315dc..6a0fe7784fb 100644 --- a/src/test/ui/drop/dropck_legal_cycles.rs +++ b/src/test/ui/drop/dropck_legal_cycles.rs @@ -1017,7 +1017,7 @@ impl<'a> Children<'a> for HM<'a> { where C: Context + PrePost<Self>, Self: Sized { if let Some(ref hm) = self.contents.get() { - for (k, v) in hm.iter().nth(index / 2) { + if let Some((k, v)) = hm.iter().nth(index / 2) { [k, v][index % 2].descend_into_self(context); } } @@ -1032,7 +1032,7 @@ impl<'a> Children<'a> for VD<'a> { where C: Context + PrePost<Self>, Self: Sized { if let Some(ref vd) = self.contents.get() { - for r in vd.iter().nth(index) { + if let Some(r) = vd.iter().nth(index) { r.descend_into_self(context); } } @@ -1047,7 +1047,7 @@ impl<'a> Children<'a> for VM<'a> { where C: Context + PrePost<VM<'a>> { if let Some(ref vd) = self.contents.get() { - for (_idx, r) in vd.iter().nth(index) { + if let Some((_idx, r)) = vd.iter().nth(index) { r.descend_into_self(context); } } @@ -1062,7 +1062,7 @@ impl<'a> Children<'a> for LL<'a> { where C: Context + PrePost<LL<'a>> { if let Some(ref ll) = self.contents.get() { - for r in ll.iter().nth(index) { + if let Some(r) = ll.iter().nth(index) { r.descend_into_self(context); } } @@ -1077,7 +1077,7 @@ impl<'a> Children<'a> for BH<'a> { where C: Context + PrePost<BH<'a>> { if let Some(ref bh) = self.contents.get() { - for r in bh.iter().nth(index) { + if let Some(r) = bh.iter().nth(index) { r.descend_into_self(context); } } @@ -1092,7 +1092,7 @@ impl<'a> Children<'a> for BTM<'a> { where C: Context + PrePost<BTM<'a>> { if let Some(ref bh) = self.contents.get() { - for (k, v) in bh.iter().nth(index / 2) { + if let Some((k, v)) = bh.iter().nth(index / 2) { [k, v][index % 2].descend_into_self(context); } } @@ -1107,7 +1107,7 @@ impl<'a> Children<'a> for BTS<'a> { where C: Context + PrePost<BTS<'a>> { if let Some(ref bh) = self.contents.get() { - for r in bh.iter().nth(index) { + if let Some(r) = bh.iter().nth(index) { r.descend_into_self(context); } } diff --git a/src/test/ui/issues/issue-30371.rs b/src/test/ui/issues/issue-30371.rs index a1ae9a36bc1..eea548c482f 100644 --- a/src/test/ui/issues/issue-30371.rs +++ b/src/test/ui/issues/issue-30371.rs @@ -1,5 +1,6 @@ // run-pass #![allow(unreachable_code)] +#![allow(for_loops_over_fallibles)] #![deny(unused_variables)] fn main() { diff --git a/src/test/ui/issues/issue-3563-2.rs b/src/test/ui/issues/issue-3563-2.rs deleted file mode 100644 index 88a449b85b8..00000000000 --- a/src/test/ui/issues/issue-3563-2.rs +++ /dev/null @@ -1,14 +0,0 @@ -// check-pass -// pretty-expanded FIXME #23616 - -trait Canvas { - fn add_point(&self, point: &isize); - fn add_points(&self, shapes: &[isize]) { - for pt in shapes { - self.add_point(pt) - } - } - -} - -pub fn main() {} diff --git a/src/test/ui/lint/for_loop_over_fallibles.rs b/src/test/ui/lint/for_loop_over_fallibles.rs new file mode 100644 index 00000000000..43d71c2e808 --- /dev/null +++ b/src/test/ui/lint/for_loop_over_fallibles.rs @@ -0,0 +1,43 @@ +// check-pass + +fn main() { + // Common + for _ in Some(1) {} + //~^ WARN for loop over an `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 + + // `Iterator::next` specific + for _ in [0; 0].iter().next() {} + //~^ WARN for loop over an `Option`. This is more readably written as an `if let` statement + //~| HELP to iterate over `[0; 0].iter()` remove the call to `next` + //~| HELP consider using `if let` to clear intent + + // `Result<impl Iterator, _>`, but function doesn't return `Result` + for _ in Ok::<_, ()>([0; 0].iter()) {} + //~^ 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 +} + +fn _returns_result() -> Result<(), ()> { + // `Result<impl Iterator, _>` + for _ in Ok::<_, ()>([0; 0].iter()) {} + //~^ 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 unwrapping the `Result` with `?` to iterate over its contents + //~| HELP consider using `if let` to clear intent + + // `Result<impl IntoIterator>` + for _ in Ok::<_, ()>([0; 0]) {} + //~^ 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 unwrapping the `Result` with `?` to iterate over its contents + //~| HELP consider using `if let` to clear intent + + Ok(()) +} diff --git a/src/test/ui/lint/for_loop_over_fallibles.stderr b/src/test/ui/lint/for_loop_over_fallibles.stderr new file mode 100644 index 00000000000..96efdf85c49 --- /dev/null +++ b/src/test/ui/lint/for_loop_over_fallibles.stderr @@ -0,0 +1,101 @@ +warning: for loop over an `Option`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:5:14 + | +LL | for _ in Some(1) {} + | ^^^^^^^ + | + = note: `#[warn(for_loops_over_fallibles)]` on by default +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:9: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 an `Option`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:15:14 + | +LL | for _ in [0; 0].iter().next() {} + | ^^^^^^^^^^^^^^^^^^^^ + | +help: to iterate over `[0; 0].iter()` remove the call to `next` + | +LL | for _ in [0; 0].iter().by_ref() {} + | ~~~~~~~~~ +help: consider using `if let` to clear intent + | +LL | if let Some(_) = [0; 0].iter().next() {} + | ~~~~~~~~~~~~ ~~~ + +warning: for loop over a `Result`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:21:14 + | +LL | for _ in Ok::<_, ()>([0; 0].iter()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Ok(_) = Ok::<_, ()>([0; 0].iter()) {} + | ~~~~~~~~~~~~~ ~~~ +help: consider using `if let` to clear intent + | +LL | if let Ok(_) = Ok::<_, ()>([0; 0].iter()) {} + | ~~~~~~~~~~ ~~~ + +warning: for loop over a `Result`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:29:14 + | +LL | for _ in Ok::<_, ()>([0; 0].iter()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Ok(_) = Ok::<_, ()>([0; 0].iter()) {} + | ~~~~~~~~~~~~~ ~~~ +help: consider unwrapping the `Result` with `?` to iterate over its contents + | +LL | for _ in Ok::<_, ()>([0; 0].iter())? {} + | + +help: consider using `if let` to clear intent + | +LL | if let Ok(_) = Ok::<_, ()>([0; 0].iter()) {} + | ~~~~~~~~~~ ~~~ + +warning: for loop over a `Result`. This is more readably written as an `if let` statement + --> $DIR/for_loop_over_fallibles.rs:36:14 + | +LL | for _ in Ok::<_, ()>([0; 0]) {} + | ^^^^^^^^^^^^^^^^^^^ + | +help: to check pattern in a loop use `while let` + | +LL | while let Ok(_) = Ok::<_, ()>([0; 0]) {} + | ~~~~~~~~~~~~~ ~~~ +help: consider unwrapping the `Result` with `?` to iterate over its contents + | +LL | for _ in Ok::<_, ()>([0; 0])? {} + | + +help: consider using `if let` to clear intent + | +LL | if let Ok(_) = Ok::<_, ()>([0; 0]) {} + | ~~~~~~~~~~ ~~~ + +warning: 6 warnings emitted + diff --git a/src/test/ui/span/issue-7575.rs b/src/test/ui/span/issue-7575.rs deleted file mode 100644 index eddd158aef0..00000000000 --- a/src/test/ui/span/issue-7575.rs +++ /dev/null @@ -1,75 +0,0 @@ -// Test the mechanism for warning about possible missing `self` declarations. -trait CtxtFn { - fn f8(self, _: usize) -> usize; - fn f9(_: usize) -> usize; -} - -trait OtherTrait { - fn f9(_: usize) -> usize; -} - -// Note: this trait is not implemented, but we can't really tell -// whether or not an impl would match anyhow without a self -// declaration to match against, so we wind up prisizeing it as a -// candidate. This seems not unreasonable -- perhaps the user meant to -// implement it, after all. -trait UnusedTrait { - fn f9(_: usize) -> usize; -} - -impl CtxtFn for usize { - fn f8(self, i: usize) -> usize { - i * 4 - } - - fn f9(i: usize) -> usize { - i * 4 - } -} - -impl OtherTrait for usize { - fn f9(i: usize) -> usize { - i * 8 - } -} - -struct Myisize(isize); - -impl Myisize { - fn fff(i: isize) -> isize { - i - } -} - -trait ManyImplTrait { - fn is_str() -> bool { - false - } -} - -impl ManyImplTrait for String { - fn is_str() -> bool { - true - } -} - -impl ManyImplTrait for usize {} -impl ManyImplTrait for isize {} -impl ManyImplTrait for char {} -impl ManyImplTrait for Myisize {} - -fn no_param_bound(u: usize, m: Myisize) -> usize { - u.f8(42) + u.f9(342) + m.fff(42) - //~^ ERROR no method named `f9` found - //~| ERROR no method named `fff` found - - -} - -fn param_bound<T: ManyImplTrait>(t: T) -> bool { - t.is_str() - //~^ ERROR no method named `is_str` found -} - -fn main() { -} diff --git a/src/test/ui/span/issue-7575.stderr b/src/test/ui/span/issue-7575.stderr deleted file mode 100644 index 4f30edb3f89..00000000000 --- a/src/test/ui/span/issue-7575.stderr +++ /dev/null @@ -1,82 +0,0 @@ -error[E0599]: no method named `f9` found for type `usize` in the current scope - --> $DIR/issue-7575.rs:62:18 - | -LL | u.f8(42) + u.f9(342) + m.fff(42) - | ^^ this is an associated function, not a method - | - = note: found the following associated functions; to be used as methods, functions must have a `self` parameter -note: candidate #1 is defined in the trait `CtxtFn` - --> $DIR/issue-7575.rs:4:5 - | -LL | fn f9(_: usize) -> usize; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -note: candidate #2 is defined in the trait `OtherTrait` - --> $DIR/issue-7575.rs:8:5 - | -LL | fn f9(_: usize) -> usize; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -note: candidate #3 is defined in the trait `UnusedTrait` - --> $DIR/issue-7575.rs:17:5 - | -LL | fn f9(_: usize) -> usize; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: items from traits can only be used if the trait is implemented and in scope - = note: the following traits define an item `f9`, perhaps you need to implement one of them: - candidate #1: `CtxtFn` - candidate #2: `OtherTrait` - candidate #3: `UnusedTrait` -help: disambiguate the associated function for candidate #1 - | -LL | u.f8(42) + <usize as CtxtFn>::f9(u, 342) + m.fff(42) - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: disambiguate the associated function for candidate #2 - | -LL | u.f8(42) + <usize as OtherTrait>::f9(u, 342) + m.fff(42) - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: disambiguate the associated function for candidate #3 - | -LL | u.f8(42) + <usize as UnusedTrait>::f9(u, 342) + m.fff(42) - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -error[E0599]: no method named `fff` found for struct `Myisize` in the current scope - --> $DIR/issue-7575.rs:62:30 - | -LL | struct Myisize(isize); - | -------------- method `fff` not found for this struct -... -LL | u.f8(42) + u.f9(342) + m.fff(42) - | --^^^ - | | | - | | this is an associated function, not a method - | help: use associated function syntax instead: `Myisize::fff` - | - = note: found the following associated functions; to be used as methods, functions must have a `self` parameter -note: the candidate is defined in an impl for the type `Myisize` - --> $DIR/issue-7575.rs:39:5 - | -LL | fn fff(i: isize) -> isize { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - -error[E0599]: no method named `is_str` found for type parameter `T` in the current scope - --> $DIR/issue-7575.rs:70:7 - | -LL | fn param_bound<T: ManyImplTrait>(t: T) -> bool { - | - method `is_str` not found for this type parameter -LL | t.is_str() - | ^^^^^^ this is an associated function, not a method - | - = note: found the following associated functions; to be used as methods, functions must have a `self` parameter -note: the candidate is defined in the trait `ManyImplTrait` - --> $DIR/issue-7575.rs:45:5 - | -LL | fn is_str() -> bool { - | ^^^^^^^^^^^^^^^^^^^ - = help: items from traits can only be used if the type parameter is bounded by the trait -help: disambiguate the associated function for the candidate - | -LL | <T as ManyImplTrait>::is_str(t) - | - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0599`. diff --git a/src/test/ui/stats/hir-stats.stderr b/src/test/ui/stats/hir-stats.stderr index 350b140aada..1521b692a75 100644 --- a/src/test/ui/stats/hir-stats.stderr +++ b/src/test/ui/stats/hir-stats.stderr @@ -26,7 +26,7 @@ ast-stats-1 Block 288 ( 3.4%) 6 48 ast-stats-1 GenericBound 352 ( 4.2%) 4 88 ast-stats-1 - Trait 352 ( 4.2%) 4 ast-stats-1 AssocItem 416 ( 4.9%) 4 104 -ast-stats-1 - TyAlias 208 ( 2.5%) 2 +ast-stats-1 - Type 208 ( 2.5%) 2 ast-stats-1 - Fn 208 ( 2.5%) 2 ast-stats-1 GenericParam 480 ( 5.7%) 5 96 ast-stats-1 PathSegment 720 ( 8.6%) 30 24 @@ -84,7 +84,7 @@ ast-stats-2 Block 288 ( 3.1%) 6 48 ast-stats-2 GenericBound 352 ( 3.8%) 4 88 ast-stats-2 - Trait 352 ( 3.8%) 4 ast-stats-2 AssocItem 416 ( 4.5%) 4 104 -ast-stats-2 - TyAlias 208 ( 2.3%) 2 +ast-stats-2 - Type 208 ( 2.3%) 2 ast-stats-2 - Fn 208 ( 2.3%) 2 ast-stats-2 GenericParam 480 ( 5.2%) 5 96 ast-stats-2 PathSegment 792 ( 8.7%) 33 24 diff --git a/src/test/ui/issues/issue-43784-supertrait.rs b/src/test/ui/traits/issue-43784-supertrait.rs index 55c26ccd2da..55c26ccd2da 100644 --- a/src/test/ui/issues/issue-43784-supertrait.rs +++ b/src/test/ui/traits/issue-43784-supertrait.rs diff --git a/src/test/ui/issues/issue-43784-supertrait.stderr b/src/test/ui/traits/issue-43784-supertrait.stderr index bb890cb99ee..bb890cb99ee 100644 --- a/src/test/ui/issues/issue-43784-supertrait.stderr +++ b/src/test/ui/traits/issue-43784-supertrait.stderr diff --git a/src/tools/clippy/clippy_lints/src/lib.register_all.rs b/src/tools/clippy/clippy_lints/src/lib.register_all.rs index 5d26e4b3360..fe1f0b56646 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_all.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_all.rs @@ -109,7 +109,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(loops::EMPTY_LOOP), LintId::of(loops::EXPLICIT_COUNTER_LOOP), LintId::of(loops::FOR_KV_MAP), - LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES), LintId::of(loops::ITER_NEXT_LOOP), LintId::of(loops::MANUAL_FIND), LintId::of(loops::MANUAL_FLATTEN), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs index 05d927dbea7..306cb6a61c9 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs @@ -227,7 +227,6 @@ store.register_lints(&[ loops::EXPLICIT_INTO_ITER_LOOP, loops::EXPLICIT_ITER_LOOP, loops::FOR_KV_MAP, - loops::FOR_LOOPS_OVER_FALLIBLES, loops::ITER_NEXT_LOOP, loops::MANUAL_FIND, loops::MANUAL_FLATTEN, diff --git a/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs b/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs index 6125d0f7a86..d6d95c95c85 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs @@ -21,7 +21,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(loops::EMPTY_LOOP), - LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES), LintId::of(loops::MUT_RANGE_BOUND), LintId::of(methods::NO_EFFECT_REPLACE), LintId::of(methods::SUSPICIOUS_MAP), diff --git a/src/tools/clippy/clippy_lints/src/loops/for_loops_over_fallibles.rs b/src/tools/clippy/clippy_lints/src/loops/for_loops_over_fallibles.rs deleted file mode 100644 index 77de90fd7b9..00000000000 --- a/src/tools/clippy/clippy_lints/src/loops/for_loops_over_fallibles.rs +++ /dev/null @@ -1,65 +0,0 @@ -use super::FOR_LOOPS_OVER_FALLIBLES; -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::source::snippet; -use clippy_utils::ty::is_type_diagnostic_item; -use rustc_hir::{Expr, Pat}; -use rustc_lint::LateContext; -use rustc_span::symbol::sym; - -/// Checks for `for` loops over `Option`s and `Result`s. -pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, method_name: Option<&str>) { - let ty = cx.typeck_results().expr_ty(arg); - if is_type_diagnostic_item(cx, ty, sym::Option) { - let help_string = if let Some(method_name) = method_name { - format!( - "consider replacing `for {0} in {1}.{method_name}()` with `if let Some({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ) - } else { - format!( - "consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ) - }; - span_lint_and_help( - cx, - FOR_LOOPS_OVER_FALLIBLES, - arg.span, - &format!( - "for loop over `{0}`, which is an `Option`. This is more readably written as an \ - `if let` statement", - snippet(cx, arg.span, "_") - ), - None, - &help_string, - ); - } else if is_type_diagnostic_item(cx, ty, sym::Result) { - let help_string = if let Some(method_name) = method_name { - format!( - "consider replacing `for {0} in {1}.{method_name}()` with `if let Ok({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ) - } else { - format!( - "consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`", - snippet(cx, pat.span, "_"), - snippet(cx, arg.span, "_") - ) - }; - span_lint_and_help( - cx, - FOR_LOOPS_OVER_FALLIBLES, - arg.span, - &format!( - "for loop over `{0}`, which is a `Result`. This is more readably written as an \ - `if let` statement", - snippet(cx, arg.span, "_") - ), - None, - &help_string, - ); - } -} diff --git a/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs b/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs index e640c62ebda..b8a263817d2 100644 --- a/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs @@ -5,7 +5,7 @@ use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_span::sym; -pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool { +pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) { if is_trait_method(cx, arg, sym::Iterator) { span_lint( cx, @@ -14,8 +14,5 @@ pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool { "you are iterating over `Iterator::next()` which is an Option; this will compile but is \ probably not what you want", ); - true - } else { - false } } diff --git a/src/tools/clippy/clippy_lints/src/loops/mod.rs b/src/tools/clippy/clippy_lints/src/loops/mod.rs index c0a0444485e..bcf278d9c83 100644 --- a/src/tools/clippy/clippy_lints/src/loops/mod.rs +++ b/src/tools/clippy/clippy_lints/src/loops/mod.rs @@ -3,7 +3,6 @@ mod explicit_counter_loop; mod explicit_into_iter_loop; mod explicit_iter_loop; mod for_kv_map; -mod for_loops_over_fallibles; mod iter_next_loop; mod manual_find; mod manual_flatten; @@ -175,49 +174,6 @@ declare_clippy_lint! { declare_clippy_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 { - /// // .. - /// } - /// ``` - #[clippy::version = "1.45.0"] - pub FOR_LOOPS_OVER_FALLIBLES, - suspicious, - "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`" -} - -declare_clippy_lint! { - /// ### What it does /// Detects `loop + match` combinations that are easier /// written as a `while let` loop. /// @@ -648,7 +604,6 @@ declare_lint_pass!(Loops => [ EXPLICIT_ITER_LOOP, EXPLICIT_INTO_ITER_LOOP, ITER_NEXT_LOOP, - FOR_LOOPS_OVER_FALLIBLES, WHILE_LET_LOOP, NEEDLESS_COLLECT, EXPLICIT_COUNTER_LOOP, @@ -739,30 +694,22 @@ fn check_for_loop<'tcx>( manual_find::check(cx, pat, arg, body, span, expr); } -fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { - let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used - +fn check_for_loop_arg(cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) { if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind { let method_name = method.ident.as_str(); // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x match method_name { "iter" | "iter_mut" => { explicit_iter_loop::check(cx, self_arg, arg, method_name); - for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name)); }, "into_iter" => { explicit_iter_loop::check(cx, self_arg, arg, method_name); explicit_into_iter_loop::check(cx, self_arg, arg); - for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name)); }, "next" => { - next_loop_linted = iter_next_loop::check(cx, arg); + iter_next_loop::check(cx, arg); }, _ => {}, } } - - if !next_loop_linted { - for_loops_over_fallibles::check(cx, pat, arg, None); - } } diff --git a/src/tools/clippy/clippy_lints/src/renamed_lints.rs b/src/tools/clippy/clippy_lints/src/renamed_lints.rs index d320eea1c37..76d6ad0b23e 100644 --- a/src/tools/clippy/clippy_lints/src/renamed_lints.rs +++ b/src/tools/clippy/clippy_lints/src/renamed_lints.rs @@ -11,8 +11,8 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::disallowed_method", "clippy::disallowed_methods"), ("clippy::disallowed_type", "clippy::disallowed_types"), ("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"), - ("clippy::for_loop_over_option", "clippy::for_loops_over_fallibles"), - ("clippy::for_loop_over_result", "clippy::for_loops_over_fallibles"), + ("clippy::for_loop_over_option", "for_loops_over_fallibles"), + ("clippy::for_loop_over_result", "for_loops_over_fallibles"), ("clippy::identity_conversion", "clippy::useless_conversion"), ("clippy::if_let_some_result", "clippy::match_result_ok"), ("clippy::logic_bug", "clippy::overly_complex_bool_expr"), @@ -31,6 +31,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::to_string_in_display", "clippy::recursive_format_impl"), ("clippy::zero_width_space", "clippy::invisible_characters"), ("clippy::drop_bounds", "drop_bounds"), + ("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"), ("clippy::into_iter_on_array", "array_into_iter"), ("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"), ("clippy::invalid_ref", "invalid_value"), diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index 493991f30e8..0133997560e 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -438,14 +438,14 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) && eq_generics(lg, rg) && both(lb, rb, |l, r| eq_block(l, r)) }, ( - TyAlias(box ast::TyAlias { + Type(box ast::TyAlias { defaultness: ld, generics: lg, bounds: lb, ty: lt, .. }), - TyAlias(box ast::TyAlias { + Type(box ast::TyAlias { defaultness: rd, generics: rg, bounds: rb, diff --git a/src/tools/clippy/src/docs.rs b/src/tools/clippy/src/docs.rs index 3bf488ab477..bd27bc7938f 100644 --- a/src/tools/clippy/src/docs.rs +++ b/src/tools/clippy/src/docs.rs @@ -170,7 +170,6 @@ docs! { "fn_to_numeric_cast_any", "fn_to_numeric_cast_with_truncation", "for_kv_map", - "for_loops_over_fallibles", "forget_copy", "forget_non_drop", "forget_ref", diff --git a/src/tools/clippy/src/docs/for_loops_over_fallibles.txt b/src/tools/clippy/src/docs/for_loops_over_fallibles.txt deleted file mode 100644 index c5a7508e45d..00000000000 --- a/src/tools/clippy/src/docs/for_loops_over_fallibles.txt +++ /dev/null @@ -1,32 +0,0 @@ -### 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 -``` -for x in opt { - // .. -} - -for x in &res { - // .. -} - -for x in res.iter() { - // .. -} -``` - -Use instead: -``` -if let Some(x) = opt { - // .. -} - -if let Ok(x) = res { - // .. -} -``` \ No newline at end of file diff --git a/src/tools/clippy/tests/ui/for_loop_unfixable.rs b/src/tools/clippy/tests/ui/for_loop_unfixable.rs index efcaffce24e..55fb3788a8b 100644 --- a/src/tools/clippy/tests/ui/for_loop_unfixable.rs +++ b/src/tools/clippy/tests/ui/for_loop_unfixable.rs @@ -8,6 +8,7 @@ clippy::for_kv_map )] #[allow(clippy::linkedlist, clippy::unnecessary_mut_passed, clippy::similar_names)] +#[allow(for_loops_over_fallibles)] fn main() { let vec = vec![1, 2, 3, 4]; diff --git a/src/tools/clippy/tests/ui/for_loop_unfixable.stderr b/src/tools/clippy/tests/ui/for_loop_unfixable.stderr index f769b4bdc94..50a86eaa68f 100644 --- a/src/tools/clippy/tests/ui/for_loop_unfixable.stderr +++ b/src/tools/clippy/tests/ui/for_loop_unfixable.stderr @@ -1,5 +1,5 @@ error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want - --> $DIR/for_loop_unfixable.rs:14:15 + --> $DIR/for_loop_unfixable.rs:15:15 | LL | for _v in vec.iter().next() {} | ^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs b/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs deleted file mode 100644 index 4b2a9297d08..00000000000 --- a/src/tools/clippy/tests/ui/for_loops_over_fallibles.rs +++ /dev/null @@ -1,73 +0,0 @@ -#![warn(clippy::for_loops_over_fallibles)] -#![allow(clippy::uninlined_format_args)] - -fn for_loops_over_fallibles() { - let option = Some(1); - let mut result = option.ok_or("x not found"); - let v = vec![0, 1, 2]; - - // check over an `Option` - for x in option { - println!("{}", x); - } - - // check over an `Option` - for x in option.iter() { - println!("{}", x); - } - - // check over a `Result` - for x in result { - println!("{}", x); - } - - // check over a `Result` - for x in result.iter_mut() { - println!("{}", x); - } - - // check over a `Result` - for x in result.into_iter() { - println!("{}", x); - } - - for x in option.ok_or("x not found") { - println!("{}", x); - } - - // make sure LOOP_OVER_NEXT lint takes clippy::precedence when next() is the last call - // in the chain - for x in v.iter().next() { - println!("{}", x); - } - - // make sure we lint when next() is not the last call in the chain - for x in v.iter().next().and(Some(0)) { - println!("{}", x); - } - - for x in v.iter().next().ok_or("x not found") { - println!("{}", x); - } - - // check for false positives - - // for loop false positive - for x in v { - println!("{}", x); - } - - // while let false positive for Option - while let Some(x) = option { - println!("{}", x); - break; - } - - // while let false positive for Result - while let Ok(x) = result { - println!("{}", x); - break; - } -} - -fn main() {} diff --git a/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr b/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr deleted file mode 100644 index f09adccabd1..00000000000 --- a/src/tools/clippy/tests/ui/for_loops_over_fallibles.stderr +++ /dev/null @@ -1,95 +0,0 @@ -error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:10:14 - | -LL | for x in option { - | ^^^^^^ - | - = help: consider replacing `for x in option` with `if let Some(x) = option` - = note: `-D clippy::for-loops-over-fallibles` implied by `-D warnings` - -error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:15:14 - | -LL | for x in option.iter() { - | ^^^^^^ - | - = help: consider replacing `for x in option.iter()` with `if let Some(x) = option` - -error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:20:14 - | -LL | for x in result { - | ^^^^^^ - | - = help: consider replacing `for x in result` with `if let Ok(x) = result` - -error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:25:14 - | -LL | for x in result.iter_mut() { - | ^^^^^^ - | - = help: consider replacing `for x in result.iter_mut()` with `if let Ok(x) = result` - -error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:30:14 - | -LL | for x in result.into_iter() { - | ^^^^^^ - | - = help: consider replacing `for x in result.into_iter()` with `if let Ok(x) = result` - -error: for loop over `option.ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:34:14 - | -LL | for x in option.ok_or("x not found") { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider replacing `for x in option.ok_or("x not found")` with `if let Ok(x) = option.ok_or("x not found")` - -error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want - --> $DIR/for_loops_over_fallibles.rs:40:14 - | -LL | for x in v.iter().next() { - | ^^^^^^^^^^^^^^^ - | - = note: `#[deny(clippy::iter_next_loop)]` on by default - -error: for loop over `v.iter().next().and(Some(0))`, which is an `Option`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:45:14 - | -LL | for x in v.iter().next().and(Some(0)) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider replacing `for x in v.iter().next().and(Some(0))` with `if let Some(x) = v.iter().next().and(Some(0))` - -error: for loop over `v.iter().next().ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement - --> $DIR/for_loops_over_fallibles.rs:49:14 - | -LL | for x in v.iter().next().ok_or("x not found") { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider replacing `for x in v.iter().next().ok_or("x not found")` with `if let Ok(x) = v.iter().next().ok_or("x not found")` - -error: this loop never actually loops - --> $DIR/for_loops_over_fallibles.rs:61:5 - | -LL | / while let Some(x) = option { -LL | | println!("{}", x); -LL | | break; -LL | | } - | |_____^ - | - = note: `#[deny(clippy::never_loop)]` on by default - -error: this loop never actually loops - --> $DIR/for_loops_over_fallibles.rs:67:5 - | -LL | / while let Ok(x) = result { -LL | | println!("{}", x); -LL | | break; -LL | | } - | |_____^ - -error: aborting due to 11 previous errors - diff --git a/src/tools/clippy/tests/ui/manual_map_option.fixed b/src/tools/clippy/tests/ui/manual_map_option.fixed index a59da4ae10b..e12ea7ec145 100644 --- a/src/tools/clippy/tests/ui/manual_map_option.fixed +++ b/src/tools/clippy/tests/ui/manual_map_option.fixed @@ -7,7 +7,7 @@ clippy::unit_arg, clippy::match_ref_pats, clippy::redundant_pattern_matching, - clippy::for_loops_over_fallibles, + for_loops_over_fallibles, dead_code )] diff --git a/src/tools/clippy/tests/ui/manual_map_option.rs b/src/tools/clippy/tests/ui/manual_map_option.rs index 0bdbefa51e8..325a6db06c4 100644 --- a/src/tools/clippy/tests/ui/manual_map_option.rs +++ b/src/tools/clippy/tests/ui/manual_map_option.rs @@ -7,7 +7,7 @@ clippy::unit_arg, clippy::match_ref_pats, clippy::redundant_pattern_matching, - clippy::for_loops_over_fallibles, + for_loops_over_fallibles, dead_code )] diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed index a6e7bdba77c..8beae8dee08 100644 --- a/src/tools/clippy/tests/ui/rename.fixed +++ b/src/tools/clippy/tests/ui/rename.fixed @@ -12,7 +12,7 @@ #![allow(clippy::disallowed_methods)] #![allow(clippy::disallowed_types)] #![allow(clippy::mixed_read_write_in_expression)] -#![allow(clippy::for_loops_over_fallibles)] +#![allow(for_loops_over_fallibles)] #![allow(clippy::useless_conversion)] #![allow(clippy::match_result_ok)] #![allow(clippy::overly_complex_bool_expr)] @@ -45,8 +45,8 @@ #![warn(clippy::disallowed_methods)] #![warn(clippy::disallowed_types)] #![warn(clippy::mixed_read_write_in_expression)] -#![warn(clippy::for_loops_over_fallibles)] -#![warn(clippy::for_loops_over_fallibles)] +#![warn(for_loops_over_fallibles)] +#![warn(for_loops_over_fallibles)] #![warn(clippy::useless_conversion)] #![warn(clippy::match_result_ok)] #![warn(clippy::overly_complex_bool_expr)] @@ -65,6 +65,7 @@ #![warn(clippy::recursive_format_impl)] #![warn(clippy::invisible_characters)] #![warn(drop_bounds)] +#![warn(for_loops_over_fallibles)] #![warn(array_into_iter)] #![warn(invalid_atomic_ordering)] #![warn(invalid_value)] diff --git a/src/tools/clippy/tests/ui/rename.rs b/src/tools/clippy/tests/ui/rename.rs index e8f57597d02..9e665047baa 100644 --- a/src/tools/clippy/tests/ui/rename.rs +++ b/src/tools/clippy/tests/ui/rename.rs @@ -12,7 +12,7 @@ #![allow(clippy::disallowed_methods)] #![allow(clippy::disallowed_types)] #![allow(clippy::mixed_read_write_in_expression)] -#![allow(clippy::for_loops_over_fallibles)] +#![allow(for_loops_over_fallibles)] #![allow(clippy::useless_conversion)] #![allow(clippy::match_result_ok)] #![allow(clippy::overly_complex_bool_expr)] @@ -65,6 +65,7 @@ #![warn(clippy::to_string_in_display)] #![warn(clippy::zero_width_space)] #![warn(clippy::drop_bounds)] +#![warn(clippy::for_loops_over_fallibles)] #![warn(clippy::into_iter_on_array)] #![warn(clippy::invalid_atomic_ordering)] #![warn(clippy::invalid_ref)] diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr index 31865a7f66d..63eb565185f 100644 --- a/src/tools/clippy/tests/ui/rename.stderr +++ b/src/tools/clippy/tests/ui/rename.stderr @@ -54,17 +54,17 @@ error: lint `clippy::eval_order_dependence` has been renamed to `clippy::mixed_r LL | #![warn(clippy::eval_order_dependence)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::mixed_read_write_in_expression` -error: lint `clippy::for_loop_over_option` has been renamed to `clippy::for_loops_over_fallibles` +error: lint `clippy::for_loop_over_option` has been renamed to `for_loops_over_fallibles` --> $DIR/rename.rs:48:9 | LL | #![warn(clippy::for_loop_over_option)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` -error: lint `clippy::for_loop_over_result` has been renamed to `clippy::for_loops_over_fallibles` +error: lint `clippy::for_loop_over_result` has been renamed to `for_loops_over_fallibles` --> $DIR/rename.rs:49:9 | LL | #![warn(clippy::for_loop_over_result)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion` --> $DIR/rename.rs:50:9 @@ -174,59 +174,65 @@ error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` LL | #![warn(clippy::drop_bounds)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds` -error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` +error: lint `clippy::for_loops_over_fallibles` has been renamed to `for_loops_over_fallibles` --> $DIR/rename.rs:68:9 | +LL | #![warn(clippy::for_loops_over_fallibles)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` + +error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` + --> $DIR/rename.rs:69:9 + | LL | #![warn(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter` error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering` - --> $DIR/rename.rs:69:9 + --> $DIR/rename.rs:70:9 | LL | #![warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` error: lint `clippy::invalid_ref` has been renamed to `invalid_value` - --> $DIR/rename.rs:70:9 + --> $DIR/rename.rs:71:9 | LL | #![warn(clippy::invalid_ref)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value` error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums` - --> $DIR/rename.rs:71:9 + --> $DIR/rename.rs:72:9 | LL | #![warn(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums` error: lint `clippy::panic_params` has been renamed to `non_fmt_panics` - --> $DIR/rename.rs:72:9 + --> $DIR/rename.rs:73:9 | LL | #![warn(clippy::panic_params)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics` error: lint `clippy::positional_named_format_parameters` has been renamed to `named_arguments_used_positionally` - --> $DIR/rename.rs:73:9 + --> $DIR/rename.rs:74:9 | LL | #![warn(clippy::positional_named_format_parameters)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally` error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` - --> $DIR/rename.rs:74:9 + --> $DIR/rename.rs:75:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints` - --> $DIR/rename.rs:75:9 + --> $DIR/rename.rs:76:9 | LL | #![warn(clippy::unknown_clippy_lints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints` error: lint `clippy::unused_label` has been renamed to `unused_labels` - --> $DIR/rename.rs:76:9 + --> $DIR/rename.rs:77:9 | LL | #![warn(clippy::unused_label)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels` -error: aborting due to 38 previous errors +error: aborting due to 39 previous errors diff --git a/src/tools/rustfmt/src/items.rs b/src/tools/rustfmt/src/items.rs index 8f35068e35f..a2a73f0a5fb 100644 --- a/src/tools/rustfmt/src/items.rs +++ b/src/tools/rustfmt/src/items.rs @@ -594,7 +594,7 @@ impl<'a> FmtVisitor<'a> { let both_type = |l: &TyOpt, r: &TyOpt| is_type(l) && is_type(r); let both_opaque = |l: &TyOpt, r: &TyOpt| is_opaque(l) && is_opaque(r); let need_empty_line = |a: &ast::AssocItemKind, b: &ast::AssocItemKind| match (a, b) { - (TyAlias(lty), TyAlias(rty)) + (Type(lty), Type(rty)) if both_type(<y.ty, &rty.ty) || both_opaque(<y.ty, &rty.ty) => { false @@ -612,7 +612,7 @@ impl<'a> FmtVisitor<'a> { } buffer.sort_by(|(_, a), (_, b)| match (&a.kind, &b.kind) { - (TyAlias(lty), TyAlias(rty)) + (Type(lty), Type(rty)) if both_type(<y.ty, &rty.ty) || both_opaque(<y.ty, &rty.ty) => { a.ident.as_str().cmp(b.ident.as_str()) @@ -621,10 +621,10 @@ impl<'a> FmtVisitor<'a> { a.ident.as_str().cmp(b.ident.as_str()) } (Fn(..), Fn(..)) => a.span.lo().cmp(&b.span.lo()), - (TyAlias(ty), _) if is_type(&ty.ty) => Ordering::Less, - (_, TyAlias(ty)) if is_type(&ty.ty) => Ordering::Greater, - (TyAlias(..), _) => Ordering::Less, - (_, TyAlias(..)) => Ordering::Greater, + (Type(ty), _) if is_type(&ty.ty) => Ordering::Less, + (_, Type(ty)) if is_type(&ty.ty) => Ordering::Greater, + (Type(..), _) => Ordering::Less, + (_, Type(..)) => Ordering::Greater, (Const(..), _) => Ordering::Less, (_, Const(..)) => Ordering::Greater, (MacCall(..), _) => Ordering::Less, diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index 7bb745eeb8b..9c3cc7820d2 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -660,7 +660,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_rewrite(ai.span, rewrite); } } - (ast::AssocItemKind::TyAlias(ref ty_alias), _) => { + (ast::AssocItemKind::Type(ref ty_alias), _) => { self.visit_ty_alias_kind(ty_alias, visitor_kind, ai.span); } (ast::AssocItemKind::MacCall(ref mac), _) => { diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 969d5fec60f..052abfdab5d 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,8 +7,8 @@ use std::path::Path; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. -const ROOT_ENTRY_LIMIT: usize = 968; -const ISSUES_ENTRY_LIMIT: usize = 2147; +const ROOT_ENTRY_LIMIT: usize = 950; +const ISSUES_ENTRY_LIMIT: usize = 2141; fn check_entries(path: &Path, bad: &mut bool) { let dirs = walkdir::WalkDir::new(&path.join("test/ui")) |