Generic in classes #3330
Generic in classes #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>
1 participant
This pull request introduces comprehensive improvements to the handling and resolution of generic types in Lua class documentation and type inference. The changes enhance the parser and type inference engine to correctly resolve and substitute generic parameters in class fields, inheritance, and method return types. Additionally, new tests are added to verify the correct behavior for various generic class scenarios.
Generic Type Resolution Improvements
script/parser/luadoc.luato process and resolve generic type parameters indoc.extends.nameanddoc.fieldelements, ensuring that generic types are correctly bound and substituted during parsing. [1] [2]resolveGenericFieldinscript/vm/compiler.luato substitute generic parameters with their concrete types when accessing fields of generic classes, and integrated this logic into field lookup and class inheritance resolution. [1] [2] [3]script/vm/compiler.luato substitute generic type parameters with their actual types when calling methods on generic class instances, ensuring accurate type inference for method returns.API and Internal Refactoring
cloneObjectfunction toscript/vm/generic.luafor creating new objects with resolved generic parameters, supporting the improved generic resolution logic.Testing Enhancements
test/type_inference/common.luato verify correct generic type substitution in class fields, inheritance, and method returns, as well as regression tests for previously reported issues with generics.Display and Usability Improvements