Linter Should Work According to Language Specifications

[jakitliang]

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

--- @generic T
--- @param class T
--- @return T
local function New( class )
  return setmetatable({}, {__index = class})
end

local Animal = {}

function Animal:Speak()
  print('Animal:Speak')
end

local Person = New(Animal)

--- Lua Diagnostics: Duplicate field `Speak`
function Person:Speak()
  print('Person:Speak')
end

Person:Speak() -- print Person:Speak

local someone = New(Person)
someone:Speak() -- print Person:Speak

Actual Behaviour

--- @generic T
--- @param class T
--- @return T
local function New( class )
  return setmetatable({}, {__index = class})
end

local Animal = {}

function Animal:Speak()
  print('Animal:Speak')
end

local Person = New(Animal)

--- No Lua Diagnostics
function Person:Speak()
  print('Person:Speak')
end

Person:Speak() -- print Person:Speak

local someone = New(Person)
someone:Speak() -- print Person:Speak

Reproduction steps

1. Just copy the code and let lsp lint them

Additional Notes

My Points:

1. Polymorphism is a Core Feature of Object-Oriented Programming:
   Polymorphism is one of the core features of object-oriented programming. It allows different objects to use the same interface but exhibit different behaviors. It is widely used in many programming languages and is a key method for writing maintainable and extensible code.

2. Lua is a Dynamic Language:
   Lua is dynamically typed, meaning it does not need to determine types at compile time like static languages. This provides great flexibility for programming. Through metatables and mechanisms such as __index and __call, Lua can implement many traditional object-oriented features, like inheritance and polymorphism.

3. If Polymorphism is Opposed, Static Languages Should Be Chosen Instead of Using duplicate-set-field:
   If dynamic polymorphism is not acceptable, instead of enforcing the duplicate-set-field rule, a feature like @Final annotation should be introduced. Until such a feature is implemented, no changes should be made.
   If dynamic polymorphism is unacceptable, instead of adding duplicate-set-field, developers should consider other programming languages such as C#, Java, or C++, which are statically bound and support the final syntax for methods.

4. Performance Trade-offs in Scripting Languages:
   The design of scripting languages typically makes trade-offs between performance and flexibility. For example, Python's OOP is based on the dict mechanism, which allows dynamic modification of object behavior. Ruby, on the other hand, uses the Object, BasicObject, Class, and ancestor lookup chains to achieve flexibility, which sacrifices some performance.
   For most scripting languages, performance is not the sole design consideration. Development efficiency, maintainability, and extensibility are the key goals. Over-optimization for performance, especially by limiting flexibility, is contrary to the design intent of scripting languages.

Log File

No response

[tomlau10]

You have to add proper @class annotation first to make it work. 😂
LuaLS cannot directly recognize your Extend() / New() function.
Each class (and its inheritance relationship) must be defined explicitly using @class

function Extends(class)
    return setmetatable({}, { __index = class })
end

---@generic T
---@param class T
---@return T
function New(class)
    return setmetatable({}, { __index = class })
end

---@class Object
local Object = {}

---@class Animal: Object
local Animal = Extends(Object)  --> Animal: Animal
function Animal:Speak()
    print('Animal.Speak')
end

---@class Person: Animal
local Person = Extends(Animal)  --> Person: Person
function Person:Speak()
    print('Person.Speak')
end

local p1 = New(Person)  --> p1: Person
p1:Speak()  --> (method) Person:Speak()

[jakitliang]

You have to add proper @class annotation first to make it work. 😂 LuaLS cannot directly recognize your Extend() / New() function. Each class (and its inheritance relationship) must be defined explicitly using @class

function Extends(class)
    return setmetatable({}, { __index = class })
end

---@generic T
---@param class T
---@return T
function New(class)
    return setmetatable({}, { __index = class })
end

---@class Object
local Object = {}

---@class Animal: Object
local Animal = Extends(Object)  --> Animal: Animal
function Animal:Speak()
    print('Animal.Speak')
end

---@class Person: Animal
local Person = Extends(Animal)  --> Person: Person
function Person:Speak()
    print('Person.Speak')
end

local p1 = New(Person)  --> p1: Person
p1:Speak()  --> (method) Person:Speak()

I don't agree with adding @class label to make it work at the first.

Lua doesn't introduce class syntax for its language.

Dictionary or we can also call it Hashmap shouldn't having limitation for the override of key value.

Why LuaLS prevent people update the key and value.

[tomlau10]

I don't agree with adding @class label to make it work at the first.

AFAIK this is how LuaLS works... (by adding appropriate annotation)
Certainly LuaLS works to some extent in an unannotated codebase.
It will try its best to infer the types, but most of the time it will just return any for a given variable.

local a = 1 + 1 --> it can infer that `a` is `integer

-- but what do you expect luals to infer the following unannotated function?
local function add(a, b)
    return a + b
end
--[[
function add(a: any, b: any)
  -> unknown
]]

-- you have to add annotation to make luals become smarter
---@param a integer
---@param b integer
local function add2(a, b)
    return a + b
end
-- even without specifying `@return integer`
-- luals can already infer that the return type is `integer` now
-- because `integer + integer -> integer`
--[[
function add2(a: integer, b: integer)
  -> integer
]]

Lua doesn't introduce class syntax for its language.

Yes I agree, and there are numerous ways to emulate OOP in Lua as well.
But now we are talking about LuaCAT (https://github.com/LuaCATS), the annotation syntax used by LuaLS.
You just can't expect LuaLS to infer all the types correct, without you giving enough information to it 😳 ?
And those @class / annotations are there for LuaLS to read, not Lua.
(lua code logic doesn't depend on annotation / comment)

---

Dictionary or we can also call it Hashmap shouldn't having limitation for the override of key value.

LuaLS seems will only check duplicate-set-field on function values on the same table:

lua-language-server/script/core/diagnostics/duplicate-set-field.lua

Lines 56 to 67 in cb964c6

	if def.type ~= 'setfield'
	and def.type ~= 'setmethod'
	and def.type ~= 'setindex' then
	goto CONTINUE
	end
	local defTopBlock = getTopFunctionOfIf(def)
	if uri == guide.getUri(def) and myTopBlock ~= defTopBlock then
	goto CONTINUE
	end
	local defValue = vm.getObjectValue(def)
	if not defValue or defValue.type ~= 'function' then
	goto CONTINUE

Using my snippet in the previous comment, even if I remove all annotation, no duplicate-set-field is generated 😕

function Extends(class)
    return setmetatable({}, { __index = class })
end

function New(class)
    return setmetatable({}, { __index = class })
end

local Object = {}

local Animal = Extends(Object)  --> Animal: { Speak: function, }
function Animal:Speak()
    print('Animal.Speak')
end

local Person = Extends(Animal)  --> Person: { Speak: function, }
function Person:Speak()
    print('Person.Speak')
end

local p1 = New(Person)  --> p1: table
-- but luals can only infer `table` return type based on the code of `New()`
p1:Speak()  --> (field) table.Speak: unknown
-- since `p1` is just of `table` type, field `Speak` of a general table is `unknown`
-- luals have no knowledge to link the method and its definition
-- hence no completion can be provided

May I know what's your implementation in Extend() ?
If luals can know that Animal and Person are different table, then setting the same named field should not trigger a duplicate-set-field warning. 🤔

Why LuaLS prevent people update the key and value.

As a last resort, you can just disable duplicate-set-field diagnostics.

[jakitliang]

I don't agree with adding @class label to make it work at the first.

AFAIK this is how LuaLS works... (by adding appropriate annotation) Certainly LuaLS works to some extent in an unannotated codebase. It will try its best to infer the types, but most of the time it will just return any for a given variable.

local a = 1 + 1 --> it can infer that `a` is `integer

-- but what do you expect luals to infer the following unannotated function?
local function add(a, b)
    return a + b
end
--[[
function add(a: any, b: any)
  -> unknown
]]

-- you have to add annotation to make luals become smarter
---@param a integer
---@param b integer
local function add2(a, b)
    return a + b
end
-- even without specifying `@return integer`
-- luals can already infer that the return type is `integer` now
-- because `integer + integer -> integer`
--[[
function add2(a: integer, b: integer)
  -> integer
]]

Lua doesn't introduce class syntax for its language.

Yes I agree, and there are numerous ways to emulate OOP in Lua as well. But now we are talking about LuaCAT (https://github.com/LuaCATS), the annotation syntax used by LuaLS. You just can't expect LuaLS to infer all the types correct, without you giving enough information to it 😳 ? And those @class / annotations are there for LuaLS to read, not Lua. (lua code logic doesn't depend on annotation / comment)

Dictionary or we can also call it Hashmap shouldn't having limitation for the override of key value.

LuaLS seems will only check duplicate-set-field on function values on the same table:

lua-language-server/script/core/diagnostics/duplicate-set-field.lua

Lines 56 to 67 in cb964c6

	if def.type ~= 'setfield'
	and def.type ~= 'setmethod'
	and def.type ~= 'setindex' then
	goto CONTINUE
	end
	local defTopBlock = getTopFunctionOfIf(def)
	if uri == guide.getUri(def) and myTopBlock ~= defTopBlock then
	goto CONTINUE
	end
	local defValue = vm.getObjectValue(def)
	if not defValue or defValue.type ~= 'function' then
	goto CONTINUE

Using my snippet in the previous comment, even if I remove all annotation, no duplicate-set-field is generated 😕

function Extends(class)
    return setmetatable({}, { __index = class })
end

function New(class)
    return setmetatable({}, { __index = class })
end

local Object = {}

local Animal = Extends(Object)  --> Animal: { Speak: function, }
function Animal:Speak()
    print('Animal.Speak')
end

local Person = Extends(Animal)  --> Person: { Speak: function, }
function Person:Speak()
    print('Person.Speak')
end

local p1 = New(Person)  --> p1: table
-- but luals can only infer `table` return type based on the code of `New()`
p1:Speak()  --> (field) table.Speak: unknown
-- since `p1` is just of `table` type, field `Speak` of a general table is `unknown`
-- luals have no knowledge to link the method and its definition
-- hence no completion can be provided

May I know what's your implementation in Extend() ? If luals can know that Animal and Person are different table, then setting the same named field should not trigger a duplicate-set-field warning. 🤔

Why LuaLS prevent people update the key and value.

As a last resort, you can just disable duplicate-set-field diagnostics.

check duplicate-set-field on function values on the same table is serious problem, Hashmap is being design as K-V data structure. But linter prevent coders to setting the value.

If you want to give some case somebody in the situation of twice modification. Just give a Information instead of Diagnostics, Error level and it should be optional for the special case usage.

By the way, linters of any other languages never lint for monkey patches.

I don't agree with the opinon:

As a last resort, you can just disable duplicate-set-field diagnostics.

Since duplicate-set-field and duplicate-index should be disabled by default.

If users want to enjoy this feature in their situation, just turn on them manually.