Enum type is lost when given as union

[rhys-vdw]

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

Windows WSL

What is the issue affecting?

Type Checking

Expected Behaviour

A parameter of union of enum entries should accept those entries.

Actual Behaviour

---Command constants.
---
---@enum CMD
CMD = {
	---@type 40
	REPAIR = nil,

	---@type 90
	RECLAIM = nil,

	---@type 110
	RESTORE = nil,

	---@type 125
	RESURRECT = nil,

	---@type 130
	CAPTURE = nil,
}

---Called when a construction unit wants to "use his nano beams".
---
---@param unitID integer
---@param unitDefID integer
---@param action
---  | -1 # Build
---  | CMD.REPAIR # Repair
---  | CMD.RECLAIM # Reclaim
---  | CMD.RESTORE # Restore
---  | CMD.RESURRECT # Resurrect
---  | CMD.CAPTURE # Capture
---@return boolean actionAllowed
function SyncedCallins:AllowBuilderHoldFire(unitID, unitDefID, action) end

local x = nil ---@type SyncedCallins
local y = x:AllowBuilderHoldFire(5, 3, CMD.REPAIR)

The enum type is being resolved to 40 and losing information about it being an enum.

[{
	"resource": "/home/rhys/spring/rts/Lua/library/generated/library.lua",
	"owner": "_generated_diagnostic_collection_name_#1",
	"code": "param-type-mismatch",
	"severity": 4,
	"message": "Cannot assign `40` to parameter `-1|CMD.CAPTURE|CMD.RECLAIM|CMD.REPAIR|CMD.RESTORE...(+1)`.\n- `40` cannot match `-1|CMD.CAPTURE|CMD.RECLAIM|CMD.REPAIR|CMD.RESTORE...(+1)`\n- `40` cannot match any subtypes in `-1|CMD.CAPTURE|CMD.RECLAIM|CMD.REPAIR|CMD.RESTORE...(+1)`\n- Type `40` cannot match `CMD.CAPTURE`\n- Type `number` cannot match `CMD.CAPTURE`\n- Type `40` cannot match `CMD.RESURRECT`\n- Type `number` cannot match `CMD.RESURRECT`\n- Type `40` cannot match `CMD.RESTORE`\n- Type `number` cannot match `CMD.RESTORE`\n- Type `40` cannot match `CMD.RECLAIM`\n- Type `number` cannot match `CMD.RECLAIM`\n- Type `40` cannot match `CMD.REPAIR`\n- Type `number` cannot match `CMD.REPAIR`\n- Literal `40` cannot match integer `-1`",
	"source": "Lua Diagnostics.",
	"startLineNumber": 3090,
	"startColumn": 40,
	"endLineNumber": 3090,
	"endColumn": 50
}]

Reproduction steps

Paste code example above into VSCode.

Additional Notes

No response

Log File

No response

[tomlau10]

AFAIK, when you define @enum CMD, those CMD.REPAIR / CMD.RECLAIM / ... "enum value types" are some hidden types and not supposed to be used. 🤔
They might be some unintended side effects (in the current implementation), as reported in these issues:

• Ability to use specific enum keys as params #2274
• Something not okay with enum types #2377

@enum type is expected to be used just like this:

---@param unitID integer
---@param unitDefID integer
---@param action
---  | -1 # Build
---  | CMD # all command enums
---@return boolean actionAllowed
function SyncedCallins:AllowBuilderHoldFire(unitID, unitDefID, action) end

=> then type checking can be done correctly

• or you have to define all those CMD.* using @alias but not @enum
• @enum is for making use of an existing table, and gives the union of all values as an enum type 😕

Additional note

I know that some users even make use of the unintended enum type behavior mentioned above, to create opaque enum:

the actual numeric values are not supposed to be visible anywhere, so they're masked away to just their types here for the overload resolution to see

inside the gist link of this comment: #2758 (comment) and #2758 (comment)

[rhys-vdw]

@enum type is expected to be used just like this: [...]

The problem here is that this function doesn't accept all of CMD, it accepts only a subset of those values.

@enum is for making use of an existing table, and gives the union of all values as an enum type 😕

CMD is an existing table in this case: https://github.com/beyond-all-reason/spring/blob/1edacaeea158c47658423a345c5be1f17201d4e7/rts/Lua/LuaConstCMD.cpp#L110-L150

or you have to define all those CMD.* using @alias but not @enum

Right, so essentially manually implementing my own @enum that behaves as one might expect?

---Command constants.
---@enum CMD
CMD = {
	---@alias CMD.REPAIR 40
	---@type CMD.REPAIR
	REPAIR = nil,

	---@alias CMD.RECLAIM 90
	---@type CMD.RECLAIM
	RECLAIM = nil,

	---@alias CMD.RESTORE 110
	---@type CMD.RESTORE
	RESTORE = nil,

	---@alias CMD.RESURRECT 125
	---@type CMD.RESURRECT
	RESURRECT = nil,

	---@alias CMD.CAPTURE 130
	---@type CMD.CAPTURE
	CAPTURE = nil,
}

---@param cmd CMD.REPAIR|CMD.CAPTURE
function captureOrRepair(cmd)
end

captureOrRepair(CMD.REPAIR)
captureOrRepair(CMD.RECLAIM)


---@param cmd CMD
function anyCmd(cmd)
end

anyCmd(CMD.RESTORE)
anyCmd(1337)

The problem with this approach:

• It fills the global namespace with all these CMD.* alias types, which will end up in our documentation output.
• It's unpleasant to look at (and confusing to maintain).
• It seems to define two types with the same name for each enum entry. The explicit alias and the internal CMD.* type defined by the @enum. This feels fragile. I assume this is doing some sort of shadowing or relying on poorly defined behaviour (e.g. what if LLS changes and prefers to consider the actual CMD.REPAIR enum field instead of my alias of the same name?).

Would it be difficult to change the implementation so the @enum types can be used individually? It seems desirable. I'm able to do (1|2|3) or (MyAlias1|MyAlias2), why not (CMD.FOO|CMD.BAR)?

It seems as though something similar to the above could be done implicitly to each enum value to make it a fully-fledged type/alias.

[tomlau10]

... the internal CMD.* type defined by the @enum

I think the root issue here is that those internal enum subtype have undefined behaviour.
They are undocumented and thus are not supposed to be used by users 😕 (in the current implementation of LuaLS)
https://luals.github.io/wiki/annotations/#enum
However whether this behaviour is a side effect / a bug / a mis-implemetation, it needs to be clarified by maintainers / authors.

---

Would it be difficult to change the implementation so the @enum types can be used individually?

I am not familiar with this part of the codebase, so I can't tell 🙁

I'm able to do (1|2|3) or (MyAlias1|MyAlias2), why not (CMD.FOO|CMD.BAR)?

Just a few observations here:

• (MyAlias1|MyAlias2) are alias, while CMD.FOO|CMD.BAR are (currently) not.
  They are just some undocumented / auto generated internal types
• alias will be shown in the global namespace (?), but those internal types won't.
• and if those internal types are somehow treated as alias, then it will conflict with your requirement (?)
  which you said: It fills the global namespace with all these CMD.* alias types, which will end up in our documentation output.

So conceptually, you are now using EnumType.field syntax to access the field type of a table which marked as enum. But this just not how the type name system works in LuaLS.
Similarly for a @class type, you just can't use ClassType.field to access the field type of a class 🤔

---@class MyClass
---@field a integer

---@param p MyClass.a # this is invalid, and conceptually incorrect
function test(p) end

[rhys-vdw]

I am not familiar with this part of the codebase, so I can't tell 🙁

I appreciate the explanation of how it works and the problems, but I think I might wait for a developer to respond then so I can work out how to move forward.

I think I have two best choices to address this problem in the immediate:

Option 1

Respect current limitations and rely on param description for usage:

---Called when a construction unit wants to "use his nano beams".
---
---@param unitID integer
---@param unitDefID integer
---@param action (-1| CMD) Will be -1 when this is a build action. Otherwise will be `CMD.RECLAIM`, `CMD....`.
---@return boolean actionAllowed
function SyncedCallins:AllowBuilderHoldFire(unitID, unitDefID, action) end

Option 2

Export two versions of the library, one with the hacky @alias annotations which to be used in IDE, and one without that can be fed into emmylua_doc_cli for generating documentation.

[tomlau10]

I think I have two best choices to address this problem in the immediate:

By experimenting with your recent example, I might have found another workaround 🤔

• define the enum value type along with the internal type

---Command constants.
---@enum CMD
CMD = {
	---@type 40|CMD.REPAIR
	REPAIR = nil,

	---@type 90|CMD.RECLAIM
	RECLAIM = nil,

	---@type 110|CMD.RESTORE
	RESTORE = nil,

	---@type 125|CMD.RESURRECT
	RESURRECT = nil,

	---@type 130|CMD.CAPTURE
	CAPTURE = nil,
}

the good

• no @alias involved => no extra type will be exported to doc json

the bad

But this has some side effects:

• now each field CMD.XXX in the CMD table has 2 types
• those internal types CMD.XXX has no integer value, they are just some internal type names
• so when you write captureOrRepair(CMD.REPAIR), you will get a param-type-mismatch warning
  because the integer part cannot match the internal type CMD.XXX
  • HOWEVER ‼ you can silence this by enabling "type.weakUnionCheck": true
    (which I always include in my projects) 🤔
    => since the internal type part satisfy the CMD.* param type specified in your captureOrRepair() function
• but since those CMD.REPAIR are just some type names without integer value
  you cannot write captureOrRepair(40) even if the enum value match
  => because internally CMD.REPAIR is a special internal enum type, which cannot match a literal value

[rhys-vdw]

Interesting! I will experiment with these approaches.

Given I'm adding types to a massive two-decade old project with tens (hundreds?) of contributors and years of development ahead of it, I should probably err on the side of simplicity rather than overfitting the types to the current version of LLS I think.

I do think changing this behaviour in LLS would be a big win for the project (seems this issue of enums arises in a few contexts, as you pointed out).

Thanks for helping explain and explore this issue @tomlau10. 🙏