From 908ce16a9adc3811814cfcbec630cfd439cc23d1 Mon Sep 17 00:00:00 2001 From: Hisham Muhammad Date: Sun, 13 Oct 2024 01:19:12 -0300 Subject: [PATCH] fix: disallow passing non-record types as arguments Fixes #813. --- spec/lang/assignment/to_interface_spec.lua | 9 +- spec/lang/call/function_spec.lua | 24 ++++- spec/lang/declaration/record_spec.lua | 9 +- spec/lang/operator/is_spec.lua | 6 +- tl.lua | 106 ++++++++++++--------- tl.tl | 106 ++++++++++++--------- 6 files changed, 152 insertions(+), 108 deletions(-) diff --git a/spec/lang/assignment/to_interface_spec.lua b/spec/lang/assignment/to_interface_spec.lua index 2c3f4e49e..1f3dd02b5 100644 --- a/spec/lang/assignment/to_interface_spec.lua +++ b/spec/lang/assignment/to_interface_spec.lua @@ -34,11 +34,12 @@ describe("assignment", function() err = {} if outer == "interface" and scope:match("with outer def") then table.insert(err, { y = 6, msg = "interfaces are abstract; consider using a concrete record" }) - end - if inner == "record" then - table.insert(err, { y = 6, msg = "cannot reassign a type" }) else - table.insert(err, { y = 6, msg = "interfaces are abstract; consider using a concrete record" }) + if inner == "record" then + table.insert(err, { y = 6, msg = "cannot reassign a type" }) + else + table.insert(err, { y = 6, msg = "interfaces are abstract; consider using a concrete record" }) + end end elseif outer == "interface" and scope == "to inner var with outer def" then -- 4 err = { { y = 6, msg = "interfaces are abstract; consider using a concrete record" } } diff --git a/spec/lang/call/function_spec.lua b/spec/lang/call/function_spec.lua index 5788ff5b6..8c7abc195 100644 --- a/spec/lang/call/function_spec.lua +++ b/spec/lang/call/function_spec.lua @@ -1,6 +1,6 @@ util = require("spec.util") -describe("function calls", function() +describe("function call", function() it("does not crash attempting to infer an emptytable when there's no return type", util.check_type_error([[ local function f() end @@ -33,6 +33,28 @@ describe("function calls", function() end ]])) + it("does not allow passing non-record type definitions as values (#813)" , util.check_type_error([[ + local enum TestEnum + "test1" + end + + local function testFunc(param1:TestEnum) + end + + testFunc(TestEnum) + + local interface X + end + + local function testFunc2(param1:X) + end + + testFunc2(X) + ]], { + { y = 8, msg = "argument 1: cannot use a type definition as a concrete value" }, + { y = 16, msg = "argument 1: interfaces are abstract" }, + })) + describe("check the arity of functions:", function() it("when excessive", util.check_type_error([[ local function f(n: number, m: number): number diff --git a/spec/lang/declaration/record_spec.lua b/spec/lang/declaration/record_spec.lua index 4a48886b5..99ee580fb 100644 --- a/spec/lang/declaration/record_spec.lua +++ b/spec/lang/declaration/record_spec.lua @@ -741,8 +741,9 @@ for i, name in ipairs({"records", "arrayrecords", "interfaces", "arrayinterfaces proto.x = 2 ]], { - { y = 16, msg = "cannot use a nested type alias as a concrete value" }, - { y = 20, msg = "cannot use a nested type alias as a concrete value" }, + { y = 16, msg = "cannot use a type definition as a concrete value" }, + { y = 18, msg = "cannot use a type definition as a concrete value" }, + { y = 20, msg = "cannot use a type definition as a concrete value" }, })) else it("cannot use nested type aliases as values (see #416)", util.check_type_error([[ @@ -768,11 +769,7 @@ for i, name in ipairs({"records", "arrayrecords", "interfaces", "arrayinterfaces proto.x = 2 ]], { { y = 16, msg = "interfaces are abstract" }, - { y = 16, msg = "cannot use a nested type alias as a concrete value" }, { y = 18, msg = "interfaces are abstract" }, - { y = 18, msg = "interfaces are abstract" }, - { y = 18, msg = "interfaces are abstract" }, - { y = 20, msg = "cannot use a nested type alias as a concrete value" }, })) end diff --git a/spec/lang/operator/is_spec.lua b/spec/lang/operator/is_spec.lua index 8ed5a5c36..154ba6575 100644 --- a/spec/lang/operator/is_spec.lua +++ b/spec/lang/operator/is_spec.lua @@ -396,10 +396,10 @@ describe("flow analysis with is", function() end ]], { { msg = "can only use 'is' on variables, not types" }, - { msg = "cannot use operator '..'" }, + { msg = "cannot use a type definition as a concrete value" }, { msg = "can only use 'is' on variables, not types" }, - { msg = "cannot use operator '+'" }, - { msg = "cannot index" }, + { msg = "cannot use a type definition as a concrete value" }, + { msg = "cannot use a type definition as a concrete value" }, })) it("produces no errors or warnings for checks on unions of records", util.check_warnings([[ diff --git a/tl.lua b/tl.lua index b4363773a..287cc002b 100644 --- a/tl.lua +++ b/tl.lua @@ -7305,6 +7305,8 @@ do local def = t.def if def.typename == "interface" then return nil, "interfaces are abstract; consider using a concrete record" + elseif not (def.typename == "record") then + return nil, "cannot use a type definition as a concrete value" end end return true @@ -7830,46 +7832,6 @@ do - - - - - - - - - - - - - - function TypeChecker:arg_check(w, all_errs, a, b, v, mode, n) - local ok, errs - - if v == "covariant" then - ok, errs = self:is_a(a, b) - elseif v == "contravariant" then - ok, errs = self:is_a(b, a) - elseif v == "bivariant" then - ok, errs = self:is_a(a, b) - if ok then - return true - end - ok = self:is_a(b, a) - if ok then - return true - end - elseif v == "invariant" then - ok, errs = self:same_type(a, b) - end - - if not ok then - self.errs:add_prefixing(w, errs, mode .. (n and " " .. n or "") .. ": ", all_errs) - return false - end - return true - end - function TypeChecker:has_all_types_of(t1s, t2s) for _, t1 in ipairs(t1s) do local found = false @@ -8125,6 +8087,54 @@ do end end + + + + + + + + + + + + + + function TypeChecker:arg_check(w, all_errs, a, b, v, mode, n) + local ok, err, errs + + if v == "covariant" then + ok, errs = self:is_a(a, b) + elseif v == "contravariant" then + ok, errs = self:is_a(b, a) + elseif v == "bivariant" then + ok, errs = self:is_a(a, b) + if ok then + return true + end + ok = self:is_a(b, a) + if ok then + return true + end + elseif v == "invariant" then + ok, errs = self:same_type(a, b) + end + + if ok and b.typename == "nominal" then + local rb = self:resolve_nominal(b) + ok, err = ensure_not_abstract(rb) + if not ok then + errs = { Err_at(w, err) } + end + end + + if not ok then + self.errs:add_prefixing(w, errs, mode .. (n and " " .. n or "") .. ": ", all_errs) + return false + end + return true + end + do local function are_same_unresolved_global_type(self, t1, t2) if t1.names[1] == t2.names[1] then @@ -12251,13 +12261,14 @@ self:expand_type(node, values, elements) }) elseif node.op.op == "as" then return gb - end - local expected = node.expected and self:to_structural(resolve_tuple(node.expected)) + elseif node.op.op == "is" and ra.typename == "typedecl" then + return self.errs:invalid_at(node, "can only use 'is' on variables, not types") + end local ok, err = ensure_not_abstract(ra) if not ok then - self.errs:add(node.e1, err) + return self.errs:invalid_at(node.e1, err) end if ra.typename == "typedecl" and ra.def.typename == "record" then ra = ra.def @@ -12270,7 +12281,7 @@ self:expand_type(node, values, elements) }) rb = self:to_structural(ub) ok, err = ensure_not_abstract(rb) if not ok then - self.errs:add(node.e2, err) + return self.errs:invalid_at(node.e2, err) end if rb.typename == "typedecl" and rb.def.typename == "record" then rb = rb.def @@ -12309,9 +12320,7 @@ self:expand_type(node, values, elements) }) if rb.typename == "integer" then self.all_needs_compat["math"] = true end - if ra.typename == "typedecl" then - self.errs:add(node, "can only use 'is' on variables, not types") - elseif node.e1.kind == "variable" then + if node.e1.kind == "variable" then self:check_metamethod(node, "__is", ra, resolve_typedecl(rb), ua, ub) node.known = IsFact({ var = node.e1.tk, typ = ub, w = node }) else @@ -12352,6 +12361,9 @@ self:expand_type(node, values, elements) }) if node.op.op == "or" then local t + + local expected = node.expected and self:to_structural(resolve_tuple(node.expected)) + if ub.typename == "nil" then node.known = nil t = ua diff --git a/tl.tl b/tl.tl index d5b2aa58a..3b4755f53 100644 --- a/tl.tl +++ b/tl.tl @@ -7305,6 +7305,8 @@ do local def = t.def if def is InterfaceType then return nil, "interfaces are abstract; consider using a concrete record" + elseif not def is RecordType then + return nil, "cannot use a type definition as a concrete value" end end return true @@ -7830,46 +7832,6 @@ do local type CompareTypes = function(TypeChecker, Type, Type): boolean, {Error} - local enum ArgCheckMode - "argument" - "return" - "self" - end - - local enum VarianceMode - "covariant" - "contravariant" - "bivariant" - "invariant" - end - - function TypeChecker:arg_check(w: Where, all_errs: {Error}, a: Type, b: Type, v: VarianceMode, mode: ArgCheckMode, n?: integer): boolean - local ok, errs: boolean, {Error} - - if v == "covariant" then - ok, errs = self:is_a(a, b) - elseif v == "contravariant" then - ok, errs = self:is_a(b, a) - elseif v == "bivariant" then - ok, errs = self:is_a(a, b) - if ok then - return true - end - ok = self:is_a(b, a) - if ok then - return true - end - elseif v == "invariant" then - ok, errs = self:same_type(a, b) - end - - if not ok then - self.errs:add_prefixing(w, errs, mode .. (n and " " .. n or "") .. ": ", all_errs) - return false - end - return true - end - function TypeChecker:has_all_types_of(t1s: {Type}, t2s: {Type}): boolean for _, t1 in ipairs(t1s) do local found = false @@ -8125,6 +8087,54 @@ do end end + local enum ArgCheckMode + "argument" + "return" + "self" + end + + local enum VarianceMode + "covariant" + "contravariant" + "bivariant" + "invariant" + end + + function TypeChecker:arg_check(w: Where, all_errs: {Error}, a: Type, b: Type, v: VarianceMode, mode: ArgCheckMode, n?: integer): boolean + local ok, err, errs: boolean, string, {Error} + + if v == "covariant" then + ok, errs = self:is_a(a, b) + elseif v == "contravariant" then + ok, errs = self:is_a(b, a) + elseif v == "bivariant" then + ok, errs = self:is_a(a, b) + if ok then + return true + end + ok = self:is_a(b, a) + if ok then + return true + end + elseif v == "invariant" then + ok, errs = self:same_type(a, b) + end + + if ok and b is NominalType then + local rb = self:resolve_nominal(b) + ok, err = ensure_not_abstract(rb) + if not ok then + errs = { Err_at(w, err) } + end + end + + if not ok then + self.errs:add_prefixing(w, errs, mode .. (n and " " .. n or "") .. ": ", all_errs) + return false + end + return true + end + do local function are_same_unresolved_global_type(self: TypeChecker, t1: NominalType, t2: NominalType): boolean if t1.names[1] == t2.names[1] then @@ -12251,13 +12261,14 @@ do elseif node.op.op == "as" then return gb - end - local expected = node.expected and self:to_structural(resolve_tuple(node.expected)) + elseif node.op.op == "is" and ra is TypeDeclType then + return self.errs:invalid_at(node, "can only use 'is' on variables, not types") + end local ok, err = ensure_not_abstract(ra) if not ok then - self.errs:add(node.e1, err) + return self.errs:invalid_at(node.e1, err) end if ra is TypeDeclType and ra.def.typename == "record" then ra = ra.def @@ -12270,7 +12281,7 @@ do rb = self:to_structural(ub) ok, err = ensure_not_abstract(rb) if not ok then - self.errs:add(node.e2, err) + return self.errs:invalid_at(node.e2, err) end if rb is TypeDeclType and rb.def.typename == "record" then rb = rb.def @@ -12309,9 +12320,7 @@ do if rb.typename == "integer" then self.all_needs_compat["math"] = true end - if ra is TypeDeclType then - self.errs:add(node, "can only use 'is' on variables, not types") - elseif node.e1.kind == "variable" then + if node.e1.kind == "variable" then self:check_metamethod(node, "__is", ra, resolve_typedecl(rb), ua, ub) node.known = IsFact { var = node.e1.tk, typ = ub, w = node } else @@ -12352,6 +12361,9 @@ do if node.op.op == "or" then local t: Type + + local expected = node.expected and self:to_structural(resolve_tuple(node.expected)) + if ub.typename == "nil" then node.known = nil t = ua