match_same_arms: FP if arms have different comments

[xFrednet]

Summary

match_same_arms lints on two arms, if the comments are different.

Lint Name

match_same_arms

Reproducer

I tried this code:

#![warn(clippy::pedantic)]

fn main() {
    let test = Some(12);
    match test {
        Some(_) => {
            // My explaination for something cool
            println!("Test code");
        },
        None => {
            // My explaination
            println!("Test code");
        },
    }
}

Playground

I saw this happen:

warning: this match arm has an identical body to another arm
  --> src/main.rs:10:9
   |
10 |           None => {
   |           ^---
   |           |
   |  _________help: try merging the arm patterns: `None | Some(_)`
   | |
11 | |             // My explaination
12 | |             println!("Test code");
13 | |         },
   | |_________^
   |
   = help: or try changing either arm body
note: other arm here
  --> src/main.rs:6:9
   |
6  | /         Some(_) => {
7  | |             // My explaination for something cool
8  | |             println!("Test code");
9  | |         },
   | |_________^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
note: the lint level is defined here
  --> src/main.rs:1:9
   |
1  | #![warn(clippy::pedantic)]
   |         ^^^^^^^^^^^^^^^^
   = note: `#[warn(clippy::match_same_arms)]` implied by `#[warn(clippy::pedantic)]`

I expected to see this happen:

No warning

Version

rustc 1.77.0-nightly (89e2160c4 2023-12-27)
binary: rustc
commit-hash: 89e2160c4ca5808657ed55392620ed1dbbce78d1
commit-date: 2023-12-27
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6

Additional Labels

These utils might be good to look at:

• https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.span_contains_comment.html
• https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.span_extract_comment.html

@rustbot label +good-first-issue

[ARandomDev99]

@rustbot claim

[ARandomDev99]

I got the basic code ready but am facing trouble with the UI tests:

let _ = match Abc::A {
    Abc::A => 0, //~ ERROR: this match arm has an identical body to the `_` wildcard arm
    Abc::B => 1,
    _ => 0,
};

The comment marking the error causes the lint to not raise any error in this part. There are several such occurrences in the UI tests. Any suggestions on how to overcome this issue?

[PartiallyUntyped]

@ARandomDev99 you can open a draft, and it'd be easier for us to help.

What do you mean with:

causes the lint to not raise any error in this part

[ARandomDev99]

@partiallytyped

let _ = match Abc::A {
    Abc::A => 0, //~ ERROR: this match arm has an identical body to the `_` wildcard arm
    Abc::B => 1,
    _ => 0,
};

I'll try to explain it the best I can. The comment here marks the expected message the emitted lint will display. However, the changes I've made take comments into account while deciding if two match arms are same or not which results in the UI test failing as these two arms have different comments and will not emit any lint.

[PartiallyUntyped]

Okay I see.

You can tell the error comment how many lines above the error is using ^.

let _ = match Abc::A {
    Abc::A => 0,
    Abc::B => 1,
    _ => 0,
};
//~^^^^ ERROR: this match arm has an identical body to the `_` wildcard arm

[ARandomDev99]

@partiallytyped thank you!

[ARandomDev99]

#12060 (comment)
#12060 (comment)

@Jarcho

While modifying SpanlessEq to check differences in comments does make sense, a lot of existing lints will have their behavior change. For example, if_same_then_else will fail their existing UI tests and need their UI tests to be modified accordingly. Is this still a reasonable change? You suggested manually opting-in for the option to ignore differences in comments. Which lints would need to ignore such differences and which ones don't?

[Jarcho]

if_same_then_else should follow what match_same_arms does, same with any other lint detecting duplicate code.

I don't know off-hand which lints would benefit from ignoring comments. Generally they only match simple expressions which don't contain blocks anyways so it's not that big of a deal.