Skip to content

Add let_underscore_must_use lint#4823

Merged
bors merged 2 commits intorust-lang:masterfrom
basil-cow:must_use_res
Dec 23, 2019
Merged

Add let_underscore_must_use lint#4823
bors merged 2 commits intorust-lang:masterfrom
basil-cow:must_use_res

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