I think there is a big problem here: 😕

• if the type of the object is any / unknown / nil / or some compound type
• the resulting type will be very weird

just using your example above

---@type Vehicle|nil
local v1 = {}
local r1 = getItemColor(v1) --> r1: VehicleColor|nilColor

PS: I am not maintainer of LuaLS, just one of the contributors.
So I cannot judge whether this PR at this state will be accepted or not.

No problem, I will review that case and update the PR

Hello @tomlau10 . I looked at this after your feeback.

I would leave the PR as-is. This indeed can end up creating weird types, but when you are using string you can also do weird stuff, namely, you can call getItemColor("nil") and you'll get nilColor. I think it is no big deal.

I have another variant of the code that works as follows:

---@type Vehicle|nil
local v1 = {}
local r1 = getItemColor(v1) --> r1: VehicleColor|nil

Meaning, nil, unknown, any, are propagated without string transformation.

Let me know which one would work best. I am fine either way. If not, what would you expect in such cases?

My use case is, I am writing Lua bindings for a major ECS, we need a syntax as follows:

---@class transform.Position : Component
---@class transform.PositionInstance
---@field x number
---@field y number
-- Retrieves the provided component value or nil if the component is not found
---@generic T
---@param component `T`Instance # component to retrieve
---@return T | nil
function entity:get(component) end
local pos = entity:get(transform.Position) --> pos: transform.PositionInstance|nil
if pos then
 print("Position: ", pos.x, pos.y)
else
 print("Entity does not have Position")
end

Please let me know your thoughts.

but when you are using string you can also do weird stuff, namely, you can call getItemColor("nil") and you'll get nilColor. I think it is no big deal.

When using literal string, I don't think anyone would write getItemColor("nil") intentionally.
But when passing as a variable, I guess the chance of passing nil/any/unknown is much higher, either intentionally or accidentally. 🤔

Moreover, the current implementation would give inconsistent result in some of the following cases:
(still using your example above)

-- 1. just a variable with no annotation
local v1 --> v1: unknown
local r1 = getItemColor(v1) --> unknown, expected, but out of my expectation that it works correctly
-- 2. a table value
local v2 = {} --> v2: table
local r2 = getItemColor(v2) --> unknown, expected, again out of my expectation that it works correctly
-- 3. nil from function
local function getnil() end
local v3 = getnil() --> v3: nil
local r3 = getItemColor(v3) --> nilColor, unexpected I think
-- 4. integer const from function
local function get1() return 1 end
local v4 = get1() --> v4: integer = 1
local r4 = getItemColor(v4) --> unknown, but I don't know what should be the behavior
-- 5. integer from function
---@return integer
local function getint() return 1 end
local v5 = getint() --> v5: integer
local r5 = getItemColor(v5) --> integerColor, seems inconsistent with const integer case
-- 6. any from function
---@return any
local function getany() return end
local v6 = getany()
local r6 = getItemColor(v6) --> anyColor

Meaning, nil, unknown, any, are propagated without string transformation.

Let me know which one would work best. I am fine either way. If not, what would you expect in such cases?

I personally think skipping nil / unknown / any would be better 👍
But TBH I seldom use backtick in my projects, so my opinions may not be representative 😂 .

Again I am not author of LuaLS, so this may need opinions from maintainers. 🤔
But I do think that skipping those special types when processing would be good enough for this feature at the moment.
Since Completion is more important than perfection as author said: #3110 (comment) 😄

Hello @tomlau10 . I looked at this after your feeback.

I would leave the PR as-is. This indeed can end up creating weird types, but when you are using string you can also do weird stuff, namely, you can call getItemColor("nil") and you'll get nilColor. I think it is no big deal.

I have another variant of the code that works as follows:

---@type Vehicle|nil
local v1 = {}
local r1 = getItemColor(v1) --> r1: VehicleColor|nil

Meaning, nil, unknown, any, are propagated without string transformation.

Let me know which one would work best. I am fine either way. If not, what would you expect in such cases?

My use case is, I am writing Lua bindings for a major ECS, we need a syntax as follows:

---@class transform.Position : Component
---@class transform.PositionInstance
---@field x number
---@field y number
-- Retrieves the provided component value or nil if the component is not found
---@generic T
---@param component `T`Instance # component to retrieve
---@return T | nil
function entity:get(component) end
local pos = entity:get(transform.Position) --> pos: transform.PositionInstance|nil
if pos then
 print("Position: ", pos.x, pos.y)
else
 print("Entity does not have Position")
end

Please let me know your thoughts.

Maybe we could get something like:

---@generic T
---@param x `T`
---@return `T`Colour
local function f(x)
 return ...
end

Since there is a v4.0 rewrite ongoing, I think it's better to first ask opinions from maintainers 🤔 @sumneko @CppCXY
Otherwise when v4.0 is released, this new syntax / feature won't be supported on day one.

And if v4.0 already planned this feature in another syntax, then we should take an implementation which is compatible with that new syntax, so to minimize any future migration overhead.

I believe that too many peculiar syntaxes have been added to sumneko for various workarounds. This is neither consistent nor does it add cognitive burden. I think a better syntax would be to use built-in special generic classes based on supporting generics.

---@generic T
---@param a T
---@return CompositeType<T, "CCC">
function f(a)
end

Of course, this is just a suggestion. I understand that implementing this in LuaLS might be somewhat difficult.

If there is no exact planning on the syntax of how v4.0 will handle this feature, then I think the current best option is to just keep the existing syntax as is 🤔

• i.e. keeping the @param `T`Color for typename capturing
• DO NOT add a new syntax @return `T`Color
• the major reason is that we don't need to change the documentation 😂
  https://luals.github.io/wiki/annotations/#param

@Frityet

Maybe we could get something like:

---@generic T
---@param x `T`
---@return `T`Colour
local function f(x)
 return ...
end

I agree this is the syntax that would make sense, but it is not how the backticks capture works right now unfortunately.

@CppCXY @tomlau10
This PR only changes where 'T' is read from. As of today, 'T' comes from a string. I am just adding that it may come from the type of the passed-in expression also, in a manner that is coherent with what exists today.
The discussion of what to do for a future version is beyond the scope of this PR.

So I agree with @tomlau10 :

If there is no exact planning on the syntax of how v4.0 will handle this feature, then I think the current best option is to just keep the existing syntax as is 🤔

• i.e. keeping the @param `T`Color for typename capturing
• DO NOT add a new syntax @return `T`Color
• the major reason is that we don't need to change the documentation 😂
  https://luals.github.io/wiki/annotations/#param

... but adding the extra that 'T' may come from the type of the passed-in parameter, instead of exclusively deal with literal strings, otherwise I can't write the bindings to the ECS:

---@class transform.Position : Component
---@class transform.PositionInstance
---@field x number
---@field y number
-- Retrieves the provided component value or nil if the component is not found
---@generic T
---@param component `T`Instance # component to retrieve
---@return T | nil # T is actually `T`Instance
function entity:get(component) end
local pos = entity:get(transform.Position) --> pos: transform.PositionInstance|nil
if pos then
 print("Position: ", pos.x, pos.y)
else
 print("Entity does not have Position")
end

... but adding the extra that 'T' may come from the type of the passed-in parameter, instead of exclusively deal with literal strings

Yes, that's what I mean 😄

• adding the new feature in this PR, while keeping the existing capturing syntax