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

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

Closed
nightcycle opened this issue May 30, 2024 · 0 comments · Fixed by #646
Closed

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

nightcycle opened this issue May 30, 2024 · 0 comments · Fixed by #646
Labels
enhancement New feature or request

Comments

@nightcycle
Copy link

nightcycle commented May 30, 2024

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!

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
Development

Successfully merging a pull request may close this issue.

2 participants