lua-language-server icon indicating copy to clipboard operation
lua-language-server copied to clipboard

@field private in @meta file triggers warning even when accessed within the same class

Open Mayron opened this issue 6 months ago • 7 comments

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?

Annotations

Expected Behaviour

When defining a class in a ---@meta file with @field private annotations, the Lua language server warns about accessing those private fields even when they are accessed within the implementation of the same class (just in a separate file). This makes private fields effectively unusable if you're organizing types in meta files.

The language server should not warn about access to @field private fields when they are accessed within the implementation of the class they belong to, even if that implementation is in a separate file.

I think this is by design rather than a bug, so maybe this is a feature request?

Actual Behaviour

Even though I'm accessing the private field in the implementation of that class, I see a warning:

Image

Reproduction steps

  1. In types.lua (meta file)
---@meta

---@class (exact) ModuleMixin
---@field private __loaded boolean
---@field Load fun(self)
---@field IsLoaded fun(self): boolean
---@field Init fun(self)
  1. ModuleMixin.lua (implementation)
---@type SmartChat.ModuleMixin
local ModuleMixin = {}

function ModuleMixin:Init()
  self.__loaded = false  -- <-- LuaLS warns that __loaded is private
end

Additional Notes

I use a @meta file to separate type annotations from implementation, which is common in larger projects. This issue makes it difficult to use private meaningfully unless everything is co-located in the same file, which defeats the purpose of a metadata/type declaration file.

Would it be possible to make private refer to "the class definition context" rather than "the file only"?

Thanks!

Log File

No response

Mayron avatar Jun 26 '25 09:06 Mayron

IIUC, the problem is because you used @type annotation

  • this makes the local ModuleMixin become a typed variable (or an instance variable?) instead of a class variable
  • if you use ---@class ModuleMixin then it will work correctly 🤔
---@class ModuleMixin
local ModuleMixin = {}

function ModuleMixin:Init()
    self.__loaded = false  -- no more warning
end

but this has a side effect

I noticed that you used the (exact) class feature, which will warn if you try to inject any field/method that are not predefined. But after you used ---@class in you implementation, seems it will not warn you about method injection ... 😕

---@class ModuleMixin
local ModuleMixin = {}

function ModuleMixin:Init()
    self.__loaded = false  -- no more warning
end

function ModuleMixin:custom()   --> no `inject-field` warning for this
    self.x = 1  --> this will still warn `inject-field` as expected
end

tomlau10 avatar Jun 26 '25 09:06 tomlau10

@tomlau10 thank you! I was just thinking about this as a solution. I wasn't sure if having @class twice in the meta file and implementation file was a bad practice, or if whatever analyser VS Code uses would think there were duplicate classes but this seems to work. Brilliant! 👍

Mayron avatar Jun 26 '25 11:06 Mayron

I guess this issue can be closed, but it might be worth updating the docs (unless I missed this) for the meta or annotation sections to clarify this. Unless, this isn't how meta files are supposed to work and I'm doing it wrong lol

Mayron avatar Jun 26 '25 11:06 Mayron

I noticed that you used the (exact) class feature, which will warn if you try to inject any field/method that are not predefined. But after you used ---@class in you implementation, seems it will not warn you about method injection ... 😕

Yea, I noticed this too. I had to disable a few warnings in the metafile to also let me define the methods in a table rather than by using @field (so that I could describe the method with additional annotation) but the exact annotation didn't like this.

---@diagnostic disable: duplicate-index, inject-field

---@class (exact) ModuleMixin
---@field private __loaded boolean
---@field private __name string
---@field private __localizedName string
---@field private __enabled boolean
---@field private __callbacks ModuleCallbacks
local ModuleMixin = {
  ---@param config ModuleConfig
  Init = function(self, config) end;
  ---@param enabled boolean
  SetEnabled = function(self, enabled) end;
  ---@return boolean
  IsEnabled = function(self) end;
  Load = function (self) end;
  ---@return boolean
  IsLoaded = function (self) end;
};

Mayron avatar Jun 26 '25 11:06 Mayron

Another better approach is to allow both meta files and actual files to define the same class, with one serving as the actual implementation and the other as annotations

CppCXY avatar Jun 26 '25 12:06 CppCXY

I wasn't sure if having @class twice in the meta file and implementation file was a bad practice

In LuaLS @class can be used multiple times. This is similar to the partial class concept of C# where you can break down the definition of a very large class into multiple files. My team employed this annotation pattern in our project 👀 .

Unless, this isn't how meta files are supposed to work and I'm doing it wrong lol

I am not sure if defining class/method signature in a meta file then use it to enforce the implementation is the intended way to use the annotation 😕. AFAIK, meta file is used to define header of a library, whether it is a 3rd party library or a c library, that you cannot write annotation to. (that's why you need a meta file).

Of course I can understand what you want to achieve here, that you want to separate the annotation and the actual code as much as possible. It's good that you found a way to workaround it. 😂


Okay back to your last reply/question: https://github.com/LuaLS/lua-language-server/issues/3211#issuecomment-3008185862

At first I think that you don't need to define methods in a table, you can just write them as method in your meta file

---@meta

---@class ModuleConfig
---@class ModuleCallbacks

---@class (exact) ModuleMixin
---@field private __loaded boolean
---@field private __name string
---@field private __localizedName string
---@field private __enabled boolean
---@field private __callbacks ModuleCallbacks
local ModuleMixin = {}

---@param config ModuleConfig
function ModuleMixin:Init(config) end

---@param enabled boolean
function ModuleMixin:SetEnabled(enabled) end

---@return boolean
function ModuleMixin:IsEnabled() end

function ModuleMixin:Load() end

---@return boolean
function ModuleMixin:IsLoaded() end

⚠ but after that I noticed the dilemma here ...

  • if in the meta file, these methods are defined outside the table then in the ModuleMixin.lua (implementation) you will get duplicate-set-field warnings 🤦
  • that's because you really "defined" the same named methods for the same class twice and this warning will only go away if you use @type instead of @class, which is an optimization done by me: https://github.com/LuaLS/lua-language-server/pull/2859/commits/d2df7788d526960a427698fe66c73c2831dd2730 (before that pr, you will get duplicate-set-field no matter you use @class or @type for the namespace variable)
  • however if you use @type there, then we will be back to the starting point ... the self.__loaded = true will throw invisible warning again 😇

I just found a hack ?! 🤯

As you may have found out, @class for the same class can actually be defined multiple times. So I just tried to define the exact attribute in another @class annotation Wow, seems it works

  • type.lua
---@meta

---@class ModuleConfig
---@class ModuleCallbacks

---@class ModuleMixin
---@field private __loaded boolean
---@field private __name string
---@field private __localizedName string
---@field private __enabled boolean
---@field private __callbacks ModuleCallbacks
local ModuleMixin = {
    ---@param config ModuleConfig
    Init = function(self, config) end;
    ---@param enabled boolean
    SetEnabled = function(self, enabled) end;
    ---@return boolean
    IsEnabled = function(self) end;
    Load = function (self) end;
    ---@return boolean
    IsLoaded = function (self) end;
};

---@class (exact) ModuleMixin
-- now mark this class as exact class

=> then you don't need the ---@diagnostic disable: duplicate-index, inject-field 😄

tomlau10 avatar Jun 26 '25 12:06 tomlau10

🤯 🤯 I think I found a solution (an optimization / a fix to be accurate) for the duplicate-set-field warnings issues

  • if the def node is inside a meta file => it should not take part in the duplicate-set-field diagnostics => because it is most probably just a defined interface https://github.com/LuaLS/lua-language-server/blob/32fec3cc99af8b9a1e45c2455a8c3bd0d3e38f66/script/core/diagnostics/duplicate-set-field.lua#L52-L60
  • add the following
            if vm.isMetaFile(guide.getUri(def)) then
                -- def node is inside meta file, this should be treated as interface only
                goto CONTINUE
            end
  • now if the same named class method is originally defined in a meta file, it will not be considered as duplicate-set-field but if you define the same named class method twice, both outside of meta files => they will still trigger duplicate set field as usual

With this fix, one can just write the interface using usual class:method style. No more duplicate-set-field in ModuleMixin:Init under main.lua 🥳

Image

tomlau10 avatar Jun 26 '25 13:06 tomlau10