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

Improve generic pattern to support optional, union, array. Fixed regression. #3031

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TIMONz1535
Copy link
Contributor

@TIMONz1535 TIMONz1535 commented Jan 6, 2025

I examined the original generic pattern code in more detail and realized that it incorrectly handles cases when it cannot parse further. We need to partially successfully parse, not cancel it by return nil.

---@param a `TC`#Comment -- ok
---@param a `TC`? -- ok
---@param a `TC`|integer -- ok
---@param a `TC`[] -- ok
  • Generic pattern now supports optional, union, array and comment without a space, like all other types.
    The signs <> are also correctly recognized, but the generics don't support it at all.
---@param a string#Comment -- ok
---@param a `TCP`2#Comment -- now ok
---@param a `TCP`2? -- now ok
---@param a `TCP`2|integer -- now ok
---@param a `TCP`2[] -- now ok
My complete test
---@param a string#Comment
---@return string
local function new(a) end
local obj = new("Vehicle")

---@generic T
---@param a T#Comment
---@return T
local function new(a) end
local obj = new("Vehicle")

---@generic TC
---@param a `TC`#Comment
---@return TC
local function new(a) end
local obj = new("Vehicle")

---@generic TCP
---@param a `TCP`2#Comment
---@return TCP
local function new(a) end
local obj = new("Vehicle")



---@param a string?Comment
---@return string?
local function new(a) end
local obj = new("Vehicle")

---@generic T
---@param a T?Comment
---@return T?
local function new(a) end
local obj = new("Vehicle")

---@generic TC
---@param a `TC`?Comment
---@return TC?
local function new(a) end
local obj = new("Vehicle")

---@generic TCP
---@param a `TCP`2?Comment
---@return TCP?
local function new(a) end
local obj = new("Vehicle")



---@param a string[]Comment
---@return string[]
local function new(a) end
local obj = new({"Vehicle"})

---@generic T
---@param a T[]Comment
---@return T[]
local function new(a) end
local obj = new({"Vehicle"})

---@generic TC
---@param a `TC`[]Comment
---@return TC[]
local function new(a) end
local obj = new({"Vehicle"})

---@generic TCP
---@param a `TCP`2[]Comment
---@return TCP[]
local function new(a) end
local obj = new({"Vehicle"})



---@param a string|integer #     the type
---@return string|integer
local function new(a) end
local obj = new("Vehicle")

---@generic T
---@param a T|integer #     the type
---@return T|integer
local function new(a) end
local obj = new("Vehicle")

---@generic TC
---@param a `TC`|integer #     the type
---@return TC|integer
local function new(a) end
local obj = new("Vehicle")

---@generic TCP
---@param a `TCP`2|integer #     the type
---@return TCP|integer
local function new(a) end
local obj = new("Vehicle")

  • Generic return can be optional.
---@return T?

local some -- `unknown`
local k, v = next() -- `nil, nil` instead of `unknown, unknown`
local k, v = next({}) -- `nil, nil` instead of `unknown, unknown`
local k, v = next({1}) -- `integer?, integer?` instead of `integer, integer`
local k, v = next({some}) -- `integer?, nil` instead of `integer, unknown`

I'm checking the protoNode (self.proto) in generic:resolve. This node is created in server\script\vm\compiler.lua:1753 in : case 'function.return' where rtn (proto) has optional but vm.createGeneric is not.

if hasGeneric then
---@cast sign -?
vm.setNode(source, vm.createGeneric(rtn, sign))
else
vm.setNode(source, vm.compileNode(rtn))
end

Well, I think generic:resolve is the best place.

if protoNode:isOptional() then
result:addOptional()
end
return result

But there is some change here. Previously the optional unresolved generic was unknown, but now it chooses between unknown|nil so becomes nil

local some -- `unknown`

-- its correct because generic is `nil`
local k, v = next() -- `nil, nil` instead of `unknown, unknown`

-- is it correct last `nil` with explicit generic `unknown`?
local k, v = next({some}) -- `integer?, nil` instead of `integer, unknown`

I think its OK, otherwise its different issue.


Minor changes

  • Removed pattern mask m.R('__') from name parsing because it the same as m.S('_'), so nothing changed.
  • Rename next variable to nextTp to avoid names conflict.
  • NextComment takes 2nd argument boolean peek, not a string 'peek'.

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 this pull request may close these issues.

1 participant