about summary refs log tree commit diff
diff options
context:
space:
mode:
authorllogiq <bogusandre@gmail.com>2024-12-13 16:52:31 +0000
committerGitHub <noreply@github.com>2024-12-13 16:52:31 +0000
commitc607408df56146d875e292966a2ea1488bed4981 (patch)
tree8c201d839868685df2bdcaec2a4a91ae17ab4fa0
parentf2aed50873e8e908119044c283c33cf6ef0b66e0 (diff)
parentefe3fe9b8c5f16bbf39af7415bbde9441bc54dbb (diff)
downloadrust-c607408df56146d875e292966a2ea1488bed4981.tar.gz
rust-c607408df56146d875e292966a2ea1488bed4981.zip
Fix `single_match` lint being emitted when it should not (#13765)
We realized when running `clippy --fix` on rustdoc (PR comment
[here](https://github.com/rust-lang/rust/pull/133537/files#r1861377721))
that some comments were removed, which is problematic. This PR checks
that comments outside of `match` arms are taken into account before
emitting the lint.

changelog: Fix `single_match` lint being emitted when it should not
-rw-r--r--clippy_lints/src/matches/mod.rs27
-rw-r--r--clippy_utils/src/lib.rs14
-rw-r--r--tests/ui/single_match.fixed30
-rw-r--r--tests/ui/single_match.rs20
-rw-r--r--tests/ui/single_match.stderr16
5 files changed, 85 insertions, 22 deletions
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
index 64969271764..1fd2ebcb54a 100644
--- a/clippy_lints/src/matches/mod.rs
+++ b/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);
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 434c26d687d..d57b872c759 100644
--- a/clippy_utils/src/lib.rs
+++ b/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 {
diff --git a/tests/ui/single_match.fixed b/tests/ui/single_match.fixed
index 4016b2699d6..3fb9afa86a5 100644
--- a/tests/ui/single_match.fixed
+++ b/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 => {},
+    }
 }
diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs
index 75edaa60605..b7d2e011151 100644
--- a/tests/ui/single_match.rs
+++ b/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 => {},
+    }
 }
diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr
index 9240b09c50a..d691b627abe 100644
--- a/tests/ui/single_match.stderr
+++ b/tests/ui/single_match.stderr
@@ -19,18 +19,6 @@ 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
    |
 LL | /     match z {
@@ -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