duplicate-index and duplicate-set-field change to disabled by default

[jakitliang]

In order to left and keep Lua as Lua.

1. Make the decorative features check duplicate-index and duplicate-set-field to be disabled by default.
2. It can be back to enable while the bug of scanning diagnostics being fixed.

The relative issue was talked in #3035 and #3039 (comment).

[lewis6991]

Information is a diagnostic just like warning.

[jakitliang]

Information is a diagnostic just like warning.

Okay, I change the severity back into warning.

[tomlau10]

I think you just don't understand how luals works...
You have to use @class if you are defining different class, to let luals know that, as I have explained very detailedly in some previous comments in #3035

I know that you don't agree with it and don't want to use @class for some reasons, but turning off this diagnostic will just silence the warning, and in luals's point of view, it is still having incorrect type information and will give incorrect completion result, and you will then think that is another issue

• because when duplicate-set-field is triggered, under the hood luals is thinking that, the 2 table values are the same
• => that's why it is complaining

---

In your very beginning example without any annotation in #3035
In the eyes of luals, I think it is effectively like this

local Object = {}

local Animal = Object
function Animal:Speak()
  print('Animal.Speak')
end

local Person = Animal
function Person:Speak() --- warning duplicate-set-field
  print('Person.Speak')
end

local p1 = Person
p1:Speak() --> (method) Animal:Speak(); wrong hover info !!!

I don't know what's your implementation of Extends(), but clearly when luals complains Duplicate field Speak, in the eyes of luals it is thinking that Animal and Person are pointing to same table value

• with your PR, certainly the duplicate-set-field warning is gone by default => OK ✅
• but then the Code completion and hover info for p1 will still be wrong ❌
  because luals thinks that the :Speak() method is setting into the same table namespace
  

And then I think you will open another issue reporting this ("incorrect" hover).
And when someone tell you to use @class, you will disagree again 🤦‍♂️ .
But I don't think there is another way to solve that, you just have to add some @class annotation to make type hint and completion work.

And once you added proper @class annotation (for Object / Animal / Person), then luals can know that they are of different table
=> will not trigger duplicate-set-* warnings

---

On the other hand, turning the diagnostic to be disabled by default will affect all users:

• maybe some users (like my team) is already relied on this feature
• if in some version it is turned off by default, there is a chance that not everyone will notice this
• then their lint check will stopped checking for this, until they figure out it is disabled and turn it on again ☹️

---

Please note that I am no maintainer, and I cannot represent maintainers's view.
If maintain accept this PR, then I don't have any other reasons to object it.

[jakitliang]

• with your PR, certainly the duplicate-set-field warning is gone by default => OK ✅
• but then the Code completion and hover info for p1 will still be wrong ❌
  because luals thinks that the :Speak() method is setting into the same table namespace

Sorry, I think you just don't understand how luals works... You have to use @class if you are defining different class, to let luals know that, as I have explained very detailedly in some previous comments in #3035

I know that you don't agree with it and don't want to use @class for some reasons, but turning off this diagnostic will just silence the warning, and in luals's point of view, it is still having incorrect type information and will give incorrect completion result, and you will then think that is another issue

• because when duplicate-set-field is triggered, under the hood luals is thinking that, the 2 table values are the same
• => that's why it is complaining

In your very beginning example without any annotation in #3035 In the eyes of luals, I think it is effectively like this

local Object = {}

local Animal = Object
function Animal:Speak()
  print('Animal.Speak')
end

local Person = Animal
function Person:Speak() --- warning duplicate-set-field
  print('Person.Speak')
end

local p1 = Person
p1:Speak() --> (method) Animal:Speak(); wrong hover info !!!

I don't know what's your implementation of Extends(), but clearly when luals complains Duplicate field Speak, in the eyes of luals it is thinking that Animal and Person are pointing to same table value

• with your PR, certainly the duplicate-set-field warning is gone by default => OK ✅
• but then the Code completion and hover info for p1 will still be wrong ❌
  because luals thinks that the :Speak() method is setting into the same table namespace
  

And then I think you will open another issue reporting this ("incorrect" hover). And when someone tell you to use @class, you will disagree again 🤦‍♂️ . But I don't think there is another way to solve that, you just have to add some @class annotation to make type hint and completion work.

And once you added proper @class annotation (for Object / Animal / Person), then luals can know that they are of different table => will not trigger duplicate-set-* warnings

On the other hand, turning the diagnostic to be disabled by default will affect all users:

• maybe some users (like my team) is already relied on this feature
• if in some version it is turned off by default, there is a chance that not everyone will notice this
• then their lint check will stopped checking for this, until they figure out it is disabled and turn it on again ☹️

Please note that I am no maintainer, and I cannot represent maintainers's view. If maintain accept this PR, then I don't have any other reasons to object it.

Please check out this kind of code:

--- @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

The LuaLS linter can't do correly scanning the case table not mark with a ---@class is a bug or defect.

I stand for disabling it defaultly and it can rollbacks to enable when LuaLS AST scanner become stronger and solve the defect of this case in the future.

Enforcing rules is kinda the point of a linter innit. We should stop it if linter works incorrect or having defects..

[CppCXY]

The main reason for these issues is that luals lacks a way to treat the variable that holds the return value as a new anonymous type that automatically inherits from the return type. This results in the need to write many classes, which is quite reasonable and common in Lua itself.

These two diagnostics themselves are also fine; it is indeed unreasonable to repeatedly declare in the same class.

In summary, the Lua LS type and inference system are not mature enough, and the lint does not match the current inference capabilities, which are overly advanced.

[jakitliang]

The main reason for these issues is that luals lacks a way to treat the variable that holds the return value as a new anonymous type that automatically inherits from the return type. This results in the need to write many classes, which is quite reasonable and common in Lua itself.

These two diagnostics themselves are also fine; it is indeed unreasonable to repeatedly declare in the same class.

In summary, the Lua LS type and inference system are not mature enough, and the lint does not match the current inference capabilities, which are overly advanced.

This is reason and truth I wanna tell. Thanks to @CppCXY!!!

[sumneko]

我不同意将其默认禁用，理由如下：

1. 当它发出假警报时，是个非常显著的表现，并且很容易禁用该诊断。
2. 根据我的实践经验，很多令人恼火、需要花费很多精力去解决的问题正是这种“拼写错误”、“不小心覆盖了字段”、“倒数循环忘了写负数步长”之类的“低级错误”（由其是Lua经常需要运行在难以调试的环境下）。

因此我认为让用户自己选择“TRUST ME”比什么都不做要更好。

[Issues-translate-bot]

Sumneko Lua translate bot

---

I don't agree with it being disabled by default for the following reasons:

1. When it gives a false alarm, it's very noticeable and it's easy to disable the diagnostic.
2. According to my practical experience, many annoying problems that require a lot of effort to solve are just this kind of "spelling errors", "accidentally overwriting fields", "forgetting to write a negative step size in the countdown loop" and so on "Low-level errors" (because Lua often needs to run in an environment that is difficult to debug).

So I think it's better to let users choose "TRUST ME" than to do nothing.

[jakitliang]

我不同意将其默认禁用，理由如下：

1. 当它发出假警报时，是个非常显著的表现，并且很容易禁用该诊断。
2. 根据我的实践经验，很多令人恼火、需要花费很多精力去解决的问题正是这种“拼写错误”、“不小心覆盖了字段”、“倒数循环忘了写负数步长”之类的“低级错误”（由其是Lua经常需要运行在难以调试的环境下）。

因此我认为让用户自己选择“TRUST ME”比什么都不做要更好。

行，那等你的 bugfix，我这边先关闭了

[Issues-translate-bot]

Sumneko Lua translate bot

---

I don't agree with it being disabled by default for the following reasons:

1. When it gives a false alarm, it's very noticeable, and it's easy to disable the diagnostic.
2. According to my practical experience, many annoying problems that require a lot of effort to solve are just such "spelling errors", "accidentally overwriting fields", and "forgetting to write a negative step size in the countdown loop" "Low-level errors" of classes (because Lua often needs to run in an environment that is difficult to debug).

So I think letting users choose "TRUST ME" themselves is better than doing nothing.

Okay, let me wait for your bugfix. I will close it here first.