Add [`err_expect`] lint #8606

b-ncMN · 2022-03-29T17:58:51Z

[expect_err] lint

Followed [lint naming conventions][lint_naming]
Added passing UI tests (including committed .stderr file)
cargo test passes locally
Executed cargo dev update_lints
Added lint documentation
Run cargo dev fmt

changelog: Added a lint to detect usage of .err().expect()

rust-highfive · 2022-03-29T17:58:55Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @camsteffen (or someone else) soon.

Please see the contribution instructions for more information.

xFrednet · 2022-03-29T18:03:39Z

Very awesome, I'll assign this to myself. Right now I'm a bit tired, but I'll try to review it later today or tomorrow 🙃

r? @xFrednet

xFrednet

This is already a great start 👍. I've added some notes for improvements and checks that need to be added. Thank you for the work, you already put into this. You're welcome to reach out if something is unclear 🙃

clippy_lints/src/methods/mod.rs

tests/ui/expect_err.rs

clippy_lints/src/methods/expect_err.rs

xFrednet · 2022-03-30T20:36:13Z

I put some more thought into the name. I think the current one expect_err doesn't work with lint attributes. #[deny(clippy::expect_err)] sounds like it denies the usage of expect_err instead of the other way around. Maybe err_then_expect, err_expect or manual_expect_err would be better names for the lint. 🤔 err_expect might be the best name as it is similar to ok_expect 🙃

xFrednet

Nice improvements 👍 I've attempted to answer all your questions, if I missed something, please say so :)

clippy_lints/src/methods/expect_err.rs

clippy_lints/src/methods/mod.rs

tests/ui/expect_err.rs

clippy_lints/src/methods/expect_err.rs

tests/ui/err_expect.rs

clippy_lints/src/methods/mod.rs

xFrednet · 2022-04-01T07:36:30Z

I'm not sure why this issue occurs https://github.com/rust-lang/rust-clippy/runs/5779347126?check_suite_focus=true

You've added something to tests/ui/min_rust_version_attr.rs which changes the line numbers for lint reports inside the related .stderr file. A simple cargo test and then cargo dev bless should update the .stderr file and fix the CI 🙃

b-ncMN · 2022-04-01T09:23:12Z

I think I went through all of your reviews, tests are passing, is this ready for merging?
@xFrednet

xFrednet

Some minor comments and then it should be ready for merge. If you can, could you maybe also squash the intermediate commits that only added formatting or fixed something?

clippy_lints/src/methods/err_expect.rs

clippy_lints/src/methods/mod.rs

b-ncMN · 2022-04-02T19:46:26Z

How are we looking now ? 😄 @xFrednet

b-ncMN · 2022-04-02T20:13:49Z

I may have done an oopsie while squashing, oh well.. I'll just squash it all into one commit

b-ncMN · 2022-04-03T22:36:12Z

@xFrednet

xFrednet

This version looks good to me, I have two more comments, that should be fixed quickly, and then we can merge this 🙃

I may have done an oopsie while squashing, oh well.. I'll just squash it all into one commit

It doesn't hurt to have everything in just one commit. Thank you for squashing. You can add the requested changes as a new commit or squash them if you like 🙃

clippy_lints/src/methods/mod.rs

clippy_lints/src/methods/err_expect.rs

- err() was meant to be employed instead of ok() - wraps comment

b-ncMN · 2022-04-06T17:48:23Z

This version looks good to me, I have two more comments, that should be fixed quickly, and then we can merge this upside_down_face

I may have done an oopsie while squashing, oh well.. I'll just squash it all into one commit

It doesn't hurt to have everything in just one commit. Thank you for squashing. You can add the requested changes as a new commit or squash them if you like upside_down_face

I have applied the changes but didn't squash this time, let me know if you need the commit description changed :)

xFrednet · 2022-04-06T18:03:08Z

Everything looks good to me, thank you for the work you put into this! I hope you also had fun 🙃

@bors r+

bors · 2022-04-06T18:03:10Z

📌 Commit a7125b0 has been approved by xFrednet

bors · 2022-04-06T18:03:17Z

⌛ Testing commit a7125b0 with merge 294f772...

Add [`err_expect`] lint [`expect_err`] lint - \[ ] Followed [lint naming conventions][lint_naming] - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - \[x] Added lint documentation - \[x] Run `cargo dev fmt` Fixes #1435 changelog: Added a lint to detect usage of .err().expect()

bors · 2022-04-06T18:03:54Z

💔 Test failed - checks-action_test

b-ncMN · 2022-04-06T18:05:38Z

Oops, I edited the body of the PR, I think bors didn't like having a newline in there

xFrednet · 2022-04-06T18:07:52Z

Correct, nice quick fix. Thank you!!

@bors retry (changelog)

bors · 2022-04-06T18:07:59Z

⌛ Testing commit a7125b0 with merge 409a936...

bors · 2022-04-06T18:21:57Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 409a936 to master...

rust-highfive assigned camsteffen Mar 29, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 29, 2022

rust-highfive assigned xFrednet and unassigned camsteffen Mar 29, 2022

b-ncMN force-pushed the err()-expect()-lint branch 4 times, most recently from a1662f3 to bcc0c9a Compare March 29, 2022 19:25

xFrednet reviewed Mar 30, 2022

View reviewed changes

xFrednet added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 30, 2022

b-ncMN changed the title ~~Add [expect_err] lint~~ Add [err_expect] lint Mar 31, 2022

xFrednet reviewed Mar 31, 2022

View reviewed changes

b-ncMN force-pushed the err()-expect()-lint branch from a11fc56 to de6b441 Compare March 31, 2022 18:46

This comment was marked as resolved.

Sign in to view

b-ncMN force-pushed the err()-expect()-lint branch 2 times, most recently from 7eee09b to 1294f26 Compare March 31, 2022 21:10

xFrednet reviewed Mar 31, 2022

View reviewed changes

tests/ui/err_expect.rs Show resolved Hide resolved

xFrednet reviewed Mar 31, 2022

View reviewed changes

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved

b-ncMN force-pushed the err()-expect()-lint branch 2 times, most recently from 6953ebd to 13461c3 Compare March 31, 2022 21:22

This comment was marked as resolved.

Sign in to view

b-ncMN force-pushed the err()-expect()-lint branch from 42aafd5 to 640824b Compare March 31, 2022 22:39

b-ncMN force-pushed the err()-expect()-lint branch from 640824b to 8fa18fe Compare April 1, 2022 09:10

xFrednet reviewed Apr 2, 2022

View reviewed changes

clippy_lints/src/methods/err_expect.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/err_expect.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved

b-ncMN force-pushed the err()-expect()-lint branch 5 times, most recently from fd0cefe to 8262085 Compare April 2, 2022 20:13

b-ncMN force-pushed the err()-expect()-lint branch from 8262085 to ba9a648 Compare April 2, 2022 20:14

b-ncMN force-pushed the err()-expect()-lint branch from ba9a648 to 5c94567 Compare April 6, 2022 12:43

xFrednet reviewed Apr 6, 2022

View reviewed changes

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/err_expect.rs Outdated Show resolved Hide resolved

b-ncMN added 2 commits April 6, 2022 19:25

Add .err().expect() lint

cebe575

Fix mistakes in documentation :

a7125b0

- err() was meant to be employed instead of ok() - wraps comment

b-ncMN force-pushed the err()-expect()-lint branch from 9c2e632 to a7125b0 Compare April 6, 2022 17:26

xFrednet approved these changes Apr 6, 2022

View reviewed changes

bors merged commit 409a936 into rust-lang:master Apr 6, 2022

bors mentioned this pull request Apr 6, 2022

Add lint to tell about let else pattern #8437

Merged

b-ncMN deleted the err()-expect()-lint branch April 6, 2022 18:24

This was referenced Apr 6, 2022

New lint is_digit_ascii_radix #8624

Merged

Fix as_deref_mut false positives in needless_option_as_deref #8646

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add [`err_expect`] lint #8606

Add [`err_expect`] lint #8606

b-ncMN commented Mar 29, 2022 •

edited

Loading

rust-highfive commented Mar 29, 2022

xFrednet commented Mar 29, 2022

xFrednet left a comment

xFrednet commented Mar 30, 2022

xFrednet left a comment

This comment was marked as resolved.

This comment was marked as resolved.

xFrednet commented Apr 1, 2022

b-ncMN commented Apr 1, 2022 •

edited

Loading

xFrednet left a comment

b-ncMN commented Apr 2, 2022

b-ncMN commented Apr 2, 2022

b-ncMN commented Apr 3, 2022

xFrednet left a comment

b-ncMN commented Apr 6, 2022

xFrednet commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

b-ncMN commented Apr 6, 2022

xFrednet commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

Add [err_expect] lint #8606

Add [err_expect] lint #8606

Conversation

b-ncMN commented Mar 29, 2022 • edited Loading

rust-highfive commented Mar 29, 2022

xFrednet commented Mar 29, 2022

xFrednet left a comment

Choose a reason for hiding this comment

xFrednet commented Mar 30, 2022

xFrednet left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

xFrednet commented Apr 1, 2022

b-ncMN commented Apr 1, 2022 • edited Loading

xFrednet left a comment

Choose a reason for hiding this comment

b-ncMN commented Apr 2, 2022

b-ncMN commented Apr 2, 2022

b-ncMN commented Apr 3, 2022

xFrednet left a comment

Choose a reason for hiding this comment

b-ncMN commented Apr 6, 2022

xFrednet commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

b-ncMN commented Apr 6, 2022

xFrednet commented Apr 6, 2022

bors commented Apr 6, 2022

bors commented Apr 6, 2022

Add [`err_expect`] lint #8606

Add [`err_expect`] lint #8606

b-ncMN commented Mar 29, 2022 •

edited

Loading

b-ncMN commented Apr 1, 2022 •

edited

Loading