False-Positive Inject-Field diagnostic suggests duplicate ---@class statement

[Eikonium]

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?

Diagnostics/Syntax Checking

Expected Behaviour

---@class Animal
Animal = {
    name = 'default'
}

---@param name string
---@return Animal
function Animal.create(name)
    local new = {} ---@type Animal
    new.name = name
    return new
end

should not produce any warning.

Actual Behaviour

The code as presented above triggers the inject-field diagnostic.

The warning message suggests resolving it by replacing ---@type by ---@class. This does in fact clear the warning, but leads to a duplicate Animal-class annotation as below.

---@class Animal
Animal = {
    name = 'default'
}

---@param name string
---@return Animal
function Animal.create(name)
    local new = {} ---@class Animal
    new.name = name --this doesn't trigger a warning anymore
    return new
end

1. The ---@type annotation should not produce a warning, as new.name = name is not injecting a new field, but rather relying on a previously defined one.
2. The warning message should not suggest to duplicate the class-statement.
3. The suggested use of ---@class (using it on an object of the class instead of the class itself) is also not documented on https://luals.github.io/wiki/annotations/#class

Reproduction steps

Paste the code from the Expected Behaviour box into an empty Lua-document in Visual Studio Code.

Additional Notes

Similar issues have been brought up in https://github.com/LuaLS/lua-language-server/issues/2303 and https://github.com/LuaLS/lua-language-server/issues/2341. I don't consider these to be duplicates of my issue, because they are about injected-field misbehaviour in nested tables.

Log File

No response

[firas-assaad]

I agree that your code should not produce warnings, and the current inject-field behavior is too strict. There's a concept of (exact) classes that have more restrictions on how they're used, and maybe the warning should be limited to those classes. For example, a class defined in C might not allow you to add new fields after an object is created.

But even if the class was defined as exact in your example, I'd argue that the language server should still be able to infer that name isn't a new field since it already knows about its default value (e.g. if you hover over Animal). In that case a missing-field warning at local new = {} might be more appropriate, but only if checking subsequent assignments is too computationally expensive.

As it stands you can explicitly define the field under ---@class Animal, but it'll have to be optional (@field name? string) or you'll get a missing-field warning. This is documented on diagnostics/#inject-field. A slightly better option is to set any variables when creating the table (local new = { name = name }). Since there are different ways to fix inject-field depending on what you're trying to do, the warning message shouldn't give a single obscure suggestion (or maybe it should suggest defining the field explicitly, since it's the more common solution in my experience).