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

RemoteEvent / BindableEvent / RemoteFunction / BindableFunction should be "unknown?" instead of "any"

Open nightcycle opened this issue 4 months ago • 0 comments

As the firing of these Roblox instances have no way to enforce type safety, I feel it's cleanest to make developers validate the types that are received.

In this below example, no errors are detected - everything is fine. That player variable could be passed into another function and cause a really weird error somewhere far away from the event.

local bindableEvent: BindableEvent

bindableEvent.Event:Connect(function(player: Player)
  -- the player is actually a string, but the dev is expecting a player
end)

bindableEvent:Fire(Players.LocalPlayer.Name)

What I propose is that it's better for the code to accept this uncertainty, and refine it in the following code. For example:

local bindableEvent: BindableEvent

bindableEvent.Event:Connect(function(player: unknown?)
  assert(typeof(player) == "Instance", `player is not instance, got type "{typeof(player)}"`)
  assert(player:IsA("Player"), `player is not player, got class "{player.ClassName}"`)
  -- the type is now certainly a player, or has given the developer a clear error at the earliest point possible
end)

bindableEvent:Fire(Players.LocalPlayer.Name)

I feel this handling of it is safe and more accurate to what's actually happening in the code, similar to the improvement made to attributes earlier.

Thank you for your time!

nightcycle avatar Oct 20 '24 02:10 nightcycle