You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It would be great if I can override fields, not just shadow them, to retain their signature and doc.
---@class (exact) Alocala= {}
---Test Comment---@paramteststring Some argumentfunctiona:Method(test) end---@class (exact) B:Alocalb= {}
functionb:Method(test) end---@typeBlocaltesttest:Method("") -- Annotations are gone
The signature wont change for overridden methods, and thus I would like to avoid copy pasting the whole doc for each method. How would I resolve this efficiently?
I looked into plugins and I can e.g. replace ---@override with a ---@see parent.method, but I don't know how I can retrieve the parents function definition directly.
Oh, looks like the Wiki docs aren't up to date anymore. There is more info on plugins on the project's GitHub pages: https://luals.github.io/wiki/plugins/. I can't really help you from here, I haven't used plugin api.
Alright thank you for the links, I made a plugin which injects ---@alias at definitions, and ---@type and ---@see at overrides. Only param/return comments disappear but otherwise seems to be working fine for me.
However I struggle to understand another concept, maybe you can help me here :)
---@class (exact) A---@fieldoxnumberlocala= {}
functiona:init()
--This is fineself.ox=0end---@class (exact) B:Alocalb= {}
functionb:init()
--This is an injection and thus invalidself.ox=0end---@typeBlocalx--But this is fine again?x.ox=0
Why does it want to inject a new field into B in the B's init? How can I specify that its fine to inject when its already inherited?
Another viewpoint would be: I don't want inheritance, but composition. Right now I have A.ox AND B.ox, but in my case it doesn't matter who introduced that value, its now just ox.
In my opinion, inject field diagnostic isn't smart enough yet and has its false positives. I suggest opening an issue or looking if an issue about this was already reported.
Alright thank you for the links, I made a plugin which injects ---@alias at definitions, and ---@type and ---@see at overrides. Only param/return comments disappear but otherwise seems to be working fine for me.
However I struggle to understand another concept, maybe you can help me here :)
---@class (exact) A---@fieldoxnumberlocala= {}
functiona:init()
--This is fineself.ox=0end---@class (exact) B:Alocalb= {}
functionb:init()
--This is an injection and thus invalidself.ox=0end---@typeBlocalx--But this is fine again?x.ox=0
Why does it want to inject a new field into B in the B's init? How can I specify that its fine to inject when its already inherited?
Another viewpoint would be: I don't want inheritance, but composition. Right now I have A.ox AND B.ox, but in my case it doesn't matter who introduced that value, its now just ox.
Could you please consider sharing your plug-in? I'm stuck with then same issue.
Appologise if you've already done publishing it. I've tried and failed to find it.
Thanks for the work and the notification, but there's still some distance to what I need.
In my use case, I have lots of objs which are only generated at runtime but customized at coding stage. They're still not getting any auto completion inside their overridden function bodies.
For example, I need to define the logic of diferent character skills, but skill information is only fetched at runtime and it's the same with skill objs. Thus, the code will end up like this:
To make it more complicated, the definition of skill, generate_skill_objs, list, list.skill1.onBegin all belongs to different files.
Could someone please look into this? Can't be more grateful.
If I understand the recent change correctly, it's for child class overriding parent class's method.
In the code you provided, the list is table<string, skill>, which means list.skill1 is still skill class (not a child class). Therefore luals will think you are redefining the skill:onBegin() in the skill class, so it will instead throw a duplicate-set-field warning.
In order to make use of the recent change, you have to create new class for each of your skill, and extending the base skill class, like the following
-- same code to generate `list` as above---@classskill1:skilllocalskill1=list.skill1functionskill1:onBegin(casterID, targetID)
print(casterID, targetID) --< integer, integerendfunctionskill1:onEnd(casterID, targetID)
print(casterID, targetID) --< integer, integerend
It can be interpreted as skill1 is implementing the skill interface 🤔
But as you can see, this requires an extra @class annotation for each of your skill, and you said the definitions belong to different files which makes things more complicated, so it may not suit you need... 😇 @NicameOurister
edit:
Seems it doesn't matter if all the definitions are in different files 🤔 Because right after using:
---@classskills1:skilllocalskill1=list.skill1
luals will ignore the original type in list.skill1 and cast it to the new skills1 type
and you can start overriding methods in the local skill1 class variable, while still getting the parent method's params' type
If I understand the recent change correctly, it's for child class overriding parent class's method. In the code you provided, the list is table<string, skill>, which means list.skill1 is still skill class (not a child class). Therefore luals will think you are redefining the skill:onBegin() in the skill class, so it will instead throw a duplicate-set-field warning.
In order to make use of the recent change, you have to create new class for each of your skill, and extending the base skill class, like the following
-- same code to generate `list` as above---@classskill1:skilllocalskill1=list.skill1functionskill1:onBegin(casterID, targetID)
print(casterID, targetID) --< integer, integerendfunctionskill1:onEnd(casterID, targetID)
print(casterID, targetID) --< integer, integerend
It can be interpreted as skill1 is implementing the skill interface 🤔 But as you can see, this requires an extra @class annotation for each of your skill, and you said the definitions belong to different files which makes things more complicated, so it may not suit you need... 😇 @NicameOurister
edit: Seems it doesn't matter if all the definitions are in different files 🤔 Because right after using:
---@classskills1:skilllocalskill1=list.skill1
luals will ignore the original type in list.skill1 and cast it to the new skills1 type
and you can start overriding methods in the local skill1 class variable, while still getting the parent method's params' type
Thanks for comment.
The recent change does cover part of my need, but not all of it. I know that I can do it this way. but there's still reason for the need of a more powerful implement.
In my case, there are tens of skill objs in every character's script, and the number of character script is more. It will be considerable lines of redundant code and makes it easier to cause name conflict.
Thus, I'm still hoping that we can just skip all those repeat annotations. Hopefully this enhancement could solve all the same related issues in #477, #1779 and #1757.
btw your link for #1779 and #1757 are incorrect, they all points to #477 🙈
Actually you can just write #1779 or paste the link https://github.com/LuaLS/lua-language-server/issues/1779 when you write comment, and github will be smart enough to turns it into [#1779](https://github.com/LuaLS/lua-language-server/issues/1779), which can be verified in Preview tab. No need to write the url markdown in full form 😄
btw your link for #1779 and #1757 are incorrect, they all points to #477 🙈
Actually you can just write #1779 or paste the link https://github.com/LuaLS/lua-language-server/issues/1779 when you write comment, and github will be smart enough to turns it into [#1779](https://github.com/LuaLS/lua-language-server/issues/1779), which can be verified in Preview tab. No need to write the url markdown in full form 😄
Strange, if I define the interface using @type fun() style, then when overriding it, params' types are there ‼️ 🤔
(although it will still trigger duplicate-set-field warnings)
---@classskilllocalskill= {}
---@typefun(self: skill, casterID: integer, targetID: integer)functionskill:onBegin(casterID, targetID) end---@typefun(self: skill, casterID: integer, targetID: integer)functionskill:onEnd(casterID, targetID) end---@returntable<string, skill>localgenerate_skill_objs=function ()
return {}
endlocallist=generate_skill_objs()
functionlist.skill1:onBegin(casterID, targetID) end--< integer, integer, but with `duplicate-set-field` warningfunctionlist.skill1:onEnd(casterID, targetID) end--< integer, integer, but with `duplicate-set-field` warning
You have to define it using @field first, then all the methods will have params type infer && no duplicate-set-field warnings
But the downside is you can't add descriptions to your params, and you have to write the definition in 1 line ... (fun() syntax do not support multi line)
It works as needed and does really solved the biggest problem, but it produces some little new problems.
The most obvious problem is that the ---field definition will mask out the real code and becomes the only target of that function's definition jump.
I can totally accept those extra ---field definitions . But I'm still kind of bothered with the definition jump mask problem. After all, the whole purpose of mine is to make my code library as accessible as possible for others.
I'm going to settle with the current solution while keep looking for a better way. Thanks again for the help.
Strange, if I define the interface using @type fun() style, then when overriding it, params' types are there
...
I'm still trying to find out why @param style won't work
I think I have successfully made it working without breaking anything 🎉 (at least all existing tests passed).
Root cause
I believe the function param infer logic resides in here:
-- local call ---@type fun(f: fun(x: number));call(function (x) end) --> x -> number
localfuncNode=vm.compileNode(func)
localfound=false
forninfuncNode:eachObject() do
ifn.type=='doc.type.function' andn.args[aindex] then
The reason why @type fun() works but not @param, is because the former one is of doc.type.function type. Here the logic will only consider the arguments' node types in a doc.type.function (the fun() annotation)
If I change the if conditions in line 1124 to include function type, then it works!
But with some side effects and breaking the tests... 🙁
Fixing side effects
I think it's because when a function is defined on its own, it will still have a function infer type. And this causes problems when trying to infer its own params' type by its own definition => loop => causing the types to be unknown instead of any
To fix this, I found out that it shall skip the infer if the argument source object is identical to self source:
if (n.type=='doc.type.function' orn.type=='function')
andn.args[aindex] andn.args[aindex] ~=sourcethen
note the n.args[aindex] ~= source added here => then all existing tests passed 🙂
Fixing duplicate set field
We still have to deal with the duplicate-set-field warning. The related logic is here:
So we have to find a way to skip the warning of this overriding class function / method use case.
The one that I can think of is that:
whenever the function definition (def in the above logic) is defined within a class namespace, we should allow overriding it (src in the above logic) in a non-class namespace.
i.e. if we mark ---@class A; local A = {}, here A has a class attached. If there are duplicated function A:f() end; function A:f() end
=> should throw warnings
---@classAlocalA= {}
functionA:f() endfunctionA:f() end-- should throw warnings
on the other hand, if it is just a type variable ---@type A; local a = {}, here a has no class attached. Then we should allow function a:f() end for overriding
---@classAlocalA= {}
functionA:f() end---@typeAlocala= {}
functiona:f() end-- allow this
So I up come with the following (add it below line 70)
ifvm.getDefinedClass(guide.getUri(def), def.node)
andnotvm.getDefinedClass(guide.getUri(src), src.node)
then-- allow type variable to override function defined in class variablegotoCONTINUEend
Conclusion
With the above proposed change, now we can have param type inference in your original annotation style, and without duplicate-set-field warnings 👀
---@classskilllocalskill= {}
---@paramcasterIDinteger---@paramtargetIDintegerfunctionskill:onBegin(casterID, targetID) end---@typetable<string, skill>locallist= {}
functionlist.skill1:onBegin(casterID, targetID)
print(casterID, targetID) -- integer, integer-- no `duplicate-set-field` as wellendfunctionlist.skill2:onBegin(casterID, targetID)
print(casterID, targetID) -- integer, integer-- no `duplicate-set-field` as wellend
I will need some time to do a more in-depth test before I create a PR. Meanwhile you may try to patch this in your local copy of luals to see the actual effect 😄@NicameOurister
I think it is no need to check duplicate-set-field since Lua is weak typed and dynamic Language.
If you wanna static features why not enjoy such like C# and TypeScript.
So I advice that it should remove the duplicate-set-field since Lua doesn't own that language level concept.
duplicate-set-field diagnostic is useful in the sense that, in sometime we just accidentally used the same function name for a completely unrelated logic, instead of intentionally to do so.
Or maybe that after solving git merge conflict, two of the same named function exist in the same class
=> this usually hints a bug in the code, as in the same class, each method name should be unique.
two :setName() are defined, this maybe due to wrong conflict resolution between 2 teammates
although this code is totally valid in lua (in which the later definition will just overwrite the previous one)
this code makes no sense and leads to confusion: maybe when reading the codebase, one will think that :setName() will set to the field self.name, but actually now it will set to self.playerName instead.
This had helped me successfully found a bug in my team's project🤦♂️ where the method wrongly defined using another namespace (instead of the namespace used in that file)
=> this caused duplicate-set-field in that other namespace
localParent=require"Parent"-- this file should define methods of `Child` class---@classChild:ParentlocalChild= {}
functionChild:f1() endfunctionChild:f2() endfunctionParent:f3() end--> don't know why suddenly used wrong namespace here ...-- and `Parent` does already have a `:f3()`--> duplicate-set-field triggeredfunctionChild:f4() end
If you mean "we just accidentally used the same function name for completely unrelated logic, instead of doing so intentionally," and think that’s important for Lua, then why don’t other linters, like Robocop and PEP8 linters, focus on this issue?
In my opinion, this kind of approach is only useful for specific scenarios, perhaps for beginners or for your own company’s logic, rather than being a common Lua practice.
It feels like gilding the lily.
And here are some additional comments:
Accidental function redefinition:
This is clearly a developer’s mistake, not an issue with the language or the linter itself. If a team encounters such issues, it should be a problem with the team’s code management, code review process, and development standards, not a universal code style issue. In many cases, redefining functions is a team-level management issue, and a linter should not enforce it as a rule.
Git merge conflict issues:
Git itself does not cause errors. Merge conflicts simply reflect issues in the team’s collaboration process (such as insufficient code review, merge strategies, etc.). Errors arising from Git merges are problems in the development workflow and code management, not something that should be addressed by a linter. Even when a merge conflict leads to a redefined function, the correct approach is to resolve it through code review, communication, and clear team conventions, not by letting a public linter enforce it.
Why it is inappropriate for a public linter to solve private issues:
Impact on development flexibility:
A public linter should follow general programming standards and should not interfere too much with specific project design decisions. For features like polymorphism, which are common in object-oriented programming, an overly strict check on function redefinition will limit the flexibility of developers.
Over-generalization of checks:
Applying a universal rule for specific situations, like Git merge conflicts, feels like an "over-generalization." These rules are more suited to address specific problems rather than common issues in the broader development ecosystem. Different teams may have very different approaches to code management and review, so a single tool should not try to address the unique problems of all teams.
Code review and process issues:
Issues with merge conflicts and duplicate code are more related to team culture and the development process. Using automated tools to enforce these checks may seem like it helps catch potential errors, but it doesn't fundamentally solve the communication and coding standard issues within teams. The correct approach is to resolve such issues through team code reviews, clear Git merge strategies, and conventions, reducing the chances of these problems from arising in the first place.
LuaLS (the language server) is to do type inferencing and provide type hints / completions.
On top of that, it comes with a diagnostic framework / system, in which it can do a number of diagnostics: https://luals.github.io/wiki/diagnostics/
Now that each diagnostics can be turned on or off, if you think this diagnostic is useless, then you can just turn it off 😅
Like in my team's project, we turned off the followings:
// ignore follow warning as code base is not fully annotated yet"diagnostics.neededFileStatus": {
"need-check-nil": "None!",
"param-type-mismatch": "None!",
"assign-type-mismatch": "None!",
"unbalanced-assignments": "None!",
"cast-local-type": "None!",
"invisible": "None!"
},
This is clearly a developer’s mistake, not an issue with the language or the linter itself
I agree with you 👍 that every mistakes are made by human, not the language or the linter tool.
(given that the language and tools are almost bug free)
But hey, this is the reason why we need a linter.👀
It can catch possible bugs which are hard to catch by human eyes.
For example one of the luals diagnostics is undefined-field, this helped my team catch quite a number of bugs:
(during code review as well as existing bugs in codebase)
Even when a merge conflict leads to a redefined function, the correct approach is to resolve it through code review
Code reviewers are also human, they can certainly reduce the chance of a coding error being merged to upstream, but they cannot catch all coding errors 😂.
Linters are the same, they can only catch possible errors which they can detect, but not all other logic errors.
Why it is inappropriate for a public linter to solve private issues:
I don't quite understand this statement 😕 .
What do you mean by private issues?
Does coding mistake count as a private issue? Because any mistake is made by a single person?
and a linter should not enforce it as a rule.
LuaLS doesn't enforce duplicate-set-field check, it's just enabled by default.
You can always turn it off (along with other diagnostics that you think are not appropriate for you projects)
PS: I am not maintainer of this project, I am just a user and a contributor.
What I said cannot represent maintainers' opinions. 😄
And that LuaLS is awesome 😀
Thank you for that PR! Very useful, quick question, is it possible to do the same for non class members? I have global functions that will be called from c++ if they have been implemented, in a manner a bit like this (I tried different variants but to no avail:
---Called when an airport's status changes---@typefun(event: AirportStatusChangedEvent)OnAirportStatusChanged=function(event) end--- Called when a runway status changes--- Implement this function to be notified when a runway's status changes---@parameventRunwayStatusChangedEventOnRunwayStatusChanged=function(event) endOnAirportConfigurationsUpdated=function(event) <--- lost type for eventlogger:info("Airport configurations updated event received, configurations count: " ..#event.configurationCount)
endOnRunwayStatusChanged=function(event) <--- lost type for eventlogger:info("Runway status changed event received, runway: " ..event.runway..", status: " ..event.icao)
end
I event tried manually overriding the type in the function parameter to no avail
OnAirportConfigurationsUpdated=function(event--[[@as AirportConfigurationsUpdatedEvent]]) <-- lost typelocale2=event--[[@as AirportConfigurationsUpdatedEvent]]<-- correct typelogger:info("Airport configurations updated event received, configurations count: " ..#event.configurationCount)
end
is it possible to do the same for non class members? I have global functions
@pierr3 Overriding global functions and auto infer their params seems not supported 🤔
My PR #2859 actually did not modify the infer logic too much:
I just used all type infos of a function to infer its param type, instead of only using its primary function type info
how a function gets it types is done by server's logic in other parts, which is out of my knowledge scope of this repo
even if it is supported, how to deal with duplicate-set-field warning will be another issue that needs to be solved, as mentioned in the Additional notes in that PR
ATM, you may need the "classic" way for defining a callback function type, and apply it manually in your actual callback function
Activity
C3pa commentedon Mar 14, 2024
We don't have a way to do this currently. You may be interested in writing a short plugin that would do that for you.
Luke100000 commentedon Mar 15, 2024
I looked into plugins and I can e.g. replace
---@overridewith a---@see parent.method, but I don't know how I can retrieve the parents function definition directly.C3pa commentedon Mar 15, 2024
Oh, looks like the Wiki docs aren't up to date anymore. There is more info on plugins on the project's GitHub pages: https://luals.github.io/wiki/plugins/. I can't really help you from here, I haven't used plugin api.
Luke100000 commentedon Mar 17, 2024
Alright thank you for the links, I made a plugin which injects
---@aliasat definitions, and---@typeand---@seeat overrides. Only param/return comments disappear but otherwise seems to be working fine for me.However I struggle to understand another concept, maybe you can help me here :)
Why does it want to inject a new field into B in the B's init? How can I specify that its fine to inject when its already inherited?
Another viewpoint would be: I don't want inheritance, but composition. Right now I have
A.oxANDB.ox, but in my case it doesn't matter who introduced that value, its now justox.C3pa commentedon Mar 17, 2024
In my opinion, inject field diagnostic isn't smart enough yet and has its false positives. I suggest opening an issue or looking if an issue about this was already reported.
NicameOurister commentedon Aug 31, 2024
同求相关支持。
使用场景:
项目中有很多角色脚本通过外部的通知进行驱动(击中\玩家输入之类),每个脚本针对相同通知的响应逻辑都不同,因此创建了一个父类用来定义一共有多少个通知响应接口以及每个接口的参数类型,具体的实现就交由各个脚本。
现有问题:
角色重写父类函数时会识别为一个在局部新建的函数,并默认将所有参数类型全部设置为 any,导致父类函数定义的参数类型在重写的函数体内全部丢失。
这里写定义的时候还是可以正确识别的。
进入函数体内之后参数类型就丢失了。
期待效果:
希望子类在重写父类函数时,除非子类手动重新指定函数的参数类型或者增加新的参数,否则子类函数直接采用父类函数定义的参数类型。
NicameOurister commentedon Aug 31, 2024
Could you please consider sharing your plug-in? I'm stuck with then same issue.
Appologise if you've already done publishing it. I've tried and failed to find it.
Luke100000 commentedon Aug 31, 2024
Here: https://pastebin.com/fmWLcjjw
It works for me but I'm certain that there are edge cases which breaks it.
Annotate overridden functions with
---@overrideand it will output something like this:NicameOurister commentedon Aug 31, 2024
Thanks so much!
NicameOurister commentedon Sep 2, 2024
已经有相同的issue了
#2158
Luke100000 commentedon Sep 8, 2024
Partially implemented in 7c481f5 now, arguments are inferred.
NicameOurister commentedon Sep 11, 2024
Thanks for the work and the notification, but there's still some distance to what I need.
In my use case, I have lots of objs which are only generated at runtime but customized at coding stage. They're still not getting any auto completion inside their overridden function bodies.
For example, I need to define the logic of diferent character skills, but skill information is only fetched at runtime and it's the same with skill objs. Thus, the code will end up like this:

To make it more complicated, the definition of skill, generate_skill_objs, list, list.skill1.onBegin all belongs to different files.
Could someone please look into this? Can't be more grateful.
tomlau10 commentedon Sep 11, 2024
If I understand the recent change correctly, it's for child class overriding parent class's method.
In the code you provided, the
lististable<string, skill>, which meanslist.skill1is stillskillclass (not a child class). Therefore luals will think you are redefining theskill:onBegin()in theskillclass, so it will instead throw aduplicate-set-fieldwarning.In order to make use of the recent change, you have to create new class for each of your skill, and extending the base skill class, like the following
It can be interpreted as
skill1is implementing theskillinterface 🤔But as you can see, this requires an extra
@classannotation for each of your skill,and you said the definitions belong to different files which makes things more complicated, so it may not suit you need... 😇 @NicameOuristeredit:
Seems it doesn't matter if all the definitions are in different files 🤔 Because right after using:
list.skill1and cast it to the newskills1typelocal skill1class variable, while still getting the parent method's params' typeNicameOurister commentedon Sep 11, 2024
Thanks for comment.
The recent change does cover part of my need, but not all of it. I know that I can do it this way. but there's still reason for the need of a more powerful implement.
In my case, there are tens of skill objs in every character's script, and the number of character script is more. It will be considerable lines of redundant code and makes it easier to cause name conflict.
Thus, I'm still hoping that we can just skip all those repeat annotations. Hopefully this enhancement could solve all the same related issues in #477, #1779 and #1757.
tomlau10 commentedon Sep 11, 2024
btw your link for
#1779and#1757are incorrect, they all points to#477🙈Actually you can just write
#1779or paste the linkhttps://github.com/LuaLS/lua-language-server/issues/1779when you write comment, and github will be smart enough to turns it into[#1779](https://github.com/LuaLS/lua-language-server/issues/1779), which can be verified inPreviewtab. No need to write the url markdown in full form 😄NicameOurister commentedon Sep 11, 2024
Thank you!
tomlau10 commentedon Sep 11, 2024
Strange, if I define the interface using‼️ 🤔
@type fun()style, then when overriding it, params' types are there(although it will still trigger
duplicate-set-fieldwarnings)tomlau10 commentedon Sep 11, 2024
I think I have found a way!! 😄 @NicameOurister
@fieldfirst, then all the methods will have params type infer && no duplicate-set-field warningsfun()syntax do not support multi line)I'm still trying to find out why
@paramstyle won't work 🤔NicameOurister commentedon Sep 12, 2024
Thanks for the solution!
It works as needed and does really solved the biggest problem, but it produces some little new problems.
The most obvious problem is that the ---field definition will mask out the real code and becomes the only target of that function's definition jump.
I can totally accept those extra ---field definitions . But I'm still kind of bothered with the definition jump mask problem. After all, the whole purpose of mine is to make my code library as accessible as possible for others.
I'm going to settle with the current solution while keep looking for a better way. Thanks again for the help.
tomlau10 commentedon Sep 14, 2024
I think I have successfully made it working without breaking anything 🎉 (at least all existing tests passed).
Root cause
I believe the function param infer logic resides in here:
lua-language-server/script/vm/compiler.lua
Lines 1110 to 1124 in b9639f6
@type fun()works but not@param, is because the former one is ofdoc.type.functiontype. Here the logic will only consider the arguments' node types in adoc.type.function(thefun()annotation)ifconditions in line 1124 to includefunctiontype, then it works!But with some side effects and breaking the tests... 🙁
Fixing side effects
functioninfer type. And this causes problems when trying to infer its own params' type by its own definition => loop => causing the types to beunknowninstead ofanysource:n.args[aindex] ~= sourceadded here => then all existing tests passed 🙂Fixing duplicate set field
We still have to deal with the
duplicate-set-fieldwarning. The related logic is here:lua-language-server/script/core/diagnostics/duplicate-set-field.lua
Lines 54 to 70 in b9639f6
So we have to find a way to skip the warning of this overriding class function / method use case.
The one that I can think of is that:
defin the above logic) is defined within aclassnamespace, we should allow overriding it (srcin the above logic) in anon-classnamespace.---@class A; local A = {}, hereAhas aclassattached. If there are duplicatedfunction A:f() end; function A:f() end=> should throw warnings
---@type A; local a = {}, hereahas noclassattached. Then we should allowfunction a:f() endfor overridingConclusion
With the above proposed change, now we can have param type inference in your original annotation style, and without
duplicate-set-fieldwarnings 👀I will need some time to do a more in-depth test before I create a PR. Meanwhile you may try to patch this in your local copy of luals to see the actual effect 😄@NicameOurister
NicameOurister commentedon Sep 14, 2024
Roger that. Thanks for all these work.
tomlau10 commentedon Sep 25, 2024
My related PR is merged recently, so this use case: #2569 (comment) will be supported in the next release 👀
jakitliang commentedon Jan 11, 2025
I think it is no need to check
duplicate-set-fieldsince Lua is weak typed and dynamic Language.If you wanna static features why not enjoy such like C# and TypeScript.
So I advice that it should remove the
duplicate-set-fieldsince Lua doesn't own that language level concept.tomlau10 commentedon Jan 11, 2025
duplicate-set-fielddiagnostic is useful in the sense that, in sometime we just accidentally used the same function name for a completely unrelated logic, instead of intentionally to do so.Or maybe that after solving git merge conflict, two of the same named function exist in the same class
=> this usually hints a bug in the code, as in the same class, each method name should be unique.
For example:
:setName()are defined, this maybe due to wrong conflict resolution between 2 teammatesthis code makes no sense and leads to confusion: maybe when reading the codebase, one will think that
:setName()will set to the fieldself.name, but actually now it will set toself.playerNameinstead.This had helped me successfully found a bug in my team's project🤦♂️ where the method wrongly defined using another namespace (instead of the namespace used in that file)
=> this caused
duplicate-set-fieldin that other namespacejakitliang commentedon Jan 11, 2025
If you mean "we just accidentally used the same function name for completely unrelated logic, instead of doing so intentionally," and think that’s important for Lua, then why don’t other linters, like Robocop and PEP8 linters, focus on this issue?
In my opinion, this kind of approach is only useful for specific scenarios, perhaps for beginners or for your own company’s logic, rather than being a common Lua practice.
It feels like gilding the lily.
And here are some additional comments:
Accidental function redefinition:
This is clearly a developer’s mistake, not an issue with the language or the linter itself. If a team encounters such issues, it should be a problem with the team’s code management, code review process, and development standards, not a universal code style issue. In many cases, redefining functions is a team-level management issue, and a linter should not enforce it as a rule.
Git merge conflict issues:
Git itself does not cause errors. Merge conflicts simply reflect issues in the team’s collaboration process (such as insufficient code review, merge strategies, etc.). Errors arising from Git merges are problems in the development workflow and code management, not something that should be addressed by a linter. Even when a merge conflict leads to a redefined function, the correct approach is to resolve it through code review, communication, and clear team conventions, not by letting a public linter enforce it.
Why it is inappropriate for a public linter to solve private issues:
Impact on development flexibility:
A public linter should follow general programming standards and should not interfere too much with specific project design decisions. For features like polymorphism, which are common in object-oriented programming, an overly strict check on function redefinition will limit the flexibility of developers.
Over-generalization of checks:
Applying a universal rule for specific situations, like Git merge conflicts, feels like an "over-generalization." These rules are more suited to address specific problems rather than common issues in the broader development ecosystem. Different teams may have very different approaches to code management and review, so a single tool should not try to address the unique problems of all teams.
Code review and process issues:
Issues with merge conflicts and duplicate code are more related to team culture and the development process. Using automated tools to enforce these checks may seem like it helps catch potential errors, but it doesn't fundamentally solve the communication and coding standard issues within teams. The correct approach is to resolve such issues through team code reviews, clear Git merge strategies, and conventions, reducing the chances of these problems from arising in the first place.
tomlau10 commentedon Jan 11, 2025
LuaLS (the language server) is to do type inferencing and provide type hints / completions.
On top of that, it comes with a
diagnosticframework / system, in which it can do a number of diagnostics: https://luals.github.io/wiki/diagnostics/Now that each diagnostics can be turned on or off, if you think this diagnostic is useless, then you can just turn it off 😅
Like in my team's project, we turned off the followings:
I agree with you 👍 that every mistakes are made by human, not the language or the linter tool.
(given that the language and tools are almost bug free)
But hey, this is the reason why we need a linter.👀
It can catch possible bugs which are hard to catch by human eyes.
For example one of the luals diagnostics is
undefined-field, this helped my team catch quite a number of bugs:(during code review as well as existing bugs in codebase)
Code reviewers are also human, they can certainly reduce the chance of a coding error being merged to upstream, but they cannot catch all coding errors 😂.
Linters are the same, they can only catch possible errors which they can detect, but not all other logic errors.
I don't quite understand this statement 😕 .
What do you mean by private issues?
Does coding mistake count as a private issue? Because any mistake is made by a single person?
LuaLS doesn't enforce
duplicate-set-fieldcheck, it's just enabled by default.You can always turn it off (along with other diagnostics that you think are not appropriate for you projects)
PS: I am not maintainer of this project, I am just a user and a contributor.
What I said cannot represent maintainers' opinions. 😄
And that LuaLS is awesome 😀
jakitliang commentedon Jan 11, 2025
Okay, good! Thats alright 👍
Robocop and Pep8 have millions of contributors and a large community that LuaLS can't match. I just give my opinion and hope it
goes stronger then.
Thank you for your recognition ❤.
pierr3 commentedon Sep 8, 2025
Thank you for that PR! Very useful, quick question, is it possible to do the same for non class members? I have global functions that will be called from c++ if they have been implemented, in a manner a bit like this (I tried different variants but to no avail:
I event tried manually overriding the type in the function parameter to no avail
tomlau10 commentedon Sep 10, 2025
@pierr3 Overriding global functions and auto infer their params seems not supported 🤔
My PR #2859 actually did not modify the infer logic too much:
ATM, you may need the "classic" way for defining a callback function type, and apply it manually in your actual callback function
---@param event RunwayStatusChangedEventon your function 😂