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.

Sign up for GitHub

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

Jump to bottom

Fix false positive for typoed crate or module suggestion #90125

Closed
wants to merge 2 commits into from
Closed

Fix false positive for typoed crate or module suggestion #90125

wants to merge 2 commits into from

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Oct 21, 2021 •
edited
Loading

follow up of #89347

closes #89592
closes #76208

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 21, 2021
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 21, 2021 •
edited
Loading

I could fix this problem for modules but not for crates. I used resolve_ident_in_module, but it cannot solve this problem for crates. Could you give me some advice?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@TaKO8Ki TaKO8Ki force-pushed the fix-typoed-crate-or-module-suggestion branch from ca60a5a to dd8bb96 Compare October 21, 2021 17:23
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 28, 2021
@bors
Copy link
Contributor

bors commented Nov 5, 2021

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

@apiraino
Copy link
Contributor

apiraino commented Dec 2, 2021

@TaKO8Ki could you perhaps investigate the CI failing? thanks! :)

@rustbot label -S-waiting-on-review +S-waiting-on-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 Dec 2, 2021
estebank
estebank reviewed Dec 2, 2021
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

The changes look reasonable! Apologies for the delay in reviewing. There was a CI failure when I last looked at this PR that needed addressing, and a recent change introduced some merge conflicts. Could you try and rebase on top of master and make sure the tests all succeed?

compiler/rustc_resolve/src/diagnostics.rs Outdated Show resolved Hide resolved
@TaKO8Ki
fix false positive typoed crate or module suggestion
b5caa1e 
fix false positive typoed crate or module suggestion
@TaKO8Ki TaKO8Ki force-pushed the fix-typoed-crate-or-module-suggestion branch from dd8bb96 to b5caa1e Compare December 3, 2021 00:47
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Dec 3, 2021 •
edited
Loading

@estebank This PR solves a problem for modules but not for extern crates. I tried using resolve_ident_in_module for extern crates as well as modules, but it didn't go well. And I have no idea how to solve it. Could you give me some advice?

For example:
https://github.com/rust-lang/rust/pull/90125/files#r761906078

TaKO8Ki
TaKO8Ki commented Dec 3, 2021
View reviewed changes
src/test/ui/suggestions/crate-or-module-typo.stderr
Comment on lines +7 to +10
help: there is a crate or module with a similar name
|
LL | use my_core::mem;
| ~~~~~~~
Copy link
Member Author

@TaKO8Ki TaKO8Ki Dec 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For example, these lines should not be included in stderr because my_core is defined in a decl_macro a.

@TaKO8Ki TaKO8Ki requested a review from estebank December 3, 2021 12:50
@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2022
@bors
Copy link
Contributor

bors commented Feb 18, 2022

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

@Dylan-DPC
Copy link
Member

@TaKO8Ki any updates on this?

@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2022
@Dylan-DPC
Copy link
Member

Closing this due to inactivity

@Dylan-DPC Dylan-DPC closed this Jun 17, 2022
@Dylan-DPC Dylan-DPC added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2022
@estebank
Copy link
Contributor

@TaKO8Ki feel free to reopen or refile when you have time again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank Awaiting requested review from estebank

Assignees

@estebank estebank

Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
9 participants
@TaKO8Ki @rust-log-analyzer @bors @apiraino @Dylan-DPC @estebank @JohnCSimon @rust-highfive @rustbot