fix [std_instead_of_core]: false positive for multi-imports#17252
fix [std_instead_of_core]: false positive for multi-imports#17252bushrat011899 wants to merge 1 commit into
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
Lintcheck changes for 5e134b4
This comment will be updated if you push new changes |
db1918a to
5dc77a7
Compare
|
Reminder, once the PR becomes ready for a review, use |
|
r? Jarcho |
3885b82 to
2eda7d5
Compare
This comment has been minimized.
This comment has been minimized.
2eda7d5 to
98e1658
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
…li-obk Fix(ICE): Remove `unwrap` from `get_module_children` # Objective Fix an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however it seems to me that it would be appropriate to just return nothing when called on a non-module-like item (if it isn't module-like, it has zero children). ## Solution - Switched from an `Option::unwrap` to an `if let Some(...)` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
98e1658 to
130da4a
Compare
130da4a to
28d43c3
Compare
…etrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
…unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
…unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
…unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
Rollup merge of #158002 - bushrat011899:get_module_children_unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.
28d43c3 to
5e134b4
Compare
| && is_stable(cx, def_id, self.msrv) | ||
| && !path.span.in_external_macro(cx.sess().source_map()) | ||
| && !is_from_proc_macro(cx, &first_segment.ident) | ||
| && !matches!(def_kind, DefKind::Macro(_)) |
There was a problem hiding this comment.
| && !matches!(def_kind, DefKind::Macro(_)) |
This PR can be trivially modified/followed-up to resolve #17260. This is because emit_lints has been modified to explicitly handle multiple items being resolved from the same path by taking the intersection of availabilities.
| for &span in span.primary_spans() { | ||
| span_lint_and_help( | ||
| cx, | ||
| lint, | ||
| span, | ||
| format!("used import from `{used_mod}` instead of `{replace_with}`"), | ||
| None, | ||
| format!("consider importing the item from `{replace_with}`"), | ||
| ); | ||
| } |
There was a problem hiding this comment.
| for &span in span.primary_spans() { | |
| span_lint_and_help( | |
| cx, | |
| lint, | |
| span, | |
| format!("used import from `{used_mod}` instead of `{replace_with}`"), | |
| None, | |
| format!("consider importing the item from `{replace_with}`"), | |
| ); | |
| } | |
| span_lint_and_help( | |
| cx, | |
| lint, | |
| span, | |
| format!("used import from `{used_mod}` instead of `{replace_with}`"), | |
| None, | |
| format!("consider importing the item from `{replace_with}`"), | |
| ); |
This PR can be trivially modified/followed-up to resolve #17261, since the required multispans are already being computed.
Objective
std_instead_of_core]: false positives forcore::io/MSRV #16964.std_instead_of_corecreates an invalid fix on stacked imports #12468Solution
The current version of this lint can improperly trigger a suggestion for certain complex
usestatements. This PR attempts to resolve this by directly tracking each segment in a path to ensure no items are inadvertently marked asstd_instead_of_core/etc.This problem is exacerbated by #16964 if/when it is merged, as unstable items will be correctly excluded from the lint on their own, but included when a part of a multi-import statement.
Notes
std_instead_of_core]: false positives forcore::io/MSRV #16964, but has been split off for ease of review. Either PR can be merged first.Please write a short comment explaining your change (or "none" for internal only changes)
changelog: fix certain false positives for [
std_instead_of_core] for mulit-imports.