Fix relative paths in private import suggestions by qaijuang · Pull Request #157524 · rust-lang/rust

qaijuang · 2026-06-06T07:29:27Z

The private import diagnostic was reusing import.module_path for the "import directly" help. That path is relative to the module where the import was written, so a relative import can be suggested from the wrong place.

For the issue repro, this suggested:

use super::public::hi;

from the crate root. The suggestion should be:

use public::hi;

For relative local imports, this PR rebuilds the help from the resolved module path and only emits it if the item and every module segment are accessible from the failing use site. Additionally, this covers the private-ancestor case from #157455 (comment), where suggesting a direct path would still be invalid. Finally, it maintains an external-alias case as a guardrail to ensure super::s::mem is not treated like a local module path.

rustbot · 2026-06-06T09:02:15Z

r? @ShoyuVanilla

rustbot has assigned @ShoyuVanilla.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 20 candidates

qaijuang · 2026-06-17T13:44:23Z

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

will do a rebase rq

qaijuang · 2026-07-10T07:16:29Z

It seems like @ShoyuVanilla is tight on schedule, safe to @rustbot reroll

ShoyuVanilla

Sorry, I've been distracted these days and this slipped my mind 😢

View changes since this review

ShoyuVanilla · 2026-07-10T08:00:48Z

+                            None
+                        };
+
+                        if let Some(module_path) = module_path {


Could this be simplified with module_path.map(..)?

ShoyuVanilla · 2026-07-10T08:00:53Z

+                            // Reuse the existing relative shortening policy. The fields above
+                            // other than `did` and `path` are not used by this helper.
+                            self.shorten_candidate_path(&mut suggestion, parent_scope.module);


This feels like a temporary workaround 😅 Could you extract the path shortening part from shorten_candidate_path into a new method?

ShoyuVanilla · 2026-07-10T08:10:09Z

Could you make this into a new test file with //@ run-rustfix? I think that would make the purpose of this test clearer.

Done. Moved the root super case to tests/ui/imports/private-import-suggestion-relative-rustfix.rs

I mean, I think leaving this test as-is before this PR and moving the added lines into a new one would be nice

ShoyuVanilla

Some minor nits:

View changes since this review

ShoyuVanilla · 2026-07-10T10:05:21Z

We have lots of tests that runs rustfix but don't name them with the postfix -rustfix 😄 Could you rename this file without it and make the name just explain what this test is for?

Makes sense 🙂‍↔️

ShoyuVanilla · 2026-07-10T10:08:48Z

I mean, I think leaving this test as-is before this PR and moving the added lines into a new one would be nice

qaijuang · 2026-07-10T11:07:02Z

I mean, I think leaving this test as-is before this PR and moving the added lines into a new one would be nice

I tried for both cases, but neither is compatible with rustfix. The private ancestor intentionally still errors after rustfix runs and external alias produces use core::mem;, which fails in the default 2015 test context with an “unresolved import core” error

ShoyuVanilla · 2026-07-13T01:49:43Z

I mean, I think leaving this test as-is before this PR and moving the added lines into a new one would be nice

I tried for both cases, but neither is compatible with rustfix. The private ancestor intentionally still errors after rustfix runs and external alias produces use core::mem;, which fails in the default 2015 test context with an “unresolved import core” error

Ah, right. I got confused. Sorry 😅 Those cases aren't the cases that can be fixed with the suggested imports

ShoyuVanilla

Thanks!
@bors r+ rollup

View changes since this review

rust-bors · 2026-07-13T01:50:50Z

📌 Commit 9a0a4bf has been approved by ShoyuVanilla

It is now in the queue for this repository.

qaijuang · 2026-07-13T08:37:17Z

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

This pull request was unapproved.

will rebase rq

rustbot · 2026-07-13T08:46:10Z

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.

qaijuang · 2026-07-13T08:46:52Z

@rustbot ready

ShoyuVanilla · 2026-07-13T08:53:29Z

@bors r+ rollup

rust-bors · 2026-07-13T08:53:32Z

📌 Commit db1ab0b has been approved by ShoyuVanilla

It is now in the queue for this repository.

…ve-path, r=ShoyuVanilla Fix relative paths in private import suggestions Resolves rust-lang#157455. The private import diagnostic was reusing `import.module_path` for the "import directly" help. That path is relative to the module where the import was written, so a relative import can be suggested from the wrong place. For the issue repro, this suggested: ```rust use super::public::hi; ``` from the crate root. The suggestion should be: ```rust use public::hi; ``` For relative local imports, this PR rebuilds the help from the resolved module path and only emits it if the item and every module segment are accessible from the failing use site. Additionally, this covers the private-ancestor case from rust-lang#157455 (comment), where suggesting a direct path would still be invalid. Finally, it maintains an external-alias case as a guardrail to ensure `super::s::mem` is not treated like a local module path.

Rollup of 11 pull requests Successful merges: - #159210 (`rust-analyzer` subtree update) - #158655 (Fix coroutine MIR saved local remapping) - #159205 (bootstrap: Replace `ShouldRun::crates` with `crate_or_deps_filtered`) - #159208 (Fix `attr_on_non_tail_expr` typo) - #157524 (Fix relative paths in private import suggestions) - #158325 (Document NonNull layout guarantees) - #158882 (enable `do_not_recommend` attr for method call errors in current solver) - #158982 (Pretty-print MIR user types too.) - #159069 (Add codegen test for constant returns after local use) - #159163 (rustc_target: Add acquire-release to implied features of v8) - #159201 (borrowck: Represent 'best blame constraint' as index into `Vec<OutlivesConstraint>`)

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 6, 2026

qaijuang marked this pull request as ready for review June 6, 2026 09:02

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 6, 2026

rustbot assigned ShoyuVanilla Jun 6, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 6, 2026

fee1-dead self-assigned this Jun 8, 2026

fee1-dead removed their assignment Jun 16, 2026