Skip to content

'unneeded return' fires when return is needed due to attribute #9361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
RalfJung opened this issue Aug 22, 2022 · 0 comments · Fixed by #9381
Closed

'unneeded return' fires when return is needed due to attribute #9361

RalfJung opened this issue Aug 22, 2022 · 0 comments · Fixed by #9381
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@RalfJung
Copy link
Member

Description

Consider code like this:

        #[allow(clippy::integer_arithmetic)] // cannot overflow
        return discriminant << data_size | data;

The return is needed here because attributes can only be applied to statements, not expressions. But nevertheless clippy says I should remove the return...

Version

rustc 1.65.0-nightly (4b695f7c4 2022-08-21)
binary: rustc
commit-hash: 4b695f7c4e1a02d160fe7e159abd0f87027c0fcf
commit-date: 2022-08-21
host: x86_64-unknown-linux-gnu
release: 1.65.0-nightly
LLVM version: 15.0.0

Additional Labels

No response
@Alexendoo Alexendoo added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Aug 22, 2022
@lukaslueg lukaslueg mentioned this issue Aug 26, 2022
Merged
bors added a commit that referenced this issue Aug 27, 2022
@bors
Auto merge of #9381 - lukaslueg:issue9361, r=dswij
d381d84 
Don't lint `needless_return` if `return` has attrs

Fixes #9361

The lint used to have a mechanic to allow `cfg`-attrs on naked `return`-statements. This was well-intentioned, yet we can have any kind of attribute, e.g. `allow`, `expect` or even custom `derive`. So the mechanic was simply removed. We now never lint on a naked `return`-statement that has attributes on it.

Turns out that the ui-test had a Catch22 in it: In `check_expect()` the `#[expect(clippy::needless_return)]` is an attribute on the `return` statement that can and will be rustfixed away without side effects. But any other attribute would also have been removed, which is what #9361 is about. The test proved the wrong thing. Removed the test, the body is tested elsewhere as well.
bors added a commit that referenced this issue Aug 27, 2022
@bors
Auto merge of #9381 - lukaslueg:issue9361, r=dswij
be8bd60 
Don't lint `needless_return` if `return` has attrs

Fixes #9361

The lint used to have a mechanic to allow `cfg`-attrs on naked `return`-statements. This was well-intentioned, yet we can have any kind of attribute, e.g. `allow`, `expect` or even custom `derive`. So the mechanic was simply removed. We now never lint on a naked `return`-statement that has attributes on it.

Turns out that the ui-test had a Catch22 in it: In `check_expect()` the `#[expect(clippy::needless_return)]` is an attribute on the `return` statement that can and will be rustfixed away without side effects. But any other attribute would also have been removed, which is what #9361 is about. The test proved the wrong thing. Removed the test, the body is tested elsewhere as well.

changelog: Ignore [`needless_return`] on `return`s with attrs
@bors bors closed this as completed in fe93b8d Aug 27, 2022
This was referenced Jun 30, 2024
Closed
Merged
bors added a commit that referenced this issue Jul 3, 2024
@bors
Auto merge of #13027 - xFrednet:12998-expect-returns, r=Jarcho
a4bdab3 
`needless_return`: Support `#[expect]` on the return statement

A fix for #9361 suppresses `clippy::needless_return` if there are any attributes on the `return` statement. This leads to some unexpected behavior, as described in #12998, where adding `#[expect(clippy::needless_return)]` suppresses the lint, but doesn't fulfill the expectation.

I now decided to manually fulfill any expectations, if they are before the attribute check.

---

Closes: #12998

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't lint needless_return if return has attrs lukaslueg/rust-clippy
2 participants
@RalfJung @Alexendoo