From 8dd310ddd7524223c1405d22155ab41b546c746a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 17 Jul 2024 20:59:46 +0200 Subject: [PATCH] Fix wrong suggestion for `single_element_loop` where parens were missing --- clippy_lints/src/loops/single_element_loop.rs | 7 +++++-- tests/ui/single_element_loop.fixed | 8 ++++++++ tests/ui/single_element_loop.rs | 7 +++++++ tests/ui/single_element_loop.stderr | 18 +++++++++++++++++- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index 108fdb69775..2b41911e8ec 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -5,7 +5,7 @@ use rustc_ast::util::parser::PREC_PREFIX; use rustc_ast::Mutability; use rustc_errors::Applicability; -use rustc_hir::{is_range_literal, BorrowKind, Expr, ExprKind, Pat}; +use rustc_hir::{is_range_literal, BorrowKind, Expr, ExprKind, Pat, PatKind}; use rustc_lint::LateContext; use rustc_span::edition::Edition; use rustc_span::sym; @@ -70,7 +70,10 @@ pub(super) fn check<'tcx>( && !contains_break_or_continue(body) { let mut applicability = Applicability::MachineApplicable; - let pat_snip = snippet_with_applicability(cx, pat.span, "..", &mut applicability); + let mut pat_snip = snippet_with_applicability(cx, pat.span, "..", &mut applicability); + if matches!(pat.kind, PatKind::Or(..)) { + pat_snip = format!("({pat_snip})").into(); + } let mut arg_snip = snippet_with_applicability(cx, arg_expression.span, "..", &mut applicability); let mut block_str = snippet_with_applicability(cx, block.span, "..", &mut applicability).into_owned(); block_str.remove(0); diff --git a/tests/ui/single_element_loop.fixed b/tests/ui/single_element_loop.fixed index 4e59c763198..64cbd5e9c90 100644 --- a/tests/ui/single_element_loop.fixed +++ b/tests/ui/single_element_loop.fixed @@ -57,4 +57,12 @@ fn main() { } }; } + + // Should lint with correct suggestion (issue #12782) + let res_void: Result = Ok(true); + + { + let (Ok(mut _x) | Err(mut _x)) = res_void; + let ptr: *const bool = std::ptr::null(); + } } diff --git a/tests/ui/single_element_loop.rs b/tests/ui/single_element_loop.rs index a55ece6b065..92406f1c1ca 100644 --- a/tests/ui/single_element_loop.rs +++ b/tests/ui/single_element_loop.rs @@ -54,4 +54,11 @@ fn main() { } }; } + + // Should lint with correct suggestion (issue #12782) + let res_void: Result = Ok(true); + + for (Ok(mut _x) | Err(mut _x)) in [res_void] { + let ptr: *const bool = std::ptr::null(); + } } diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index dfae5605b57..73453dd2dfd 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -83,5 +83,21 @@ LL + }; LL + } | -error: aborting due to 7 previous errors +error: for loop over a single element + --> tests/ui/single_element_loop.rs:61:5 + | +LL | / for (Ok(mut _x) | Err(mut _x)) in [res_void] { +LL | | let ptr: *const bool = std::ptr::null(); +LL | | } + | |_____^ + | +help: try + | +LL ~ { +LL + let (Ok(mut _x) | Err(mut _x)) = res_void; +LL + let ptr: *const bool = std::ptr::null(); +LL + } + | + +error: aborting due to 8 previous errors