fix: comprehensive generic type resolution improvements #3330
Conversation
Fixes LuaLS#1929 - Classes can now extend generic classes with specific type parameters and fields are properly resolved. Changes: - Parser: Support doc.type.sign for class extends (e.g., Bar: Foo<integer>) - Parser: Include doc.field in bindGeneric() to convert generic type names - VM: Add resolveGenericField() to resolve generic parameters in field types - VM: Handle doc.type.sign extends in class field inheritance search - VM: Export vm.cloneObject() for generic type resolution Example that now works: ---@Class Foo<T> ---@field a T ---@Class Bar: Foo<integer> local x ---@type Bar local what = x.a -- Now infers as 'integer' instead of 'unknown'
…expansion - Fix LuaLS#1863: Method return types on generic classes now resolve correctly When calling methods like Box<string>:getValue(), the return type T is now properly resolved to the concrete type (string) from the class instance. - Fix LuaLS#1853: Self-referential generic classes no longer cause infinite expansion in hover display. The resolved extends clause is now hidden from type display while still being available for type checking. - Add test cases for both fixes
Summary of ChangesHello @ian-pascoe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a substantial upgrade to the type inference system's ability to manage generic types in Lua class definitions. It provides a more robust and accurate mechanism for resolving generic parameters in class structures, method calls, and inheritance chains. The changes aim to improve the reliability of type checking and enhance the user experience by preventing misleading type displays and infinite recursion in the UI. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements for handling generic types in classes, including parsing, type inference for fields, inheritance, and method returns. The changes are well-structured and include a comprehensive set of new tests to validate the functionality.
I've found a couple of issues in script/vm/compiler.lua. One is a minor documentation mismatch that could cause confusion. The other is a more significant issue in the logic for resolving generic return types, which is incomplete and doesn't handle composite types. I've provided detailed feedback and suggestions for both.
script/vm/compiler.lua
Outdated
| if next(genericMap) then | ||
| -- Check the return node for global type references that match generic params | ||
| local newReturnNode = vm.createNode() | ||
| local hasReplacement = false | ||
| for retNode in returnNode:eachObject() do | ||
| if retNode.type == 'global' and retNode.cate == 'type' then | ||
| local resolvedNode = genericMap[retNode.name] | ||
| if resolvedNode then | ||
| newReturnNode:merge(resolvedNode) | ||
| hasReplacement = true | ||
| else | ||
| newReturnNode:merge(retNode) | ||
| end | ||
| else | ||
| newReturnNode:merge(retNode) | ||
| end | ||
| end | ||
| if hasReplacement then | ||
| returnNode = newReturnNode | ||
| end | ||
| end |
There was a problem hiding this comment.
The current implementation for resolving generic return types is incomplete. It only handles direct replacement of a generic type parameter (e.g., T becomes string) but fails for composite types like T[] or OtherType<T>. This will lead to incorrect type inference for methods that return such types from a generic class instance.
To fix this, you should use vm.cloneObject on the parser.object representation of the return type (returnObject), which is available in this function's scope. This function is designed for deep cloning and substitution of generics and will correctly handle composite types.
The suggested implementation below replaces the current logic. It resolves class generics on the parser.object and then re-compiles it and re-applies function generic resolution.
if next(genericMap) and returnObject and returnObject.type ~= 'generic' then
local newReturnObject = vm.cloneObject(returnObject, genericMap)
if newReturnObject then
-- Re-compile the node with class generics resolved
returnNode = vm.compileNode(newReturnObject)
-- Re-resolve function generics, as they were on the original returnNode
for rnode in returnNode:eachObject() do
if rnode.type == 'generic' then
returnNode = rnode:resolve(guide.getUri(source), args)
break
end
end
end
end
script/vm/compiler.lua
Outdated
| ---@param uri uri | ||
| ---@param classGlobal vm.global | ||
| ---@param typeName string |
There was a problem hiding this comment.
The documentation for @param uri, @param classGlobal, and @param typeName does not match the function signature of bindReturnOfFunction. This seems to be either misplaced documentation or a copy-paste error. Please remove or correct it to avoid confusion for future maintenance.
Address PR review comment: The previous implementation only handled direct T → string substitution but failed for composite types like T[] or Wrapper<T>. Changes: - Use vm.cloneObject on the actual return annotation (doc.return) instead of iterating through compiled node objects - Extend vm.cloneObject to handle doc.type.name when name matches a generic param (converts to doc.generic.name with _resolved set) - Extend vm.cloneObject to handle doc.type.sign for nested generic types, but only when signs contain doc.type.name needing resolution (preserves function-level generics like Callback<<T>>) Added test cases for: - T[] resolving to string[] - Wrapper<T> resolving to Wrapper<string>
…patterns - Add support for @Generic T in @overload annotations (LuaLS#723) - Parse fun<T>(x: T): T syntax in @field and @type annotations (LuaLS#1170) - Fix methods with @Generic T and @param self T to resolve return type to receiver's concrete type (e.g., List<number>:identity() returns List<number> instead of unknown) (LuaLS#1000) - Add comprehensive test cases for all generic patterns
ian-pascoe
changed the title
- Fix double-clone bug in cloneObject for doc.type.function returns - Add containsGenericName() for early-exit generic detection - Add getClassGenericMap() to eliminate code duplication - Refactor resolveGenericField() to use new helpers
|
@sumneko Please review when you get a chance |
1 participant
Summary
This PR introduces comprehensive improvements to generic type handling in the Lua Language Server, fixing multiple long-standing issues with generic class resolution.
Addresses #1861 (Generics Super Issue)
Fixed Issues
class Bar: Foo<integer>)Box<string>:getValue()returnsstring)@overloadannotationsfun<T>syntax for inline generic function typesT<X, Y>#1000 - Methods with@generic Tand@param self Tresolve to receiver's concrete typeChanges
Parser (
script/parser/luadoc.lua)fun<T, V>(...)syntax for inline generic functions in@fieldand@typedoc.type.signfor class extends (e.g.,Bar: Foo<integer>)doc.fieldanddoc.overloadinbindGeneric()for generic name bindingVM Compiler (
script/vm/compiler.lua)resolveGenericField()to substitute generic parameters when accessing fieldsdoc.type.signextends in class field inheritance search@param self Tpattern to resolve return type to receiver's concrete typeT[],Wrapper<T>)Generic System (
script/vm/generic.lua)vm.cloneObject()for generic type resolutiondoc.type.name→doc.generic.nameconversion when name matches generic paramdoc.type.signfor nested generic typesDisplay
hideView = trueon resolved extends clauses to prevent recursive hover expansion