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

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

Closed
Eikonium opened this issue Feb 18, 2024 · 1 comment · Fixed by #2747
Closed

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

Eikonium opened this issue Feb 18, 2024 · 1 comment · Fixed by #2747

Comments

@Eikonium
Copy link

Eikonium commented Feb 18, 2024

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.
grafik

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 #2303 and #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
Copy link
Contributor

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).

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 a pull request may close this issue.

2 participants