Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: scope parsing for table/turple declaration #3041

Closed
TIMONz1535 opened this issue Jan 13, 2025 · 2 comments · Fixed by #3055
Closed

Feature request: scope parsing for table/turple declaration #3041

TIMONz1535 opened this issue Jan 13, 2025 · 2 comments · Fixed by #3055

Comments

@TIMONz1535
Copy link
Contributor

TIMONz1535 commented Jan 13, 2025

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

Windows

What is the issue affecting?

Annotations

Expected Behaviour

I want to create a generic class with a private field, but this is not supported.

---@class Array<T>: { [integer]: T }
---@field private length integer

length field refers to an Array class, not an Array<T>, well the signs (generic class) are not good at this moment.

But I can do table/turple declaration

---@class Array<T>: { [integer]: T, length: integer }

unfortunately I can't specify the scope result.visible (private, protected, public, package).

Actual Behaviour

It's easy to add this for parsing in luadoc, what about this syntax?

---@class Array<T>: { [integer]: T, (private) length: integer }  -- OK
---@class Array<T>: { [integer]: T, private length: integer } -- OK
---@class Array<T>: { [integer]: T, (private length: integer) } -- OK
---@class Array<T>: { [integer]: T, ((private) length: integer) } -- wrong?

or

---@class Array<T>: { [integer]: T, (private) length: integer }  -- OK
---@class Array<T>: { [integer]: T, private length: integer } -- wrong
---@class Array<T>: { [integer]: T, (private length: integer) } -- wrong
---@class Array<T>: { [integer]: T, ((private) length: integer) } -- OK

References:

parseTable

if checkToken('symbol', '(', 1) then
nextToken()
needCloseParen = true
end
field.name = parseName('doc.field.name', field)
or parseIndexField(field)

result.visible

try(function ()
local tp, value = nextToken()
if tp == 'name' then
if value == 'public'
or value == 'protected'
or value == 'private'
or value == 'package' then
local tp2 = peekToken(1)
local tp3 = peekToken(2)
if tp2 == 'name' and not tp3 then
return false
end
result.visible = value
result.start = getStart()
return true
end
end
return false
end)

I want to hear someone's opinion.

@tomlau10
Copy link
Contributor

tomlau10 commented Jan 14, 2025

The bug related to generic class completion

length field refers to an Array class, not an Array, well the signs (generic class) are not good at this moment.

There is indeed some bugs related to the generic class completion.
Even if your write without the private, you can't get field completion when you just have an Array<string> variable

---@class Array<T>: { [integer]: T }
---@field length integer

---@type Array<string>
local t

t.<trigger completion>
--> it doesn't suggest `length` here

So this is actually a bug related to generic class completion, and it is reported in this issue: #2945

My previous debugging journey

I have debugged it before, and found a fix in this comment: #2945 (comment)
Basically is to add a : case 'doc.type.sign' inside the searchFieldSwitch in vm/compiler.lua. 🤔
But unfortunately, until now no one have time to test it and PR it 🙈

I just tested my patch, seems it can solve your issue as well 👀
Maybe you can try my patch and add more tests to test it thoroughly, then PR it?

From c2c96a15548ba1bdea86f426c377735e209fd675 Mon Sep 17 00:00:00 2001
From: Tom Lau <[email protected]>
Date: Sun, 10 Nov 2024 10:36:34 +0800
Subject: [PATCH] wip

---
 script/vm/compiler.lua | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/script/vm/compiler.lua b/script/vm/compiler.lua
index 92da538e..16a38a58 100644
--- a/script/vm/compiler.lua
+++ b/script/vm/compiler.lua
@@ -304,6 +304,17 @@ local searchFieldSwitch = util.switch()
             end
         end
     end)
+    : case 'doc.type.sign'
+    : call(function (suri, source, key, pushResult)
+        if not source.node[1] then
+            return
+        end
+        local global = vm.getGlobal('type', source.node[1])
+        if not global then
+            return
+        end
+        vm.getClassFields(suri, global, key, pushResult)
+    end)
     : case 'global'
     : call(function (suri, node, key, pushResult)
         if node.cate == 'variable' then
@@ -748,6 +759,7 @@ function vm.getNodesOfParentNode(source, key)
             break
         end
     end
+    local skipDocTypeFieldRes = false
     for node in parentNode:eachObject() do
         if not hasClass
         or (
@@ -761,6 +773,14 @@ function vm.getNodesOfParentNode(source, key)
                 if mark[res] then
                     return
                 end
+                if res.type == 'doc.type.field' and skipDocTypeFieldRes then
+                    mark[res] = true
+                    return
+                end
+                if node.type == 'global' or node.type == 'doc.type.sign' then
+                    -- found definition from class, no need to search extended doc table fields
+                    skipDocTypeFieldRes = true
+                end
                 mark[res] = true
                 if markDoc then
                     docedResults[#docedResults+1] = res
-- 
2.32.0.windows.1
  • the upper change is the major fix, which lets generic class to search the type definition
  • the lower fix is optional, because I found that when triggering completion in t.| case
    => it will suggest integer (which I believe is extracted from the { [integer]: T } part)
    => this part skipped it
    ‼️ edit: seems the optional fix part has some problem... it will not search testing field in the case : { [integer]: T, testing: string } ...
    maybe just don't include the optional fix part 🫠

demo of your use case with my patch

  • can suggest the private length field within object method ✅
    image
  • will not suggest the private field when triggering from instance of the class ✅
    image

Just some more comment related to the suggested syntax.
I'm not sure if introducing scope in table literal is a good idea 😕
Because it is specific to the @field, which in turns specific to @class annotation only?
A normal table doesn't have scoped field concept, and the related invisible diagnostic check are based on class I believe?

@CppCXY
Copy link
Collaborator

CppCXY commented Jan 14, 2025

Actually, types can be written across multiple lines:

---@class Array<T>: {
--- [integer]: T,
--- length: integer,
---}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants