From 04d70d04fcf9412d7006ee4646f56f8bf5f2149f Mon Sep 17 00:00:00 2001 From: WeiTheShinobi Date: Sat, 31 Aug 2024 05:02:07 +0800 Subject: [PATCH 1/2] [`single_match`, `single_match_else`] fix suggestion when `match` irrefutable --- clippy_lints/src/matches/single_match.rs | 7 +++++ tests/ui/single_match.fixed | 18 +++++++++++ tests/ui/single_match.rs | 27 ++++++++++++++++ tests/ui/single_match.stderr | 40 +++++++++++++++++++++++- tests/ui/single_match_else.fixed | 4 +++ tests/ui/single_match_else.rs | 10 ++++++ tests/ui/single_match_else.stderr | 14 ++++++++- 7 files changed, 118 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index b6930f7b9d10..4bd46bced38e 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -91,6 +91,13 @@ fn report_single_pattern(cx: &LateContext<'_>, ex: &Expr<'_>, arm: &Arm<'_>, exp format!(" else {}", expr_block(cx, els, ctxt, "..", Some(expr.span), &mut app)) }); + if snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") { + let msg = "this pattern is irrefutable, `match` is useless"; + let sugg = expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app); + span_lint_and_sugg(cx, lint, expr.span, msg, "try", sugg, app); + return; + } + let (pat, pat_ref_count) = peel_hir_pat_refs(arm.pat); let (msg, sugg) = if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind && let (ty, ty_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(ex)) diff --git a/tests/ui/single_match.fixed b/tests/ui/single_match.fixed index 5249c4408899..04129ec0f13b 100644 --- a/tests/ui/single_match.fixed +++ b/tests/ui/single_match.fixed @@ -296,3 +296,21 @@ fn issue11365() { if let Some(A | B) = &Some(A) { println!() } } + +#[derive(Eq, PartialEq)] +pub struct Data([u8; 4]); + +const DATA: Data = Data([1, 2, 3, 4]); +const CONST_I32: i32 = 1; + +fn irrefutable_match() { + { println!() } + + { println!() } + + let i = 0; + { + let a = 1; + let b = 2; + } +} diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index 882098a56e78..2d51cee3fd99 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -360,3 +360,30 @@ fn issue11365() { None | Some(_) => {}, } } + +#[derive(Eq, PartialEq)] +pub struct Data([u8; 4]); + +const DATA: Data = Data([1, 2, 3, 4]); +const CONST_I32: i32 = 1; + +fn irrefutable_match() { + match DATA { + DATA => println!(), + _ => {}, + } + + match CONST_I32 { + CONST_I32 => println!(), + _ => {}, + } + + let i = 0; + match i { + i => { + let a = 1; + let b = 2; + }, + _ => {}, + } +} diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index ceb2a193bf7b..9578421d2cec 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -216,5 +216,43 @@ LL | | None | Some(_) => {}, LL | | } | |_____^ help: try: `if let Some(A | B) = &Some(A) { println!() }` -error: aborting due to 20 previous errors +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:371:5 + | +LL | / match DATA { +LL | | DATA => println!(), +LL | | _ => {}, +LL | | } + | |_____^ help: try: `{ println!() }` + +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:376:5 + | +LL | / match CONST_I32 { +LL | | CONST_I32 => println!(), +LL | | _ => {}, +LL | | } + | |_____^ help: try: `{ println!() }` + +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:382:5 + | +LL | / match i { +LL | | i => { +LL | | let a = 1; +LL | | let b = 2; +LL | | }, +LL | | _ => {}, +LL | | } + | |_____^ + | +help: try + | +LL ~ { +LL + let a = 1; +LL + let b = 2; +LL + } + | + +error: aborting due to 23 previous errors diff --git a/tests/ui/single_match_else.fixed b/tests/ui/single_match_else.fixed index 163be16ad8be..fb57411aaa42 100644 --- a/tests/ui/single_match_else.fixed +++ b/tests/ui/single_match_else.fixed @@ -171,3 +171,7 @@ fn issue_10808(bar: Option) { }, } } + +fn irrefutable_match() -> Option<&'static ExprNode> { + { Some(&NODE) } +} diff --git a/tests/ui/single_match_else.rs b/tests/ui/single_match_else.rs index 3f1fd2b31832..2d9e877ee0fe 100644 --- a/tests/ui/single_match_else.rs +++ b/tests/ui/single_match_else.rs @@ -199,3 +199,13 @@ fn issue_10808(bar: Option) { }, } } + +fn irrefutable_match() -> Option<&'static ExprNode> { + match ExprNode::Butterflies { + ExprNode::Butterflies => Some(&NODE), + _ => { + let x = 5; + None + }, + } +} diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 61c348260d05..61209053fd0f 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -197,5 +197,17 @@ LL + println!("None"); LL + } | -error: aborting due to 9 previous errors +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match_else.rs:204:5 + | +LL | / match ExprNode::Butterflies { +LL | | ExprNode::Butterflies => Some(&NODE), +LL | | _ => { +LL | | let x = 5; +LL | | None +LL | | }, +LL | | } + | |_____^ help: try: `{ Some(&NODE) }` + +error: aborting due to 10 previous errors From e3ca249e9647396b9a3849e147aa53e168cf4c73 Mon Sep 17 00:00:00 2001 From: WeiTheShinobi Date: Fri, 6 Sep 2024 23:10:42 +0800 Subject: [PATCH 2/2] fix `single_match` suggestion --- clippy_lints/src/matches/single_match.rs | 24 ++++++++++++++--- tests/ui/single_match.fixed | 10 +++++-- tests/ui/single_match.rs | 15 +++++++++++ tests/ui/single_match.stderr | 33 +++++++++++++++++++++--- tests/ui/single_match_else.fixed | 2 +- tests/ui/single_match_else.stderr | 2 +- 6 files changed, 75 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index 4bd46bced38e..e7cef5bdbd76 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{expr_block, snippet, SpanRangeExt}; +use clippy_utils::source::{expr_block, snippet, snippet_block_with_context, SpanRangeExt}; use clippy_utils::ty::implements_trait; use clippy_utils::{ is_lint_allowed, is_unit_expr, peel_blocks, peel_hir_pat_refs, peel_middle_ty_refs, peel_n_hir_expr_refs, @@ -9,7 +9,7 @@ use rustc_arena::DroplessArena; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{walk_pat, Visitor}; -use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind, QPath}; +use rustc_hir::{Arm, Expr, ExprKind, HirId, Node, Pat, PatKind, QPath, StmtKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self, AdtDef, ParamEnv, TyCtxt, TypeckResults, VariantDef}; use rustc_span::{sym, Span}; @@ -93,8 +93,24 @@ fn report_single_pattern(cx: &LateContext<'_>, ex: &Expr<'_>, arm: &Arm<'_>, exp if snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") { let msg = "this pattern is irrefutable, `match` is useless"; - let sugg = expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app); - span_lint_and_sugg(cx, lint, expr.span, msg, "try", sugg, app); + let (sugg, help) = if is_unit_expr(arm.body) { + (String::new(), "`match` expression can be removed") + } else { + let mut sugg = snippet_block_with_context(cx, arm.body.span, ctxt, "..", Some(expr.span), &mut app) + .0 + .to_string(); + if let Node::Stmt(stmt) = cx.tcx.parent_hir_node(expr.hir_id) + && let StmtKind::Expr(_) = stmt.kind + && match arm.body.kind { + ExprKind::Block(block, _) => block.span.from_expansion(), + _ => true, + } + { + sugg.push(';'); + } + (sugg, "try") + }; + span_lint_and_sugg(cx, lint, expr.span, msg, help, sugg.to_string(), app); return; } diff --git a/tests/ui/single_match.fixed b/tests/ui/single_match.fixed index 04129ec0f13b..dcf5a1d33c2a 100644 --- a/tests/ui/single_match.fixed +++ b/tests/ui/single_match.fixed @@ -304,13 +304,19 @@ const DATA: Data = Data([1, 2, 3, 4]); const CONST_I32: i32 = 1; fn irrefutable_match() { - { println!() } + println!(); - { println!() } + println!(); let i = 0; { let a = 1; let b = 2; } + + + + + + println!() } diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index 2d51cee3fd99..3ba5eebd01b7 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -386,4 +386,19 @@ fn irrefutable_match() { }, _ => {}, } + + match i { + i => {}, + _ => {}, + } + + match i { + i => (), + _ => (), + } + + match CONST_I32 { + CONST_I32 => println!(), + _ => {}, + } } diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index 9578421d2cec..9240b09c50a9 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -223,7 +223,7 @@ LL | / match DATA { LL | | DATA => println!(), LL | | _ => {}, LL | | } - | |_____^ help: try: `{ println!() }` + | |_____^ help: try: `println!();` error: this pattern is irrefutable, `match` is useless --> tests/ui/single_match.rs:376:5 @@ -232,7 +232,7 @@ LL | / match CONST_I32 { LL | | CONST_I32 => println!(), LL | | _ => {}, LL | | } - | |_____^ help: try: `{ println!() }` + | |_____^ help: try: `println!();` error: this pattern is irrefutable, `match` is useless --> tests/ui/single_match.rs:382:5 @@ -254,5 +254,32 @@ LL + let b = 2; LL + } | -error: aborting due to 23 previous errors +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:390:5 + | +LL | / match i { +LL | | i => {}, +LL | | _ => {}, +LL | | } + | |_____^ help: `match` expression can be removed + +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:395:5 + | +LL | / match i { +LL | | i => (), +LL | | _ => (), +LL | | } + | |_____^ help: `match` expression can be removed + +error: this pattern is irrefutable, `match` is useless + --> tests/ui/single_match.rs:400:5 + | +LL | / match CONST_I32 { +LL | | CONST_I32 => println!(), +LL | | _ => {}, +LL | | } + | |_____^ help: try: `println!()` + +error: aborting due to 26 previous errors diff --git a/tests/ui/single_match_else.fixed b/tests/ui/single_match_else.fixed index fb57411aaa42..c2ca746976bd 100644 --- a/tests/ui/single_match_else.fixed +++ b/tests/ui/single_match_else.fixed @@ -173,5 +173,5 @@ fn issue_10808(bar: Option) { } fn irrefutable_match() -> Option<&'static ExprNode> { - { Some(&NODE) } + Some(&NODE) } diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 61209053fd0f..a2801751a430 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -207,7 +207,7 @@ LL | | let x = 5; LL | | None LL | | }, LL | | } - | |_____^ help: try: `{ Some(&NODE) }` + | |_____^ help: try: `Some(&NODE)` error: aborting due to 10 previous errors