lua-language-server
lua-language-server copied to clipboard
Attempt to implement generic referencing
It is an attempt to implement generic references inside strings. e.g.
---@generic T
---@param f T
local function test(f)
end
---@class Test
f "Test" -- This is a reference to class 'Test'
I am not really interested in the feature, as a use case has yet to arise (though I could imagine it arising), it is mainly an attempt to understand the language server better. And especially because I am modifying the source data inside getRefs, I wouldn't recommend blind merging.
Any feedback would gladly be appretiated. I am trying my best to understand this awesome project :)
Thank you for your PR. I have read your implementation. The overall idea is OK, but there are still many boundary conditions that need to be handled to pass the unit test, such as: https://github.com/sumneko/lua-language-server/blob/2dc94335203311d87ecf501b34fa9b973b4b8cb0/test/references/common.lua#L62-L67
In addition, I suggest you implement unit tests first. For example, strings are expressed in many ways, but it seems that you only implement the expression of double quotation marks.
Thanks for your reply. So it is okay to mark generic Arguments as genArg = true? It felt like a quick hack, but if the ast is generated over and over, maybe it doesn't pose a problem.
https://github.com/sumneko/lua-language-server/blob/37cf09ad81a337e4378e39b61a8c52d65e45cfa7/script/vm/ref.lua#L169-L176
Thanks for your reply. So it is okay to mark generic Arguments as
genArg = true? It felt like a quick hack, but if theastis generated over and over, maybe it doesn't pose a problem.
There is no problem caching data on the ast itself, because whether it is genArg or not is determined by the current file itself. The modification of other files cannot be affected, and the ast will be recompiled when the current file is modified.
However, after careful consideration, it is not necessary to add the genArg attribute.
- Implement the definition witch supports finding
---@class TESTbyf "TEST". This feature should not need to modify the ast. - When searching reference of
---@class TEST, scan all files and useguide.eachSourceType(root, 'string')to find out all strings"TEST", then just usevm.getDefsfor them to check if they are defined by---@class TEST. You should callsearchInAllFilesfor it. - Then
vm.getRefscan found out correct results, so you don't need to check it again inrename
EDIT: I'm not right. The function caller and the function definer can be in different files. The genArg is defined at the call, but the semantics are determined by the definer. Therefore, you cannot modify the ast at the caller.
You can use vm.compileNode(source) - > node, and then add attributes on node. The node will be reset when any state changes.
Hello again, now I've got some time on my hands to look into this subject again. Some questions occured to me: About the syntax in the tests: Do I understand correctly, that:
TEST [[
t[<~'a'~>] = 1
print(t.<!a!>)
]]
means, if I want to get the reference of t['a'] I expect to find it at print(t.a)?
And what does the call to findSource mean (in core.reference)?
I assume it trys to find out, what the example is, by which to find all references?
Edit:
What does the defMap parameter mean in the switch inside vm.ref?
means, if I want to get the reference of
t['a']I expect to find it atprint(t.a)?
<?xxx?> is request positions and <!xxx!> is response positions.
<~~> is equal to <!<?xxx?>!>.
And what does the call to
findSourcemean (incore.reference)? I assume it trys to find out, what the example is, by which to find all references?
findSource is to convert position to parser.object ({line = 3, charater = 10} -> {type = 'getfield', ...})
What does the
defMapparameter mean in the switch insidevm.ref?
- find refercens of
thisCat:getName() - find definitions of
thisCat:getName()->animal:getName() - save
animal:getName()intodefMap - search all
xxx.getNamein the workspace. - test if the definitions of each
xxx.getNamematchs thedefMap, if yes, add them into reference results.
So, I think I got it working, but I haven't tested it thoroughly (see unit tests). I wanted to play arround with it, but I forgot how to compile and launch the server in a seperate VSCode Window.
Changes:
I added array searching functions in utility.
I added vm.isArgumentGenericLiteral() in vm.generic.
I modified defs, definition, refs and reference to find references and definitions, as you layed it out for me:
1. Implement the definition witch supports finding `---@class TEST` by `f "TEST"`. This feature should not need to modify the ast. 2. When searching reference of `---@class TEST`, scan all files and use `guide.eachSourceType(root, 'string')` to find out all strings `"TEST"`, then just use `vm.getDefs` for them to check if they are defined by `---@class TEST`. You should call `searchInAllFiles` for it. 3. Then `vm.getRefs` can found out correct results, so you don't need to check it again in `rename`
Though I think I do need to check if the given reference is a generic literal argument in rename (not with genArg stored inside the node though, but with vm.isArgumentGenericLiteral()), because in a debug session, when figuring out what to do, there were other references inside another test case, unrelated to what I did.
I copied the searchInAllFiles into defs, I don't think this is a good design, because I would rather just put it somewhere else to import, but I didn't do that, because I don't even know if it is necessary or if there is another solution.
Any feedback would be appretiated.
I have not had time recently, and it will take a few days to review.
Please don't feel pressured by me, I appretiate that this repo is activly maintained by someone =)
I am closing this pull request because I no longer require the proposed feature at this time. I'll leave the linked discussion here for reference, in case I or someone else comes back to it in the future.