LuaLS / lua-language-server

A language server that offers Lua language support - programmed in Lua
https://luals.github.io
MIT License
3.35k stars 318 forks source link

Attributes are not automaticly recognized #573

Closed sewbacca closed 3 years ago

sewbacca commented 3 years ago

Describe the bug Sometimes the language server claims that fields are undefinied, even if they are clearly defined

To Reproduce Steps to reproduce the behavior:

  1. Create new project with lua file
  2. Define an attribute in one function
  3. Use it in another
  4. See warning

Expected behavior No warning

Screenshots example.gif

Environment (please complete the following information):

Provide logs https://pastebin.com/HpnqW8Yb

sumneko commented 3 years ago

I'm not sure whether this feature should be supported, just think of the following code:

---@class car
local mt = {}
mt.seat = 0

function mt:addSeat()
    self.sead = self.sead + 1
end

Do I think this sead is a defined field?

In my imagination, if you mark the class, then you should explicitly define all fields:

---@class car
---@field windows any
local mt = {}
mt.seat = 0
mt.driver = nil

function mt:check()
    return self.seat, self.driver, self.windows
end
sewbacca commented 3 years ago

I was using this feature for a long time now and it served me well. In my eyes, Lua's object system is still dynamicly typed, so for me it makes sense to add fields on the fly. Normally when I define fields, I use a constructor called init (similair to pythons __init__) and define all fields there with default objects. If I cannot use this feature anymore, I would have to duplicate code for every attribute:

---@class Wheel : Super
local Wheel = class()

---@class Car : Super
local Car = class()

-- Duplicate fields here
---@type Wheel
mt.wheel = nil

function mt:init()
    self.wheel = Wheel:new()
end

Maybe it would be best to let the programmer decide which style to use. I would recommend a diagnostics field for allow-fuzzy-field-declaration, so it can be enabled or disabled on the fly with the @diagnostics annotation. Another option would be to declare a function as a constructor and only allow fuzzy field declaration there. Maybe this could be used to also solve the following problem:


local MyClass = { }

---@constructor
---@generic T
---@param self T
---@return T
function MyClass:new(...)
    local object = setmetatable({}, self)
    object:init(...)
    return object
end

---@constructor
function MyClass:init(speed)
    self.speed = speed
end

function MyClass:accel(speed)
   self.sppeed = self.sppeed + speed
end

local myObject = MyClass:new(12) -- No argument hint here

In this example init would be declared as a constructor, hence speed would be recognized inside init, but the misspell in accel would be warned, since here is no fuzzy field declaration allowed. The last line shows another problem with my goto object system. I use new for creating my object and init as the initializer, because in most cases thats what changes. However since new and init are different functions, there are no argument hints. This setup would also allow for a __call field to be recognized as a constructor if the programmer wishes to use that syntax.

MikuAuahDark commented 3 years ago

In my opinion, explicit is better than implicit, but unfortunately the new behavior breaks lots of scripts type annotation.

I use this plugin on work on and we suddenly noticed all of the fields gets undefined when we upgrade to v2.0. We rather spend our time completing the project rather than explicitly defining fields which can take days due to large codebase. For now I told everyone to downgrade just before v2.0 to get the old behavior.

TL;DR: Old behavior with option to set to new behavior would be nice, where new behavior is default.