refactor: single source of truth for impl-qualified function names#627
Merged
rocketman-code merged 2 commits intomainfrom Apr 24, 2026
Merged
refactor: single source of truth for impl-qualified function names#627rocketman-code merged 2 commits intomainfrom
rocketman-code merged 2 commits intomainfrom
Conversation
Walks up the AST from a function node to find the nearest enclosing impl or trait block. Handles: standalone fn (bare name), inherent impl (Type::method), trait impl (<Type as Trait>::method), trait default (Trait::method), nested fn in impl (bare, breaks at enclosing fn), and nested impl in fn (inner impl type). Includes 6 unit tests and a cross-path agreement test that feeds the same source to both FnCollector (resolve) and qualified_name_for_fn (rewriter path), asserting identical names. Covers generics. Updates stale InjectionCollector doc references.
Rewriter: delete qualified_fn_name, call naming::qualified_name_for_fn at both guard-injection sites. Update test keys for trait-qualified format (T::method). Resolve: remove current_impl and current_trait state from FnCollector. Simplify visit_impl, visit_impl_fn, visit_trait, visit_trait_fn, walk_fn_body, and visit_nested_fn to use the shared function. Delete find_ancestor_impl_context and macro_call_impl_context. The macro path inlines a parent-walk with fn-break guard since expanded AST lacks original parent context. Fixes: find_ancestor_impl_context walked past enclosing functions, causing nested bare fns inside impl methods to inherit the impl prefix.
ed59433 to
006b385
Compare
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.
Summary
naming::qualified_name_for_fnthat derives impl/trait-qualified names from AST positioncurrent_implandcurrent_traitstate variables from FnCollectorfind_ancestor_impl_contextandmacro_call_impl_contextNet -53 lines of code. Resolve lost 97 lines of state management, rewriter lost 29 lines of duplicate logic.
Fixes a latent bug:
find_ancestor_impl_contextwalked past enclosing functions, causing nested bare fns inside impl methods to inherit the impl prefix (Outer::helperinstead ofhelper).Test Plan
qualified_name_for_fncovering all edge casesT::method)