Skip to content

Add let_underscore_must_use lint #4823

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
Merged
merged 2 commits into from
Dec 23, 2019
Merged

Add let_underscore_must_use lint #4823

merged 2 commits into from
Dec 23, 2019

Conversation

@basil-cow
Copy link
Contributor

@basil-cow basil-cow commented Nov 16, 2019 •
edited
Loading

changelog: closes #4812 , added a new let_underscore_must_use lint, moved is_must_use_ty to utils, added is_must_use_fn util function

@basil-cow
Copy link
Contributor Author

basil-cow commented Nov 16, 2019

ping @flip1995 , is this fail my mistake or not?

@llogiq
Copy link
Contributor

llogiq commented Nov 16, 2019

Toolstate says clippy master is currently broken, so it's probably not your mistake.

@RalfJung
Copy link
Member

RalfJung commented Nov 17, 2019

Yeah this is Manishearth/compiletest-rs#202

flip1995
flip1995 requested changes Nov 18, 2019
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall. Two small changes.

clippy_lints/src/let_underscore.rs Outdated
/// let _ = f().is_ok();
/// ```
pub LET_UNDERSCORE_MUST_USE,
pedantic,
Copy link
Member

@flip1995 flip1995 Nov 18, 2019

Choose a reason for hiding this comment

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

Suggested change
pedantic,
restriction,

This was requested as a restriction lint, and I agree that it should be a restriction lint.

tests/ui/let_underscore.rs
}

fn main() {
let _ = f();
Copy link
Member

@flip1995 flip1995 Nov 18, 2019

Choose a reason for hiding this comment

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

Please add a test for a function, that is not must_use and doesn't have a must_use type.

@bors
Copy link
Contributor

bors commented Nov 23, 2019

☔ The latest upstream changes (presumably #4839) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) A-lint Area: New lints labels Nov 25, 2019
@basil-cow basil-cow force-pushed the must_use_res branch 3 times, most recently from fe06d5b to afb3f62 Compare December 7, 2019 09:44
@flip1995
Copy link
Member

flip1995 commented Dec 12, 2019

Thanks

@bors r+

@bors
Copy link
Contributor

bors commented Dec 12, 2019

📌 Commit afb3f62 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Dec 12, 2019

⌛ Testing commit afb3f62 with merge b6ef9bc...

bors added a commit that referenced this pull request Dec 12, 2019
@bors
Auto merge of #4823 - Areredify:must_use_res, r=flip1995
b6ef9bc 
Add `let_underscore_must_use` lint

changelog: closes #4812 , added a new `let_underscore_must_use` lint, moved `is_must_use_ty` to utils, added `is_must_use_fn` util function
@bors
Copy link
Contributor

bors commented Dec 12, 2019

💔 Test failed - status-appveyor

@flip1995
Copy link
Member

flip1995 commented Dec 12, 2019

./util/dev update_lints required

@bors
Copy link
Contributor

bors commented Dec 22, 2019

☔ The latest upstream changes (presumably #4930) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995
Copy link
Member

flip1995 commented Dec 22, 2019

Sorry for my absence. Can you rebase this, so we can merge this?

@basil-cow
move is_must_use_ty to utils
8b4a3b7
@basil-cow
implemented let_underscore lint
a310cb2 
actually add files

update lints

change to pedantic
@basil-cow
Copy link
Contributor Author

basil-cow commented Dec 22, 2019

Done

@phansch
Copy link
Contributor

phansch commented Dec 23, 2019

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Dec 23, 2019

📌 Commit a310cb2 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Dec 23, 2019

⌛ Testing commit a310cb2 with merge b38b026...

bors added a commit that referenced this pull request Dec 23, 2019
@bors
Auto merge of #4823 - Areredify:must_use_res, r=flip1995
b38b026 
Add `let_underscore_must_use` lint

changelog: closes #4812 , added a new `let_underscore_must_use` lint, moved `is_must_use_ty` to utils, added `is_must_use_fn` util function
@bors
Copy link
Contributor

bors commented Dec 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing b38b026 to master...

@bors bors merged commit a310cb2 into rust-lang:master Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flip1995 flip1995 flip1995 approved these changes

Assignees

No one assigned

Labels

A-lint Area: New lints S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New lint: let _ binding of must use function

6 participants

@basil-cow @llogiq @RalfJung @bors @flip1995 @phansch