Skip to content

Commit

Permalink
Fix single_match lint being emitted when it should not
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Nov 30, 2024
1 parent 650e0c8 commit 436be6a
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 22 deletions.
27 changes: 25 additions & 2 deletions clippy_lints/src/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ mod wild_in_or_pats;
use clippy_config::Conf;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::walk_span_to_context;
use clippy_utils::{higher, is_direct_expn_of, is_in_const_context, is_span_match, span_contains_cfg};
use clippy_utils::{
higher, is_direct_expn_of, is_in_const_context, is_span_match, span_contains_cfg, span_extract_comments,
};
use rustc_hir::{Arm, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -1059,7 +1061,28 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
}

redundant_pattern_match::check_match(cx, expr, ex, arms);
single_match::check(cx, ex, arms, expr);
let source_map = cx.tcx.sess.source_map();
let mut match_comments = span_extract_comments(source_map, expr.span);
// We remove comments from inside arms block.
if !match_comments.is_empty() {
for arm in arms {
for comment in span_extract_comments(source_map, arm.body.span) {
if let Some(index) = match_comments
.iter()
.enumerate()
.find(|(_, cm)| **cm == comment)
.map(|(index, _)| index)
{
match_comments.remove(index);
}
}
}
}
// If there are still comments, it means they are outside of the arms, therefore
// we should not lint.
if match_comments.is_empty() {
single_match::check(cx, ex, arms, expr);
}
match_bool::check(cx, ex, arms, expr);
overlapping_arms::check(cx, ex, arms);
match_wild_enum::check(cx, ex, arms);
Expand Down
14 changes: 10 additions & 4 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2977,12 +2977,18 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool {
///
/// Comments are returned wrapped with their relevant delimiters
pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String {
span_extract_comments(sm, span).join("\n")
}

/// Returns all the comments a given span contains.
///
/// Comments are returned wrapped with their relevant delimiters.
pub fn span_extract_comments(sm: &SourceMap, span: Span) -> Vec<String> {
let snippet = sm.span_to_snippet(span).unwrap_or_default();
let res = tokenize_with_text(&snippet)
tokenize_with_text(&snippet)
.filter(|(t, ..)| matches!(t, TokenKind::BlockComment { .. } | TokenKind::LineComment { .. }))
.map(|(_, s, _)| s)
.join("\n");
res
.map(|(_, s, _)| s.to_string())
.collect::<Vec<_>>()
}

pub fn span_find_starting_semi(sm: &SourceMap, span: Span) -> Span {
Expand Down
20 changes: 18 additions & 2 deletions tests/ui/single_match.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ fn single_match() {
};

let x = Some(1u8);
if let Some(y) = x { println!("{:?}", y) }
match x {
// Note the missing block braces.
// We suggest `if let Some(y) = x { .. }` because the macro
// is expanded before we can do anything.
Some(y) => println!("{:?}", y),
_ => (),
}

let z = (1u8, 1u8);
if let (2..=3, 7..=9) = z { dummy() };
Expand Down Expand Up @@ -318,5 +324,15 @@ fn irrefutable_match() {



println!()
println!();

let mut x = vec![1i8];

// Should not lint.
match x.pop() {
// bla
Some(u) => println!("{u}"),
// more comments!
None => {},
}
}
10 changes: 10 additions & 0 deletions tests/ui/single_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,4 +401,14 @@ fn irrefutable_match() {
CONST_I32 => println!(),
_ => {},
}

let mut x = vec![1i8];

// Should not lint.
match x.pop() {
// bla
Some(u) => println!("{u}"),
// more comments!
None => {},
}
}
16 changes: 2 additions & 14 deletions tests/ui/single_match.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,6 @@ LL + println!("{:?}", y);
LL ~ };
|

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> tests/ui/single_match.rs:23:5
|
LL | / match x {
LL | | // Note the missing block braces.
LL | | // We suggest `if let Some(y) = x { .. }` because the macro
LL | | // is expanded before we can do anything.
LL | | Some(y) => println!("{:?}", y),
LL | | _ => (),
LL | | }
| |_____^ help: try: `if let Some(y) = x { println!("{:?}", y) }`

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> tests/ui/single_match.rs:32:5
|
Expand Down Expand Up @@ -279,7 +267,7 @@ LL | / match CONST_I32 {
LL | | CONST_I32 => println!(),
LL | | _ => {},
LL | | }
| |_____^ help: try: `println!()`
| |_____^ help: try: `println!();`

error: aborting due to 26 previous errors
error: aborting due to 25 previous errors

0 comments on commit 436be6a

Please sign in to comment.