Luau Type Checking Release

DataBrain · November 23, 2020, 10:57pm

Another small bug report:

I have a table typed as {[Player]: BasePart?}, since there both can and can not be a BasePart at an arbitrary Player-typed key. Unfortunately, when iterating over this record with pairs, it also sets the value to a type that is nilable, even though it’s impossible to have a nil value with a non-nil key on a table.

This is in a heartbeat-bound polling loop, so even though it’s not too big of a deal to check if proxy then, it’s still a needless truthiness check that I have to place here that affects runtime performance because it thinks proxy can be nil here.

Dekkonot · November 24, 2020, 6:02am

One major grievance I have is the method for importing external types. I have a module right now that requires a module and assigns it to a variable for the sole purpose of using it for type annotations (you can see the type module here). This is annoying, since it has runtime consequences and is a bit unwieldy.

Would it be possible for us to get an import keyword or something like it so that we can import the types from another module/script without having to require it directly?

nooneisback · November 24, 2020, 1:17pm

Currently converting my 3D skybox module to use types, but forgot to annotate them on the functions themselves. Got this messed up error which left me confused for a while:

W000: (531,4) Type '({+ AmbientColorDay: Color3, AmbientColorNight: Color3, ... 3 more ... +}, Color3) -> ()' could not be converted into 't1 where Skybox3D = {| AmbientColorDay: Color3, AmbientColorNight: Color3, ... 19 more ... |} ; t1 = (Skybox3D, Color3) -> ()'

Here are the code snippets that caused this:

type Skybox3D = {
	...
	SetViewportFrameZIndex:(Skybox3D,number)->(),
	...

	SetWorldScale:(Skybox3D,number)->(),
	SetCameraOrigin:(Skybox3D,Vector3)->(),

	SetLightSource:(Skybox3D,string)->(),

	SetAmbientColorDay:(Skybox3D,Color3)->(),
	SetAmbientColorNight:(Skybox3D,Color3)->(),
	SetLightColorDay:(Skybox3D,Color3)->(),
	SetLightColorNight:(Skybox3D,Color3)->(),

	SetActive:(Skybox3D,boolean)->(),
	Destroy:(Skybox3D)->(),
}

...

local skySetWorldScale = function(self,scale)
	...
end
	
local skySetCameraOrigin = function(self,ori)
	...
end
	
local skySetViewportFrameZIndex = function(self,zindex)
	...
end

local skySetLightSource = function(self,source)
	...
end

local skySetAmbientColorDay = function(self,ambientday)
	...
end

local skySetAmbientColorNight = function(self,ambientnight)
	...
end

local skySetLightColorDay = function(self,lightday)
	...
end

local skySetLightColorNight = function(self,lightnight)
	...
end


local skySetActive = function(self,active)
	...
end

local skyDestroy = function(self)
	...
end

devSparkle · November 24, 2020, 11:18pm

I’m having an interesting issue. As you can probably guess, should the type be nil then number 1 will be used for the assignment.

Dekkonot · November 25, 2020, 1:15am

I have no idea how to solve this problem but can we just not have this warning? It seems like it’s going to rarely if ever be useful.

DataBrain · November 25, 2020, 3:39am

It seems types alias statements have some amount of scope to them, so it’s almost always better to feed types annotations forwards. The exception is that you can circularly reference types within another type alias.

I’m not sure if this is intentional behavior for the type system or a bug to be fixed, but I’ve been using the following (extremely verbose, but type safe to external users of the module) boilerplate for typed OOP code:

OOP Boilerplate

--!strict



type ClassName_Members = {
	
}
type ClassName_Methods = {
	
}
type ClassName_Metatable = { __index: ClassName_Methods }

export type ClassName = typeof((function()
	local t: ClassName_Members
	local u: ClassName_Metatable
	return setmetatable(t, u)
end)())

local ClassName = {}

local ClassName_Methods: ClassName_Methods = {
	
}

local ClassName_Metatable: ClassName_Metatable = {
	__index = ClassName_Methods
}

function ClassName.new(): ClassName
	local self = {}
	setmetatable(self, ClassName_Metatable)
	
	-- Name self type to make output less verbose
	type InferredSelf = typeof(self)
	
	return self
end

return ClassName

You can ctrl+f and replace the text ‘ClassName’ with your class’s name
It’s super redundant and verbose, but it does get the job done.

As an example of this in action, I’m currently writing a custom character framework that forks the core “PlayerModule” script with a typesafe style of OOP:

DataBrain · November 25, 2020, 4:17am

@zeuxcg
Found a small typing issue with the Connect method of RBXScriptSignal

--!strict
local function subscribe(signal: RBXScriptSignal): RBXScriptConnection
	return signal:Connect(function() end)
end

Xan_TheDragon · November 26, 2020, 8:09pm

Here’s some incorrect code path parsing.

Dekkonot · November 26, 2020, 11:21pm

That seems to work, though I’ll be the first one to say “wow I hate this”.

It seems like in order to make it properly typesafe you only have to add types to the metatable though, which seems… more reasonable. Not quite good though.

Thank you for the tip about making internal types though. That helps with debugging a lot.

DataBrain · November 27, 2020, 5:53pm

@zeuxcg @Apakovtac Here’s another bug related to unions and type inference:

The following code will produce an erroneous warning that a field is missing in my table, even though I have it specified in my type annotation that this field is optional.

Code:

--!strict

export type GroupRankInfo = {
	ShortName: string,
	LongName: string,
	PrimaryColor: Color3,
	InsigniaIcon: string,
	Description: string,
	RankUpXP: number?,
}

local RANK_DATA: {[number]: GroupRankInfo} = {
	[1] = {
		ShortName = 'CIT',
		LongName = 'Citizen',
		PrimaryColor = Color3.fromRGB(0, 255, 255),
		InsigniaIcon = '',
		Description =
[[Rank Description Goes Here]],
		RankUpXP = 2000,
	},
	[2] = {
		ShortName = 'PVT',
		LongName = 'Private',
		PrimaryColor = Color3.fromRGB(60, 60, 255),
		InsigniaIcon = '',
		Description =
[[Rank Description Goes Here]],
		RankUpXP = 5000,
	},
	[3] = {
		ShortName = 'REC',
		LongName = 'Recruit',
		PrimaryColor = Color3.fromRGB(60, 60, 255),
		InsigniaIcon = '',
		Description =
[[Rank Description Goes Here]],
		RankUpXP = nil,
	},
}

What seems to be happening here, is that the type checker is inferring the type of the table within the constructor expression, and then trying to coerce this inferred type onto my annotated type, instead of the other way around.

If I change this type to something that uses unions in general, it will still try to coerce an inferred type onto my annotated type:

Simplified repro

--!strict

export type MyType = {
	foo: number | string,
}

local MY_OBJECT: {[number]: MyType} = {
	[5] = {
		foo = 'fighters',
	},
	[3] = {
		foo = 1,
	},
	[5] = {
		foo = 2,
	},
}

This only seems to happen when I use non-string values as keys; if I use strings as keys, I don’t seem to get the bug:

Doesn't show any warnings:

--!strict

export type MyType = {
	foo: number | string,
}

local MY_OBJECT: {[string]: MyType} = {
	baz = {
		foo = 'fighters',
	},
	qux = {
		foo = 1,
	},
	quux = {
		foo = 2,
	},
}

imKirda · November 28, 2020, 10:34am

I found 6 more issues

As Code

--!strict

-- 1

local Players = game:GetService('Players')

Players:cat()

-- 2

local Label: TextLabel = Instance.new('TextLabel')

local Clone: TextLabel = Label:Clone()

-- 3

local Caller: Script = getfenv(2).script

local Values: {[string]: ObjectValue} = {}

for _, Value in ipairs( Caller:GetChildren() ) do

if Value:IsA('ObjectValue') then

local Name: string = Value.Name

Values[Name] = Value

end

end

-- 4

loadstring( [[print('Hello')]] )()

-- 5

local A = 'string'

A = nil

-- 6

coroutine.wrap(function(a: number, b: boolean)

end)('string', 'hello')

In 1 issue i Dekkomot said that services are typed automatically whereas here it does not even on the strict mode.
The 2 issue Clone() does not return type of instance that has been cloned but return type Instance
In the 3 issue when using getfenv(2).script which in my case is script calling the function and then check if it’s children is ObjectValue still warns after checking it.
4 issue is that loadstring does not return function even if it does
At 5 issue you can’t set variable to nil… if you can’t do it like that then how?
6 issue is bit similar to 4 issue, i am giving strings when calling the coroutine but it only accepts numbers - there is no warnings

sleitnick · November 29, 2020, 4:39am

I think this has been addressed already, but dealing with nil cases is incredibly difficult and nearly impossible. Here’s a simple example, spliced from my A* implementation:

type Node = {
	Position = Vector3;
	Parent: Node?;
}

-- ... do A* stuff

-- Backtrace path:
if (pathFound) then
	local node = goalNode
	while (node) do
		table.insert(path, 1, node.Position)
		node = node.Parent -- W000: Type 't1 where Node = {...stuff} ; t1 = Node | nil' could not be converted into 'Node'
	end
end

Ok, so the logical thing would be to add an if check to see if it’s nil, however I get the same warning:

...
	local node = goalNode
	while (node) do
		table.insert(path, 1, node.Position)
		if (node.Parent) then
			node = node.Parent -- W000: Type 't1 where Node = {...stuff} ; t1 = Node | nil' could not be converted into 'Node'
		end
	end
...

Wunder_Wulfe · November 29, 2020, 10:52pm

funny
also yes, the error doesnt go away if i did this instead:

signal = RunService:IsServer() and RunService.Stepped or RunService.RenderStepped

On the bright side, I found a temporary solution for generics.

type Pointer<T> = {Value: T};
local Pointer = {
	new = function(value: any): Pointer<any>
		return {Value = value};
	end
};

local myNum: Pointer<number> = Pointer.new(5.5)

but since the removal of @metatable / __metatable I couldnt implement automatic getvalue()
also an interesting behavior when indexing objects

nezuo · November 30, 2020, 7:46pm

Is there a way to define a type like that includes everything but nil?

Wunder_Wulfe · November 30, 2020, 9:13pm

oh ok

If I remove the return type on AddChild it works just fine, so I guess it has something to do with self

fun_enthusiast · November 30, 2020, 11:42pm

You’re in luck!

When you use typeof(expr) in type context like this, the expression is never actually evaluated. Luau just inspects it to figure out type it would be.

You never pay any runtime cost when you do this.

zeuxcg · December 1, 2020, 3:36am

For people who are posting bug reports in this thread, please keep doing this but pleeease share the actual code we can copy & paste instead of just the screenshot, as it will make reproducing the issue that much easier.

zeuxcg · December 1, 2020, 4:36am

Thanks - handling null checks better is on our list for the next set of improvements to the type checker. In this case what should’ve also worked is typing node as Node? explicitly, but it looks like while loops don’t enforce the non-nullability contracts either atm. All of this will be fixed

zeuxcg · December 1, 2020, 5:14pm

Thanks - btw note that you can say {MyType} instead of {[number]: MyType} now.

DataBrain · December 1, 2020, 5:26pm

Is that recommended for record types, and not just array types? I know the current type system holds no distinction between the two, but to me {[number]: MyType} better describes what the type is. This is a map from group rank (nonsequential numbers) to rank data, even though the simplified example I gave had sequential indices.