luau-lsp icon indicating copy to clipboard operation
luau-lsp copied to clipboard

GetAttribute should return "unknown?", not "any"

Open nightcycle opened this issue 9 months ago • 0 comments

Hello! I've been working with attributes a lot recently, and occasionally I'll get into some trouble when I misconfigure an instance. Since GetAttribute returns an "any" instead of an "unknown?", sometimes I'll forget to set up the proper handling for these kind of situations - typically leading to weird errors at later points in the code.

Let's assume I have an attribute called "Id" which I'm supposed to store as a number but occasionally forget and store as a string, and soemtimes forget to add at all.

Current behavior:

local idArray: {number} = {}

function addInst(inst: Instance)
	table.insert(idArray, inst:GetAttribute("Id")) --returns an any with no issues raised, even though it could be null / string
end

Ideally, the above should be flagged as an error - something that can only be worked around through refining the type. For example:

local idArray: {number} = {}

function addInst(inst: Instance)
	local id = inst:GetAttribute("Id") -- returns an unknown?
	if type(id) == "number" then
		table.insert(idArray, id)
	else
		error(`inst "(inst:GetFullName())" has a bad Id attribute value`) 	
	end
end

As attributes are pretty widely used, I think there might be a lot of hidden bad-type handling as a result. Adding this change would in my opinion lead to a safer overall dev ecosystem + prevent the naturally occuring mistakes that type checkers are meant to protect from.

Thank you!

nightcycle avatar May 30 '24 16:05 nightcycle