Fix single_match lint being emitted when it should not

Author: GuillaumeGomez

clippy_lints/src/matches/mod.rs

@@ -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;
@@ -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);

clippy_utils/src/lib.rs

@@ -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 {

tests/ui/single_match.fixed

@@ -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() };
@@ -318,5 +324,25 @@ fn irrefutable_match() {
 
 
 
-    println!()
+    println!();
+
+    let mut x = vec![1i8];
+
+    // Should not lint.
+    match x.pop() {
+        // bla
+        Some(u) => println!("{u}"),
+        // more comments!
+        None => {},
+    }
+    // Should not lint.
+    match x.pop() {
+        // bla
+        Some(u) => {
+            // bla
+            println!("{u}");
+        },
+        // bla
+        None => {},
+    }
 }

tests/ui/single_match.rs

@@ -401,4 +401,24 @@ 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 => {},
+    }
+    // Should not lint.
+    match x.pop() {
+        // bla
+        Some(u) => {
+            // bla
+            println!("{u}");
+        },
+        // bla
+        None => {},
+    }
 }

tests/ui/single_match.stderr

@@ -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
    |
@@ -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