fix: avoid prelude paths when imports.preferPrelude is false#22098
Merged
ChayimFriedman2 merged 2 commits intoApr 26, 2026
Conversation
When prefer_prelude=false (the default), the import path algorithm now actively deprioritizes paths that pass through a module named `prelude`, rather than ignoring the setting entirely. Two changes in find_path.rs: - Rename Choice::prefer_due_to_prelude to has_prelude_segment and track it unconditionally (not gated on the config). Make try_select compare bidirectionally: prefer prelude paths when prefer_prelude=true, avoid them when false. - Add container_max_len helper that relaxes the search length limit by 1 when the current best is a prelude path and prefer_prelude=false, so non-prelude alternatives one segment longer are not silently cut off. Fixes the case where krate::prelude::Foo (3 segs) was always chosen over krate::module::sub::Foo (4 segs) regardless of the preferPrelude setting. Generated with AI assistance (Claude).
Contributor
|
Note that we require disclosure of AI tools, if you used them. See Use of AI tools in CONTRIBUTING.md. |
Contributor
Author
|
@ChayimFriedman2 Yep, used AI, already disclosed in the commit message. |
Contributor
|
The current code passing the best path so far's length as |
Contributor
Author
|
@ChayimFriedman2 doing in this PR only. |
Souradip121
added a commit
to Souradip121/rust-analyzer
that referenced
this pull request
Apr 26, 2026
Per review on PR rust-lang#22098: the original code passing the current best path's length as the container search ceiling was a flawed local optimization. The correct ceiling is the recursive budget the caller passed down, minus the one segment we'll push (the item name) on top. Replacing both call sites with `max_len - 1` removes the broken pruning (which also originally caused the prelude-avoidance bug) and makes the container_max_len helper unnecessary. The bidirectional prelude comparison in Choice::try_select still picks the better-ranked path among everything the budget allows. Generated with AI assistance (Claude).
This comment has been minimized.
This comment has been minimized.
Per review feedback: the previous code passing the current best path's length as the container search ceiling was a flawed local optimization. The correct ceiling is the recursive budget the caller passed down, minus the one segment we'll push (the item name) on top. Replacing both call sites with `max_len - 1` removes the broken pruning (which also originally caused the prelude-avoidance bug) and makes the container_max_len helper unnecessary. The bidirectional prelude comparison in Choice::try_select still picks the better-ranked path among everything the budget allows. Generated with AI assistance (Claude).
f38e78f to
ef72f16
Compare
Contributor
Author
|
@ChayimFriedman2 check once! |
ChayimFriedman2
approved these changes
Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
Choice::prefer_due_to_prelude→has_prelude_segment; track itunconditionally instead of gating on the config value
Choice::try_selectbidirectional: deprioritize prelude paths whenprefer_prelude: false, prefer them whentruecontainer_max_lenhelper that relaxes the path-search limit by 1 whenthe current best is a prelude path and
prefer_prelude: false, so a canonicalpath one segment longer is not silently cut off before it can be considered
Before / After
Notes
rust-analyzer.imports.preferPrelude: false(the default)now has correct semantics in both directions
find_in_preludeearly-return (Rust language prelude items already in scope)is intentionally untouched; this fix only affects ranked path selection
Used AI for faster development, but properly reviewed the code myself after writing it.