Repository navigation
Fix invalid suggestions and misleading notes for imports - #162688
chenyukang wants to merge 2 commits into
Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @petrochenkov. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@rustbot reroll |
This comment has been minimized.
This comment has been minimized.
rustbot
added
S-waiting-on-author
58482ba to
73601a2
Compare
|
This PR was rebased onto a different main 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. |
| let mut next_binding = Some((decl, false)); | ||
| let mut next_ident = ident; | ||
| while let Some(binding) = next_binding { | ||
| while let Some((binding, path_to_binding_accessible)) = next_binding { |
There was a problem hiding this comment.
What I have in mind is like, to reduce the nesting depth and help reading code:
while let Some((binding, path_to_binding_accessible)) = next_binding.take() {
...
if res != Res::Err
&& let DeclKind::Import { source_decl, import, .. } = binding.kind
&& !source_decl.span.is_dummy()
&& let Some(source) = match import.kind {
ImportKind::Single { source, .. } => Some(source),
ImportKind::Glob { .. }
| ImportKind::MacroUse { .. }
| ImportKind::MacroExport => Some(next_ident),
ImportKind::ExternCrate { .. } => None,
}
{
...
next_ident = source;
next_binding = Some((source_decl, path_is_accessible));
}| segments: import | ||
| .module_path | ||
| .iter() | ||
| .filter(|segment| segment.ident.name != kw::PathRoot) |
There was a problem hiding this comment.
| .filter(|segment| segment.ident.name != kw::PathRoot) | |
| .filter(|segment| { | |
| segment.ident.name != kw::PathRoot || segment.ident.span.at_least_rust_2018() | |
| }) |
| source_res, | ||
| &parent_scope, | ||
| ) { | ||
| path_is_accessible = true; |
There was a problem hiding this comment.
import_source_suggestion_path only checks the Res. So if the path points to the different source_decl in parent_scope, we shouldn't label this import with you could import this re-export, right?
Maybe we should also check that this path would point to the same source_decl.
| let resolves_to_source = | ||
| match self.cm().maybe_resolve_path(&segments, source_res.ns(), parent_scope, None) { | ||
| PathResult::NonModule(partial_res) => partial_res.full_res() == Some(source_res), | ||
| PathResult::Module(ModuleOrUniformRoot::Module(module)) => { |
There was a problem hiding this comment.
I think it is fine to include ModuleAndExternPrelude here
| PathResult::Module(ModuleOrUniformRoot::Module(module)) => { | |
| PathResult::Module( | |
| ModuleOrUniformRoot::Module(module) | |
| | ModuleOrUniformRoot::ModuleAndExternPrelude(module), | |
| ) => module.res() == Some(source_res), |
|
☔ The latest upstream changes (presumably #164804) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Fixes #149418
It's better to review by commits.
The first commit is adding test.
The second commit is trying to fix the original issue in #149418.
The last commit fix a seperated accessibility issue by the way.