luau icon indicating copy to clipboard operation
luau copied to clipboard

Writing common property to a type union errors

Open JohnnyMorganz opened this issue 3 years ago • 3 comments

If you create a type union (e.g., through a type predicate) and then attempt to assign a property which is common to all types within the union, it fails with a type error. Example:

--!strict
local inst = Instance.new("SpotLight")

if inst:IsA("SpotLight") then
	inst.Enabled = true -- OK
end

if
	inst:IsA("SpotLight")
	or inst:IsA("SurfaceLight")
	or inst:IsA("BillboardGui")
	or inst:IsA("Beam")
then
	inst.Enabled = true -- TypeError: Expected type table, got 'Beam | BillboardGui | SpotLight | SurfaceLight' instead
end

JohnnyMorganz avatar Jun 10 '22 15:06 JohnnyMorganz

This also fails for type intersections, potentially related to the same issue, but may be separate?

type Foo = {
	Bar: string,
} & { Baz: number }

local x: Foo = { Bar = "1", Baz = 2 }
local y = x["Bar"] -- TypeError: Expected type table, got '{| Bar: string |} & {| Baz: number |}' instead

JohnnyMorganz avatar Oct 26 '22 09:10 JohnnyMorganz

I wanted to see if it was easy enough to patch in the current type checker. It seems the getIndexTypeFromType function already reduces a union of tables into its common properties. So I thought maybe we can just use that when the LValue is a Union type (i.e., just add get<UnionType>(lhs) to the following condition: https://github.com/Roblox/luau/blob/76bea81a7b798b3324f97a72a2a8faac29089eec/Analysis/src/TypeInfer.cpp#L3348-L3359

This does indeed work and fix the issue. However, it causes the "tagged_unions_immutable_tag" test case to (quite rightly) fail: https://github.com/Roblox/luau/blob/76bea81a7b798b3324f97a72a2a8faac29089eec/tests/TypeInfer.singletons.test.cpp#L210-L221

Now, this seems a bit trickier to resolve, and I am guessing where the meat of the issue lies.

In real-world code, I mostly see this error when using ClassTypes (typically Roblox Instances), which, if I am not mistaken, don't have any "tagged union" effect. Would it be feasible to half-fix this only for a union of ClassTypes? That would resolve most of the false positives I see

JohnnyMorganz avatar Jul 03 '23 14:07 JohnnyMorganz

This also fails for type intersections, potentially related to the same issue, but may be separate?

I want to note that this is fixed in the new typechecker, hope that it will be ready soon. Although there are still some issues in original example (it works as is, but there are hidden refinement issues).

vegorov-rbx avatar Jul 05 '23 10:07 vegorov-rbx