Incorrect type on string-key fields when combined with `[string]`

[justarandomgeek]

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?

Type Checking

Expected Behaviour

we have a (generated) class like

---@class GlobalModSettings
---@field ["heat-visualizer-updates-per-surface-per-tick"] ModSetting<int32>
---@field ["nixie-tube-update-speed-alpha"] ModSetting<int32>
---@field ["nixie-tube-update-speed-numeric"] ModSetting<int32>
---@field [string] ModSetting<(int32)|(double)|(boolean)|(string)|(Color)>

which has narrower types for the known entries and a general type for the "everything else" field, but when i then use this type like

local x = settings.global["heat-visualizer-updates-per-surface-per-tick"]

x is given the type from the [string] field, rather than the one that is an exact match for that specific string

Actual Behaviour

(this template is always off screen and i've written the whole thing in the first box...)

Reproduction steps

(this template is always off screen and i've written the whole thing in the first box...)

Additional Notes

No response

Log File

No response

[tomlau10]

workaround

I tested a bit and found that if you write the annotation as

---@class GlobalModSettings
---@field heat-visualizer-updates-per-surface-per-tick ModSetting<int32>
---@field nixie-tube-update-speed-alpha ModSetting<int32>
---@field nixie-tube-update-speed-numeric ModSetting<int32>
---@field [string] ModSetting<(int32)|(double)|(boolean)|(string)|(Color)>

then the type narrowing will work.

But this has a side effect:

• since heat-visualizer-updates-per-surface-per-tick contains special character -
• when you choose the auto completion for this field, the server will wrongly complete it as settings.global[heat-visualizer-updates-per-surface-per-tick] without double quotes 😕
• I think this is another bug
  => server should adds double quote if key name contains special character / is a reserve word

---

analysis

I have fixed a similar issue before in PR #3224, but further testing found out that this issue is a bit different 😕

• seems that for ---@field const_name type def, the server will treat them has having a concrete field key
  (i duno how to express this concept better 🙈 i am just an amateur contributor and dont know much about how language server works)
  => and always narrow to the defined type
  

  lua-language-server/script/vm/compiler.lua

  Lines 527 to 553 in d965edc

  	local sets = class:getSets(suri)
  	for _, set in ipairs(sets) do
  	if set.type == 'doc.class' then
  	-- check ---@field
  	for _, field in ipairs(set.fields) do
  	local fieldKey = guide.getKeyName(field)
  	if fieldKey then
  	-- ---@field x boolean -> class.x
  	if key == vm.ANY
  	or key == vm.ANYDOC
  	or fieldKey == key then
  	if not searchedFields[fieldKey] then
  	pushResult(field, true)
  	hasFounded[fieldKey] = true
  	end
  	end
  	goto CONTINUE
  	end
  	if key == vm.ANY or key == vm.ANYDOC then
  	pushResult(field, true)
  	goto CONTINUE
  	end
  	::CONTINUE::
  	end
  	end
  	end
  	copyToSearched()

• but for a ---@field [expr] type, server will merge all the types if the current value can be casted to the expr type.
  This syntax is not for defining a single concrete field key type, as you can write ---@field ["a"|"b"|"c"] string
  

  lua-language-server/script/vm/compiler.lua

  Lines 624 to 675 in d965edc

  	-- search for typed @field, eg: ---@field [string] boolean
  	-- only if type for this field key is not found
  	if not searchedFields[key] and key ~= vm.ANY and key ~= vm.ANYDOC then
  	for _, set in ipairs(sets) do
  	if set.type == 'doc.class' then
  	for _, field in ipairs(set.fields) do
  	local fieldKey = guide.getKeyName(field)
  	if fieldKey then
  	-- already processed above
  	goto CONTINUE
  	end
  	local keyType = type(key)
  	if keyType == 'table' then
  	-- ---@field [integer] boolean -> class[integer]
  	local fieldNode = vm.compileNode(field.field)
  	if vm.isSubType(suri, key.name, fieldNode) then
  	local nkey = '|' .. key.name
  	if not searchedFields[nkey] then
  	pushResult(field, true)
  	hasFounded[nkey] = true
  	end
  	end
  	else
  	local keyObject
  	if keyType == 'number' then
  	if math.tointeger(key) then
  	keyObject = { type = 'integer', [1] = key }
  	else
  	keyObject = { type = 'number', [1] = key }
  	end
  	elseif keyType == 'boolean'
  	or keyType == 'string' then
  	keyObject = { type = keyType, [1] = key }
  	end
  	if keyObject and field.field.type ~= 'doc.field.name' then
  	-- ---@field [integer] boolean -> class[1]
  	local fieldNode = vm.compileNode(field.field)
  	if vm.isSubType(suri, keyObject, fieldNode) then
  	local nkey = '|' .. keyType
  	if not searchedFields[nkey] then
  	pushResult(field, true)
  	hasFounded[nkey] = true
  	end
  	end
  	end
  	end
  	::CONTINUE::
  	end
  	end
  	end
  	copyToSearched()
  	end

• now knowing the bug behavior, the following can also trigger the same issue:

---@class Test
---@field [1] boolean
---@field [number] string
local Test
local v = Test[1]   --> v: boolean|string
-- expected is `boolean` only

---

Unfortunately I have no idea how this can be fixed / let alone what's the best way to fix it 😕

• Maybe when using the indexed field syntax and there is only 1 string literal, the server should treat it as a normal field key?
• but this approach will not work on the case ---@field ["a"|"b"|"c"] string => there are multiple string literals 😇

[justarandomgeek]

i should probably note that the specific-value strings will vary for each user who generates these depending on what other related bits they have installed locally

[tomlau10]

Maybe when using the indexed field syntax and there is only 1 string literal, the server should treat it as a normal field key?

By using this approach, I seem to make it work:

• modifying the local function parseIndexField in script/parser/luadoc.lua
  

  lua-language-server/script/parser/luadoc.lua

  Lines 292 to 300 in d965edc

  	local function parseIndexField(parent)
  	if not checkToken('symbol', '[', 1) then
  	return nil
  	end
  	nextToken()
  	local field = parseType(parent)
  	nextSymbolOrError ']'
  	return field
  	end

local function parseIndexField(parent)
    if not checkToken('symbol', '[', 1) then
        return nil
    end
    nextToken()
    local field = parseType(parent)
    nextSymbolOrError ']'
    if field and #field.types == 1
    and (
        field.types[1].type == 'doc.type.string'
        or field.types[1].type == 'doc.type.integer'
    )
    then
        -- for field name having special characters, eg:
        -- ---@field ["field-key"] string
        -- ---@field [1] boolean
        -- return them as a single named field
        return field.types[1]
    end
    return field
end

• after parsing an indexed field, if the types defined within only contains exactly 1 string or integer
  => just return this field type object directly
• this seems let the server "extract" the actual field literal when tracing the field type, and skips the original fallback index infer logic 🤔
• As noted above, this only works for ---@field ["key-a"] type but not for ---@field["key-a"|"key-b"] type.

---

Also I haven't tested if it would cause other problems.
You may want to try this patch and open a PR after adding more test cases 👀

[justarandomgeek]

As noted above, this only works for ---@field ["key-a"] type but not for ---@field["key-a"|"key-b"] type.

in general it seems like it should prefer the narrowest matching field def to allow for the second example but the first one would be sufficient for my immediate needs