Skip to content

Emit a single error when importing a path with _ #142805

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

Merged
merged 2 commits into from
Jun 25, 2025

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jun 20, 2025

When encountering use _;, use _::*' or similar, do not emit two errors for that single mistake. This also side-steps the issue of resolve errors suggesting adding a crate named _ to Cargo.toml.

Fix #142662.

@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 20, 2025
@@ -711,6 +711,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

let mut diag = struct_span_code_err!(self.dcx(), span, E0432, "{msg}");

if errors.iter().all(|(_, err)| err.segment == Some(kw::Underscore)) {
Copy link
Member

Choose a reason for hiding this comment

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

This leads to inconsistent behavior in code like use {_, a}. I'd rather we filter out the errors mentioning _ like we're doing with that errors.retain call above.

Copy link
Member

Choose a reason for hiding this comment

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

And while we're at it, we probably should delay a bug in the:

if errors.is_empty() {
    return;
}

on 691.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this pr touches this test, could we also rename it with some more descriptive name and put a small comment with a link on an issue related to this instead?

@compiler-errors
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 22, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@estebank estebank force-pushed the underscore-import branch 2 times, most recently from 0967071 to 7493828 Compare June 23, 2025 21:25
_ => true,
// If we've encountered something like `use _;`, we've already emitted an error stating
// that `_` is not a valid identifier, so we silence the resolve error.
_ => err.segment != Some(kw::Underscore),
Copy link
Member

Choose a reason for hiding this comment

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

Could you split this out into two retain calls? This is on the error path after all, and I think it's easier to understand since these two match arms have nothing to do with each other.

Copy link
Member

Choose a reason for hiding this comment

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

ping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you meant two retain calls, not two branches in the same match, gotcha

@estebank estebank force-pushed the underscore-import branch from 7493828 to d944f9e Compare June 23, 2025 22:25
@compiler-errors
Copy link
Member

Please add a test for the case that I asked for:

This leads to inconsistent behavior in code like use {_, a}. I'd rather we filter out the errors mentioning _ like we're doing with that errors.retain call above.

When encountering `use _;`, `use _::*'` or similar, do not emit two errors for that single mistake. This also side-steps the issue of resolve errors suggesting adding a crate named `_` to `Cargo.toml`.
@estebank estebank force-pushed the underscore-import branch from d944f9e to d82fb1e Compare June 24, 2025 16:21
@estebank
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 24, 2025
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 24, 2025

📌 Commit d82fb1e has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 24, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 24, 2025
…iler-errors

Emit a single error when importing a path with `_`

When encountering `use _;`, `use _::*'` or similar, do not emit two errors for that single mistake. This also side-steps the issue of resolve errors suggesting adding a crate named `_` to `Cargo.toml`.

Fix rust-lang#142662.
bors added a commit that referenced this pull request Jun 24, 2025
Rollup of 9 pull requests

Successful merges:

 - #142645 (Also emit suggestions for usages in the `non_upper_case_globals` lint)
 - #142657 (mbe: Clean up code with non-optional `NonterminalKind`)
 - #142799 (rustc_session: Add a structure for keeping both explicit and default sysroots)
 - #142805 (Emit a single error when importing a path with `_`)
 - #142882 (Lazy init diagnostics-only local_names in borrowck)
 - #142883 (Add impl_trait_in_bindings tests from #61773)
 - #142943 (Don't include current rustc version string in feature removed help)
 - #142965 ([RTE-497] Ignore `c-link-to-rust-va-list-fn` test on SGX platform)
 - #142972 (Add a missing mailmap entry)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d7e10f0 into rust-lang:master Jun 25, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jun 25, 2025
rust-timer added a commit that referenced this pull request Jun 25, 2025
Rollup merge of #142805 - estebank:underscore-import, r=compiler-errors

Emit a single error when importing a path with `_`

When encountering `use _;`, `use _::*'` or similar, do not emit two errors for that single mistake. This also side-steps the issue of resolve errors suggesting adding a crate named `_` to `Cargo.toml`.

Fix #142662.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leading underscore in path makes rust suggest to add a crate named _ with edition >= 2018
6 participants