Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for overriding functions #2158

Closed
evur opened this issue Jun 9, 2023 · 4 comments · Fixed by #2859
Closed

Add support for overriding functions #2158

evur opened this issue Jun 9, 2023 · 4 comments · Fixed by #2859
Labels
enhancement New feature or request

Comments

@evur
Copy link

evur commented Jun 9, 2023

The language server currently doesn't automatically infer types when overriding functions.

Example of the desired behavior:

---@class
FooClass = {
	---@return FooClass
	new = function()
		-- create an instance of FooClass
	end
	
	---@param foo number
	---@param bar string
	foo_callback = function(self, foo, bar) end
}

test = FooClass.new()

-- automatically infer types from FooClass:foo_callback
function test:foo_callback(foo, bar)
	-- foo is of type number
	-- bar is of type string
end
@evur
Copy link
Author

evur commented Jun 9, 2023

There are also other issues requesting this or even reporting it as a bug:
#1779
#1757
#1794

@PinewoodPip
Copy link

Worth noting that the type inference here is half-functional, as described here: #1801
Hovering over the function will show the correct types, but they will not work within the function itself.

@sumneko sumneko added the enhancement New feature or request label Aug 15, 2023
@ImpleLee
Copy link

#477

@mgmchenry
Copy link

Suggestion: Add this by default to every configuration file

{
    "Lua.diagnostics.disable": [
		"duplicate-set-field"
	]
}

Am I kidding? idk.
I've been sitting on this for a couple of weeks as I accumulate many dozens of lines that say

---@diagnostic disable-next-line: duplicate-set-field

The more of those I have, the more wrong it feels. All of my work arounds feel worse.
Sometimes the type of a field/member/whatever is a function, maybe with a specific signature. It's not sacred. It's a variable. Maybe just a value in a table.

I have 15 tabs open to pages discussing the issue. I've been reading them and browsing the LuaLS repos for hours now.
The most frustrating of which is this one:
#1801 (comment)

Maybe ---@override would be great.

I'm getting trauma flashbacks to luau's evolving type system with this issue. Getting the best of a flexible language like lua along with some of the comforts of a strongly typed language is hard. There were a lot of people looking at that and there was a lot of iteration. It's understandably very hard to get right.

There are languages and environments I would insist on having any callbacks run through a subscribe function.
I think (100% opinion based) that a lot of places where lua is used and can benefit from LuaLS type inference and annotation, it also benefits from avoiding a temptation to dive into OOP and polymorphism and instead just embrace more functional styles, data oriented approaches, maybe interfaces with annotation.

I don't know that an ---@override annotation is going to get it right if someone gets around to taking a crack at it after all these years.
I think as it's been the last few years, "duplicate set field" may be doing more harm than good and is better off disabled. Judging by the discourse, people are spending a lot of hours trying to work around it, changing the format of their code to try to appease the warnings, and I doubt anywhere near that amount of time has been saved by the warning working as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants