#nojira Strict mode (#103)

* Enable strict mode for most files, and integrate function generics where applicable.
* A few files are nonstrict and nocheck, with comments explaining the Luau gaps.
* Make some notes about things Luau analzye could do better, and manually annotate to work around.
* Consistently assert when utf8.len returns nil (meaning invalid utf8 sequence).
* Object.assign() can't be strongly typed because Luau currently can't do intersections of type packs.
* Fold in PR feedback, add note about Lluau analyze improvements

CHANGELOG.md

@@ -1,5 +1,10 @@
 # LuauPolyfills Changelog
 
+## Unreleased
+
+* Strict luau type mode enabled on almost all polyfill files and tests. This may highlight latent type issues in code that consumes the polyfills, but was tested against apollo-client-lua (fixed merged), roact-alignment (no fixes needed), and jest-roblox (no fixes needed).
+* Fix for `Array.from` to respect the `thisArg` argument when it is supplied.
+
 ## 0.2.6
 * add `has` method to `WeakMap`
 * add `expect` method to `Promise`

foreman.toml

@@ -1,5 +1,5 @@
 [tools]
 rotrieve = { source = "roblox/rotriever", version = "=0.5.0-rc.4" }
 rojo = { source = "rojo-rbx/rojo", version = "6.2.0" }
-selene = { source = "Kampfkarren/selene", version = "0.14" }
-stylua = { source = "JohnnyMorganz/StyLua", version = "0.10.1" }
+selene = { source = "Kampfkarren/selene", version = "0.15" }
+stylua = { source = "JohnnyMorganz/StyLua", version = "0.11.1" }

roblox-internal.toml

@@ -0,0 +1,6 @@
+[[table.freeze.args]]
+type = "table"
+
+[[table.isfrozen.args]]
+type = "table"
+

rotriever.toml

@@ -6,7 +6,7 @@ content_root = "src"
 files = ["*", "!**/__tests__/**"]
 
 [dev_dependencies]
-JestGlobals = "github.com/roblox/jest-roblox@2.1.2"
-TestEZ = "github.com/roblox/jest-roblox@2.0.1"
+JestGlobals = "github.com/roblox/jest-roblox@2.2.1"
+TestEZ = "github.com/roblox/jest-roblox@2.2.1"
 RegExp = "github.com/roblox/[email protected]"
 Promise = "github.com/evaera/[email protected]"

selene.toml

@@ -1,4 +1,4 @@
-std = "roblox+testez"
+std = "roblox+testez+roblox-internal"
 
 [config]
 empty_if = { comments_count = true }

src/Array/.robloxrc

@@ -0,0 +1,8 @@
+{
+	"language": {
+		"mode": "strict"
+	},
+	"lint": {
+		"*": "enabled"
+	}
+}

src/Array/__tests__/concat.spec.lua

@@ -1,6 +1,7 @@
 -- Some tests are adapted from examples at:
 -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat
 return function()
+	type Array<T> = { [number]: T }
 	local Array = script.Parent.Parent
 	local LuauPolyfill = Array.Parent
 	local concat = require(Array.concat)
@@ -44,16 +45,16 @@ return function()
 	end)
 
 	it("concatenates values to an array", function()
-		local letters = { "a", "b", "c" }
+		local letters: Array<any> = { "a", "b", "c" }
 		local alphaNumeric = concat(letters, 1, { 2, 3 })
-		jestExpect(alphaNumeric).toEqual({ "a", "b", "c", 1, 2, 3 })
+		jestExpect(alphaNumeric).toEqual({ "a", "b", "c", 1, 2, 3 } :: Array<any>)
 	end)
 
 	it("concatenates nested arrays", function()
 		local num1 = { { 1 } }
-		local num2 = { 2, { 3 } }
+		local num2: Array<any> = { 2, { 3 } }
 		local numbers = concat(num1, num2)
-		jestExpect(numbers).toEqual({ { 1 }, 2, { 3 } })
+		jestExpect(numbers).toEqual({ { 1 }, 2, { 3 } } :: Array<any>)
 	end)
 
 	if _G.__DEV__ then

src/Array/__tests__/filter.spec.lua

@@ -3,6 +3,7 @@
 
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local filter = require(Array.filter)
 	local isFinite = require(LuauPolyfill.Number.isFinite)
@@ -43,10 +44,10 @@ return function()
 			{ id = 0 },
 			{ id = 3 },
 			{ id = 12.2 },
-			{},
-			{ id = nil },
+			{} :: any,
+			{ id = nil } :: any,
 			{ id = 0 / 0 },
-			{ id = "undefined" },
+			{ id = "undefined" :: any },
 		}
 
 		local invalidEntries = 0
@@ -97,10 +98,15 @@ return function()
 			}
 
 			local modifiedWords = filter(words, function(word, index, arr)
-				if arr[index + 1] == nil then
-					arr[index + 1] = "undefined"
+				-- Luau FIXME: I cannot get the narrowing to work here: TypeError: Value of type 'Array<any>?' could be nil
+				if arr == nil or index == nil then
+					return false
+				end
+
+				if (arr :: Array<any>)[index :: number + 1] == nil then
+					(arr :: Array<any>)[index :: number + 1] = "undefined"
 				end
-				arr[index + 1] = arr[index + 1] .. " extra"
+				(arr :: Array<any>)[index :: number + 1] = (arr :: Array<any>)[index :: number + 1] .. " extra"
 				return #word < 6
 			end)
 
@@ -118,7 +124,10 @@ return function()
 			}
 
 			local modifiedWords = filter(words, function(word, index, arr)
-				table.insert(arr, "new")
+				if arr == nil then
+					return false
+				end
+				table.insert(arr :: Array<any>, "new")
 				return #word < 6
 			end)
 
@@ -136,7 +145,10 @@ return function()
 			}
 
 			local modifiedWords = filter(words, function(word, index, arr)
-				table.remove(arr)
+				if arr == nil then
+					return false
+				end
+				table.remove(arr :: Array<any>)
 				return #word < 6
 			end)
 

src/Array/__tests__/find.spec.lua

@@ -1,5 +1,6 @@
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local find = require(Array.find)
 
@@ -35,7 +36,8 @@ return function()
 		local array = { "foo" }
 		find(array, function(...)
 			arguments = { ... }
+			return false
 		end)
-		jestExpect(arguments).toEqual({ "foo", 1, array })
+		jestExpect(arguments).toEqual({ "foo", 1, array } :: Array<any>)
 	end)
 end

src/Array/__tests__/findIndex.spec.lua

@@ -1,5 +1,6 @@
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local findIndex = require(Array.findIndex)
 
@@ -35,8 +36,9 @@ return function()
 		local array = { "foo" }
 		findIndex(array, function(...)
 			arguments = { ... }
+			return false
 		end)
-		jestExpect(arguments).toEqual({ "foo", 1, array })
+		jestExpect(arguments).toEqual({ "foo", 1, array } :: Array<any>)
 	end)
 
 	-- the following tests were taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex
@@ -51,7 +53,7 @@ return function()
 	end)
 
 	it("returns first prime element", function()
-		local function isPrime(num)
+		local function isPrime(num: number)
 			for i = 2, num - 1 do
 				if num % i == 0 then
 					return false

src/Array/__tests__/forEach.spec.lua

@@ -49,7 +49,7 @@ return function()
 	it("forEach an array of numbers with a mixed field inserted", function()
 		local mock = jest.fn()
 		local numbers = { 1, 4, 9 }
-		numbers["NotANumber"] = "mixed"
+		numbers["NotANumber" :: any] = "mixed" :: any
 		forEach(numbers, function(num)
 			mock(num)
 		end)
@@ -104,7 +104,8 @@ return function()
 			return result
 		end
 
-		local nested = { 1, 2, 3, { 4, 5, { 6, 7 }, 8, 9 } }
+		-- Luau FIXME: Luau should realize this isn't an array in this single assingment: TypeError: Type '{number}' could not be converted into 'number'
+		local nested = { 1, 2, 3, { 4, 5, { 6, 7 } :: any, 8, 9 } :: any }
 		jestExpect(flatten(nested)).toEqual({ 1, 2, 3, 4, 5, 6, 7, 8, 9 })
 	end)
 end

src/Array/__tests__/from.spec.lua

@@ -2,6 +2,7 @@
 -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local from = require(Array.from)
 	local Set = require(LuauPolyfill).Set
@@ -54,7 +55,8 @@ return function()
 		local map = Map.new()
 		map:set("key1", 31337)
 		map:set("key2", 90210)
-		jestExpect(from(map)).toEqual({ { "key1", 31337 }, { "key2", 90210 } })
+		-- Luau FIXME: Luau doesn't understand multi-typed arrays
+		jestExpect(from(map)).toEqual({ { "key1", 31337 :: any }, { "key2", 90210 :: any } })
 	end)
 
 	it("returns an empty array from an empty Map", function()
@@ -63,27 +65,37 @@ return function()
 
 	describe("with mapping function", function()
 		it("maps each character", function()
-			jestExpect(from("bar", function(character, index)
+			jestExpect(from("bar", function(character: string, index)
 				return character .. index
 			end)).toEqual({ "b1", "a2", "r3" })
 		end)
 
 		it("maps each element of the array", function()
 			jestExpect(from({ 10, 20 }, function(element, index)
-				return element + index
+				-- Luau FIXME: Luau should infer element type as number without annotation
+				return element :: number + index
 			end)).toEqual({ 11, 22 })
 		end)
 
+		it("maps each element of the array with this arg", function()
+			local this = { state = 7 }
+			jestExpect(from({ 10, 20 }, function(self, element)
+				-- Luau FIXME: Luau should infer element type as number without annotation
+				return element :: number + self.state
+			end, this)).toEqual({ 17, 27 })
+		end)
+
 		it("maps each element of the array from a Set", function()
 			jestExpect(from(Set.new({ 1, 3 }), function(element, index)
-				return element + index
+				-- Luau FIXME: Luau should infer element type as number without annotation
+				return element :: number + index
 			end)).toEqual({ 2, 5 })
 		end)
 
 		it("maps each element of the array from a Map", function()
 			local map = Map.new()
 			map:set(-90210, 90210)
-			jestExpect(from(map, function(element, index)
+			jestExpect(from(map, function(element: Array<number>, index)
 				return element[1] + element[2] + index
 			end)).toEqual({ -90210 + 90210 + 1 })
 		end)

src/Array/__tests__/isArray.spec.lua

@@ -2,6 +2,7 @@
 -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local isArray = require(Array.isArray)
 
@@ -57,6 +58,6 @@ return function()
 	it("returns true for valid arrays", function()
 		jestExpect(isArray({ "a", "b", "c" })).toEqual(true)
 		jestExpect(isArray({ 1, 2, 3 })).toEqual(true)
-		jestExpect(isArray({ 1, "b", function() end })).toEqual(true)
+		jestExpect(isArray({ 1, "b", function() end } :: Array<any>)).toEqual(true)
 	end)
 end

src/Array/__tests__/map.spec.lua

@@ -51,7 +51,7 @@ return function()
 
 	it("Mapping an array of numbers using a function containing an argument", function()
 		local numbers = { 1, 4, 9 }
-		local doubles = map(numbers, function(num)
+		local doubles = map(numbers, function(num: number)
 			return num * 2
 		end)
 		jestExpect(doubles).toEqual({ 2, 8, 18 })

src/Array/__tests__/reduce.spec.lua

@@ -26,7 +26,9 @@ return function()
 
 	it("throws if no initial value is provided and the array is empty", function()
 		jestExpect(function()
-			reduce({}, function() end)
+			reduce({}, function()
+				return false
+			end)
 		end).toThrow("reduce of empty array with no initial value")
 	end)
 
@@ -35,7 +37,9 @@ return function()
 		-- invalid argument, so it needs to be cast to any
 		local reduceAny: any = reduce
 		jestExpect(function()
-			reduceAny(nil, function() end)
+			reduceAny(nil, function()
+				return false
+			end)
 		end).toThrow()
 		jestExpect(function()
 			reduceAny({ 0, 1 }, nil)

src/Array/__tests__/slice.spec.lua

@@ -1,9 +1,8 @@
---!nocheck
--- nocheck here since a test here purposefully violates the type check to test argument validation
 -- Tests adapted directly from examples at:
 -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice
 return function()
 	local Array = script.Parent.Parent
+	type Array<T> = { [number]: T }
 	local LuauPolyfill = Array.Parent
 	local slice = require(Array.slice)
 
@@ -12,9 +11,9 @@ return function()
 	local jestExpect = JestGlobals.expect
 
 	it("Invalid argument", function()
-		-- Luau analysis correctly warns and prevents this abuse case!
 		jestExpect(function()
-			slice(nil, 1)
+			-- Luau analysis correctly warns and prevents this abuse case, typecast to force it through
+			slice((nil :: any) :: Array<any>, 1)
 		end).toThrow()
 	end)
 

src/Array/__tests__/some.spec.lua

@@ -41,7 +41,7 @@ return function()
 	end)
 
 	it("Converting any value to Boolean", function()
-		local truthy_values = { true, "true", 1 }
+		local truthy_values = { true, "true" :: any, 1 :: any }
 		local getBoolean = function(value)
 			return some(truthy_values, function(t)
 				return t == value

src/Array/concat.lua

@@ -1,3 +1,4 @@
+--!strict
 local Array = script.Parent
 local isArray = require(Array.isArray)
 

src/Array/every.lua

@@ -1,12 +1,12 @@
 --!strict
 type Array<T> = { [number]: T }
-type callbackFn = (element: any, index: number?, array: Array<any>?) -> boolean
-type callbackFnWithThisArg = (thisArg: any, element: any, index: number?, array: Array<any>?) -> boolean
+type callbackFn<T> = (element: T, index: number, array: Array<T>?) -> boolean
+type callbackFnWithThisArg<T, U> = (self: U, element: T, index: number, array: Array<T>?) -> boolean
 type Object = { [string]: any }
 
 -- Implements Javascript's `Array.prototype.every` as defined below
 -- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every
-return function(t: Array<any>, callback: callbackFn | callbackFnWithThisArg, thisArg: Object?): boolean
+return function<T, U>(t: Array<T>, callback: callbackFn<T> | callbackFnWithThisArg<T, U>, thisArg: U?): boolean
 	if typeof(t) ~= "table" then
 		error(string.format("Array.every called on %s", typeof(t)))
 	end
@@ -23,9 +23,9 @@ return function(t: Array<any>, callback: callbackFn | callbackFnWithThisArg, thi
 
 		if kValue ~= nil then
 			if thisArg ~= nil then
-				testResult = (callback :: callbackFnWithThisArg)(thisArg, kValue, k, t)
+				testResult = (callback :: callbackFnWithThisArg<T, U>)(thisArg, kValue, k, t)
 			else
-				testResult = (callback :: callbackFn)(kValue, k, t)
+				testResult = (callback :: callbackFn<T>)(kValue, k, t)
 			end
 
 			if not testResult then