Skip to content
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.

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

Lint if args adapted to Unit and discarded #10150

Merged
merged 1 commit into from Nov 29, 2022

Conversation

som-snytt
Copy link
Contributor

@scala-jenkins scala-jenkins added this to the 2.13.10 milestone Sep 21, 2022
@lrytz lrytz modified the milestones: 2.13.10, 2.13.11 Sep 26, 2022
@SethTisue
Copy link
Member

Is there an existing flag this could or should be folded into? (As you may recall, I had the same reaction to -Wnonunit-statement and -Wvalue-discard and you convinced me it made sense for them to be separate. But now this goes to three.)

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Oct 26, 2022
@som-snytt
Copy link
Contributor Author

som-snytt commented Oct 26, 2022

-Wvalue-discard is not a lint because it is a commonly-leveraged language feature.

This lint is stronger than the adapted-args lint because merely adapted implies still consumed and not discarded.

This is a lint because throwing away method args is not a commonly-leveraged language feature. To quote the paulpism, "this is unlikely to be what you want."

I see from the test that -Wvalue-discard still correctly warns. I think that is fine. The clever messaging: "args will be discarded" and "value was discarded."

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, @SethTisue ?

@SethTisue SethTisue self-assigned this Nov 24, 2022
@SethTisue SethTisue merged commit bce7748 into scala:2.13.x Nov 29, 2022
@som-snytt som-snytt deleted the issue/12495-lint-discarded-args branch November 29, 2022 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
4 participants