Fix using static resolution for generic methods with type inference#269
Open
theolivenbaum wants to merge 3 commits intomasterfrom
Open
Fix using static resolution for generic methods with type inference#269theolivenbaum wants to merge 3 commits intomasterfrom
using static resolution for generic methods with type inference#269theolivenbaum wants to merge 3 commits intomasterfrom
Conversation
… `using static` When a generic method is imported via `using static` and called with inferred type arguments, `SharpSixRewriter` previously failed to fully qualify the method call with the inferred types, leading to downstream resolution errors (`UnknownIdentifierResolveResult`). This change: 1. Modifies `SharpSixRewriter.VisitIdentifierName` to detect identifiers referring to static members imported via `using static`. 2. Resolves the symbol and checks if it's a generic method. 3. If it is a generic method with inferred arguments, it explicitly generates a `GenericNameSyntax` with the fully qualified name and the inferred type arguments. 4. Otherwise, it generates a fully qualified `IdentifierName`. This ensures that the downstream compiler receives a fully resolved and qualified method call, bypassing ambiguity or inference issues. Added regression tests in `Tests/H5.Compiler.IntegrationTests/Language/UsingStaticTests.cs`. Co-authored-by: theolivenbaum <8791811+theolivenbaum@users.noreply.github.com>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
…a `using static`.
The previous fix incorrectly generated `GenericName` nodes containing dot-qualified identifiers (e.g. `GenericName("Namespace.Class.Method")`), which caused downstream errors like `?` in the output or parser failures.
This change:
1. Updates `SharpSixRewriter` to generate valid `MemberAccessExpression` nodes for `using static` calls.
2. Uses `SyntaxHelper.GenerateTypeSyntax` to correctly generate the containing type syntax.
3. Constructs the final expression as `MemberAccess(TypeSyntax, GenericName)`.
This ensures valid AST structure and fixes the reported regression.
Verified with `UsingStaticTests.cs`.
Co-authored-by: theolivenbaum <8791811+theolivenbaum@users.noreply.github.com>
…a `using static`.
The previous fix incorrectly generated `GenericName` nodes containing dot-qualified identifiers (e.g. `GenericName("Namespace.Class.Method")`), which caused downstream errors like `?` in the output or parser failures because the entire string was treated as a single identifier.
This change:
1. Updates `SharpSixRewriter.VisitIdentifierName` to detect when a `using static` method is a generic method with inferred type arguments.
2. Instead of generating a monolithic `GenericName`, it now constructs a valid `MemberAccessExpression` combining:
- A `TypeSyntax` generated via `SyntaxHelper.GenerateTypeSyntax` for the containing class.
- A `GenericNameSyntax` for the method name and its inferred type arguments.
3. This results in a correct AST structure (e.g., `Class.Method<T>`) that the downstream NRefactory parser and JS emitter handle correctly.
Verified with `UsingStaticTests.cs` (regression) and `UsingStaticAdvancedTests.cs` (new scenario matching the user report).
Co-authored-by: theolivenbaum <8791811+theolivenbaum@users.noreply.github.com>
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
1 participant
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.
Fixes an issue where
using staticgeneric methods with inferred type arguments caused "UnknownIdentifierResolveResult" errors in the generated JavaScript. The rewriter now correctly expands these calls into fully qualified generic invocations.PR created automatically by Jules for task 686686676756840054 started by @theolivenbaum