Fix needless_continue FP when match type is not unit or never

[profetia]

Closes #14550
Closes #15548

changelog: [needless_continue] fix FP when match type is not unit or never

[rustbot]

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[profetia]

Also closes #15548

[github-actions]

No changes for 13bd9b5

[samueltardieu]

The macro handling is wrong. Take the following code:

#![warn(clippy::needless_continue)]

macro_rules! mac {
    ($e:expr => $($rest:tt);*) => {
        loop {
            match $e {
                1 => continue,
                //~^ needless_continue
                2 => break,
                n => println!("{n}"),
            }
            $($rest;)*
        }
    };
}

fn main() {
    mac!(2 => );
    mac!(1 => {println!("foobar")});
}

With your patch, it gives:

warning: this `continue` expression is redundant
  --> /tmp/t.rs:7:22
   |
 7 |                 1 => continue,
   |                      ^^^^^^^^
...
18 |     mac!(2 => );
   |     ----------- in this macro invocation
   |
   = help: consider dropping the `continue` expression

However, if you execute the program as-is, it loops without printing anything, while if we drop the continue as suggested, it prints "foobar" in loop.

Linting the content of macros is dangerous as it may depend on how the macro is called and expanded.

[profetia]

Fixed. Thank you!

[samueltardieu]

By looking at the lintcheck results, it looks like you should also ensure that loop and continue come from the same context: https://docs.rs/encoding_rs/0.8.34/src/encoding_rs/iso_2022_jp.rs.html#161

Otherwise, you risk having the same problem with another invocation of the same macro, similar to the case I have exposed when the continue was in the macro.

[llogiq]

At the very least we should have a few more macro test cases. I find dealing with macros sufficiently head-warping that I want to have some confidence we're not breaking user code containing them.

[llogiq]

Also is the PR title correct in that we fix a false positive? Lintcheck shows 44 new warnings, which seems to imply that the PR is increasing the lint surface, not reducing it.

[profetia]

Those are FPs introduced when migrating the lint to late pass, and they are now fixed. Thank you! @samueltardieu @llogiq

[profetia]

r? clippy

[samueltardieu]

Once you remove the useless ref, it looks good to go.

View changes since this review

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.