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

Add lint manual_inspect #12287

Merged
merged 2 commits into from
Jun 16, 2024
Merged

Add lint manual_inspect #12287

merged 2 commits into from
Jun 16, 2024

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Feb 12, 2024

fixes #12250

A great example of a lint that sounds super simple, but has a pile of edge cases.


changelog: Add lint manual_inspect

@rustbot
Copy link
Collaborator

rustbot commented Feb 12, 2024

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 12, 2024
@Jarcho Jarcho force-pushed the issue_12250 branch 5 times, most recently from 65439cf to 509614b Compare February 13, 2024 23:22
@bors
Copy link
Contributor

bors commented Feb 19, 2024

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


use super::MANUAL_INSPECT;

#[expect(clippy::too_many_lines)]
Copy link
Contributor

Choose a reason for hiding this comment

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

might make sense to factor out some methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are distinct parts which it could be split at, but that would just end up with functions taking 5+ arguments and multiple return values. Not really an improvement.

Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

This looks sensible. I have but one nit, but if you feel it's too much work to refactor that method, I'm ok with just merging it after a rebase.

@llogiq
Copy link
Contributor

llogiq commented Feb 29, 2024

@rustbot author

@rustbot rustbot 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 Feb 29, 2024
@mpalmer
Copy link

mpalmer commented Apr 28, 2024

Hey @Jarcho, are you up for getting this rebased / refactored for merge? If not, lmk and I'll have a crack at it. I'd like to see this lint land, as I'm sure I've been guilty of this now and then in the past.

@llogiq
Copy link
Contributor

llogiq commented Jun 15, 2024

Thanks for seeing this through.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 15, 2024

📌 Commit 404a6de has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 15, 2024

⌛ Testing commit 404a6de with merge debcef0...

bors added a commit that referenced this pull request Jun 15, 2024
Add lint `manual_inspect`

fixes #12250

A great example of a lint that sounds super simple, but has a pile of edge cases.

changelog: Add lint `manual_inspect`
@bors
Copy link
Contributor

bors commented Jun 15, 2024

💔 Test failed - checks-action_dev_test

@llogiq
Copy link
Contributor

llogiq commented Jun 16, 2024

@bors retry

bors added a commit that referenced this pull request Jun 16, 2024
Add lint `manual_inspect`

fixes #12250

A great example of a lint that sounds super simple, but has a pile of edge cases.

----

changelog: Add lint `manual_inspect`
@bors
Copy link
Contributor

bors commented Jun 16, 2024

⌛ Testing commit 404a6de with merge aae7ddb...

@bors
Copy link
Contributor

bors commented Jun 16, 2024

💔 Test failed - checks-action_dev_test

* Delay the parsing of the use node
* Mark when the `SyntaxContext` changes rather than return `None`
* Return a default value if the HIR tree is broken rather than `None`
@llogiq
Copy link
Contributor

llogiq commented Jun 16, 2024

Ok, let's try once more.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2024

📌 Commit 22710f3 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 16, 2024

⌛ Testing commit 22710f3 with merge 16efa56...

@bors
Copy link
Contributor

bors commented Jun 16, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 16efa56 to master...

@bors bors merged commit 16efa56 into rust-lang:master Jun 16, 2024
11 checks passed
facebook-github-bot pushed a commit to facebook/buck2 that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookexperimental/rust-shed that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/buck2-change-detector that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebook/starlark-rust that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/reindeer that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookexperimental/allocative that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/gazebo that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest inspect_err instead of map_err with a closure that returns the original error
5 participants