diff --git a/spec/lang/code_gen/local_type_spec.lua b/spec/lang/code_gen/local_type_spec.lua index 1fa2eab6e..86036871f 100644 --- a/spec/lang/code_gen/local_type_spec.lua +++ b/spec/lang/code_gen/local_type_spec.lua @@ -239,4 +239,43 @@ describe("local type code generation", function() ]]) end) + it("always elides local type require used as a variable, even if incorrect use of interfaces or aliases", util.gen([[ + local interface IFoo + end + + local type Alias = IFoo + + local record Foo is IFoo + end + + local function register(_id:any, _value:any) + end + + local foo:Foo + + register(IFoo, foo) + + register(Alias, foo) + ]], [[ + + + + + + + + + local function register(_id, _value) + end + + local foo + + register(IFoo, foo) + + register(Alias, foo) + ]], nil, { + { y = 14, msg = "interfaces are abstract" }, + { y = 16, msg = "interfaces are abstract" }, + })) + end) diff --git a/spec/lang/stdlib/require_spec.lua b/spec/lang/stdlib/require_spec.lua index 5ecfb84a5..0f3ba4adf 100644 --- a/spec/lang/stdlib/require_spec.lua +++ b/spec/lang/stdlib/require_spec.lua @@ -1216,7 +1216,7 @@ describe("require", function() assert.same({}, result.syntax_errors) assert.same({ - { filename = "main.tl", x = 37, y = 1, msg = "type is not a record" }, + { filename = "main.tl", x = 30, y = 1, msg = "'require' did not return a type, got integer" }, { filename = "main.tl", x = 45, y = 1, msg = "cannot index key 'Foo' in type integer" }, }, result.type_errors) end) @@ -1296,4 +1296,60 @@ describe("require", function() assert.same({}, result.type_errors) end) + it("a module returning an interface gives a helpful error message (#829)", function () + util.mock_io(finally, { + ["ifoo.tl"] = [[ + local interface IFoo + bar: function(self) + end + + return IFoo + ]], + ["main.tl"] = [[ + local IFoo = require("ifoo") + + local record Foo is IFoo + end + + function Foo:bar() + print("bar") + end + ]], + }) + local result, err = tl.process("main.tl") + + assert.same({}, result.syntax_errors) + assert.same({ + { filename = "main.tl", x = 33, y = 1, msg = "module type is abstract: interface IFoo" } + }, result.type_errors) + assert.same({ + { filename = "main.tl", x = 19, y = 1, msg = "hint: consider using 'local type' instead", tag = "hint" } + }, result.warnings) + end) + + it("a module returning an interface instance can be required concretely (#825)", function () + util.mock_io(finally, { + ["foo.tl"] = [[ + local interface IFoo + bar: function(self) + end + + local foo: IFoo = {} + + return foo + ]], + ["main.tl"] = [[ + local foo = require("foo") + + foo.bar = function(self) + print("bar") + end + ]], + }) + local result, err = tl.process("main.tl") + + assert.same({}, result.syntax_errors) + assert.same({}, result.type_errors) + end) + end) diff --git a/spec/util.lua b/spec/util.lua index 95b3d1b62..3b3dc30aa 100644 --- a/spec/util.lua +++ b/spec/util.lua @@ -599,13 +599,22 @@ function util.check_types(code, types) end end -local function gen(lax, code, expected, gen_target) +local function gen(lax, code, expected, gen_target, type_errors) return function() local ast, syntax_errors = tl.parse(code, "foo.tl") assert.same({}, syntax_errors, "Code was not expected to have syntax errors") local gen_compat = gen_target == "5.4" and "off" or nil local result = tl.check(ast, "foo.tl", { feat_lax = lax and "on" or "off", gen_target = gen_target, gen_compat = gen_compat }) - assert.same({}, result.type_errors) + + if type_errors then + local batch = batch_assertions() + local result_type_errors = combine_result(result, "type_errors") + batch_compare(batch, "type errors", type_errors, result_type_errors) + batch:assert() + else + assert.same({}, result.type_errors) + end + local output_code = tl.pretty_print_ast(ast, gen_target) local expected_ast, expected_errors = tl.parse(expected, "foo.tl") @@ -616,11 +625,11 @@ local function gen(lax, code, expected, gen_target) end end -function util.gen(code, expected, gen_target) +function util.gen(code, expected, gen_target, type_errors) assert(type(code) == "string") assert(type(expected) == "string") - return gen(false, code, expected, gen_target) + return gen(false, code, expected, gen_target, type_errors) end function util.run_check_type_error(...) diff --git a/tl.lua b/tl.lua index 6e8cbcb61..54a5f5120 100644 --- a/tl.lua +++ b/tl.lua @@ -2323,6 +2323,23 @@ local simple_types = { ["self"] = true, } +local function node_is_require_call(n) + if not (n.e1 and n.e2) then + return nil + end + if n.op and n.op.op == "." then + + return node_is_require_call(n.e1) + elseif n.e1.kind == "variable" and n.e1.tk == "require" and + n.e2.kind == "expression_list" and #n.e2 == 1 and + n.e2[1].kind == "string" then + + + return n.e2[1].conststr + end + return nil +end + do @@ -2960,23 +2977,6 @@ do return fail(ps, i, "syntax error") end - local function node_is_require_call(n) - if not (n.e1 and n.e2) then - return nil - end - if n.op and n.op.op == "." then - - return node_is_require_call(n.e1) - elseif n.e1.kind == "variable" and n.e1.tk == "require" and - n.e2.kind == "expression_list" and #n.e2 == 1 and - n.e2[1].kind == "string" then - - - return n.e2[1].conststr - end - return nil - end - local function node_is_require_call_or_pcall(n) local r = node_is_require_call(n) if r then @@ -6341,6 +6341,23 @@ function Errors:add_prefixing(w, src, prefix, dst) end end +local function ensure_not_abstract(t, node) + if t.typename == "function" and t.macroexp then + return nil, "macroexps are abstract; consider using a concrete function" + elseif t.typename == "typedecl" then + local def = t.def + if def.typename == "record" then + return true + elseif node and node_is_require_call(node) then + return nil, "module type is abstract: " .. tostring(t.def) + elseif def.typename == "interface" then + return nil, "interfaces are abstract; consider using a concrete record" + end + return nil, "cannot use a type definition as a concrete value" + end + return true +end + @@ -6368,7 +6385,9 @@ local function check_for_unused_vars(scope, is_global) end elseif var.used and t.typename == "typedecl" and var.aliasing then var.aliasing.used = true - var.aliasing.declared_at.elide_type = false + if ensure_not_abstract(t) then + var.aliasing.declared_at.elide_type = false + end end end if list then @@ -7298,20 +7317,6 @@ do end end - local function ensure_not_abstract(t) - if t.typename == "function" and t.macroexp then - return nil, "macroexps are abstract; consider using a concrete function" - elseif t.typename == "typedecl" then - 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 - end - local function ensure_not_method(t) if t.typename == "function" and t.is_method then @@ -7768,10 +7773,9 @@ do return ret end - function TypeChecker:add_to_scope(node, name, t, attribute, narrow, dont_check_redeclaration) - local scope = self.st[#self.st] - local var = scope.vars[name] - if narrow then + do + local function narrow_var(scope, node, name, t, attribute, narrow) + local var = scope.vars[name] if var then if var.is_narrowed then var.t = t @@ -7792,42 +7796,44 @@ do return var end - if not dont_check_redeclaration and - node and - name ~= "self" and - name ~= "..." and - name:sub(1, 1) ~= "@" then - - self:check_if_redeclaration(name, node, t) - end + function TypeChecker:add_var(node, name, t, attribute, narrow) + if self.feat_lax and node and is_unknown(t) and (name ~= "self" and name ~= "...") and not narrow then + self.errs:add_unknown(node, name) + end + if not attribute then + t = drop_constant_value(t) + end - if var and not var.used then + if self.collector and node then + self.collector.add_to_symbol_list(node, name, t) + end + local scope = self.st[#self.st] + if narrow then + return narrow_var(scope, node, name, t, attribute, narrow) + end - self.errs:unused_warning(name, var) - end + if node then + if name ~= "self" and name ~= "..." and name:sub(1, 1) ~= "@" then + self:check_if_redeclaration(name, node, t) + end + if not ensure_not_abstract(t) then + node.elide_type = true + end + end - var = { t = t, attribute = attribute, is_narrowed = nil, declared_at = node } - scope.vars[name] = var + local var = scope.vars[name] + if var and not var.used then - return var - end - function TypeChecker:add_var(node, name, t, attribute, narrow, dont_check_redeclaration) - if self.feat_lax and node and is_unknown(t) and (name ~= "self" and name ~= "...") and not narrow then - self.errs:add_unknown(node, name) - end - if not attribute then - t = drop_constant_value(t) - end + self.errs:unused_warning(name, var) + end - local var = self:add_to_scope(node, name, t, attribute, narrow, dont_check_redeclaration) + var = { t = t, attribute = attribute, declared_at = node } + scope.vars[name] = var - if self.collector and node then - self.collector.add_to_symbol_list(node, name, t) + return var end - - return var end @@ -11145,7 +11151,7 @@ self:expand_type(node, values, elements) }) if not (ty.typename == "typedecl") then - return a_type(n, "typedecl", { def = ty }) + return self.errs:invalid_at(n.e1, "'require' did not return a type, got %s", ty) end if ty.is_alias then return self:resolve_typealias(ty) @@ -11366,6 +11372,9 @@ self:expand_type(node, values, elements) }) assert(var) self:add_var(var, var.tk, t, var.attribute, is_localizing_a_variable(node, i) and "localizing") + if var.elide_type then + self.errs:add_warning("hint", node, "hint: consider using 'local type' instead") + end local infertype = infertypes.tuple[i] if ok and infertype then @@ -11404,6 +11413,9 @@ self:expand_type(node, values, elements) }) end self:add_global(var, var.tk, t, is_inferred) + if var.elide_type then + self.errs:add_warning("hint", node, "hint: consider using 'global type' instead") + end self:dismiss_unresolved(var.tk) end @@ -11666,14 +11678,15 @@ self:expand_type(node, values, elements) }) local expected = self:find_var_type("@return") if not expected then - expected = self:infer_at(node, got) - local module_type = resolve_tuple(expected) + local module_type = resolve_tuple(got) if module_type.typename == "nominal" then self:resolve_nominal(module_type) - self.module_type = module_type.found + self.module_type = module_type.resolved else self.module_type = drop_constant_value(module_type) end + + expected = self:infer_at(node, got) self.st[2].vars["@return"] = { t = expected } end local expected_t = expected.tuple @@ -11723,7 +11736,7 @@ self:expand_type(node, values, elements) }) tuple = flatten_tuple(tuple) for i, t in ipairs(tuple.tuple) do - local ok, err = ensure_not_abstract(t) + local ok, err = ensure_not_abstract(t, node[i]) if not ok then self.errs:add(node[i], err) end @@ -12150,6 +12163,15 @@ self:expand_type(node, values, elements) }) before = function(self, node) self:widen_all_unions(node) self:begin_scope(node) + + local expected = node.expected + if expected and expected.typename == "function" then + for i, t in ipairs(expected.args.tuple) do + if node.args[i] then + node.args[i].expected = t + end + end + end end, before_statements = function(self, node, children) local args = children[1] @@ -12672,9 +12694,13 @@ self:expand_type(node, values, elements) }) after = function(self, node, children) local t = children[1] if not t then - t = self.feat_lax and - a_type(node, "unknown", {}) or - a_type(node, "any", {}) + if node.expected and node.tk == "self" then + t = node.expected + else + t = self.feat_lax and + a_type(node, "unknown", {}) or + a_type(node, "any", {}) + end end if node.tk == "..." then t = a_vararg(node, { t }) diff --git a/tl.tl b/tl.tl index ad2ffff4c..a8bf93972 100644 --- a/tl.tl +++ b/tl.tl @@ -2323,6 +2323,23 @@ local simple_types: {TypeName:boolean} = { ["self"] = true, } +local function node_is_require_call(n: Node): string + if not (n.e1 and n.e2) then + return nil + end + if n.op and n.op.op == "." then + -- `require("str").something` + return node_is_require_call(n.e1) + elseif n.e1.kind == "variable" and n.e1.tk == "require" + and n.e2.kind == "expression_list" and #n.e2 == 1 + and n.e2[1].kind == "string" + then + -- `require("str")` + return n.e2[1].conststr + end + return nil -- table.insert cares about arity +end + do ----------------------------------------------------------------------------- local record ParseState @@ -2960,23 +2977,6 @@ local function parse_literal(ps: ParseState, i: integer): integer, Node return fail(ps, i, "syntax error") end -local function node_is_require_call(n: Node): string - if not (n.e1 and n.e2) then - return nil - end - if n.op and n.op.op == "." then - -- `require("str").something` - return node_is_require_call(n.e1) - elseif n.e1.kind == "variable" and n.e1.tk == "require" - and n.e2.kind == "expression_list" and #n.e2 == 1 - and n.e2[1].kind == "string" - then - -- `require("str")` - return n.e2[1].conststr - end - return nil -- table.insert cares about arity -end - local function node_is_require_call_or_pcall(n: Node): string local r = node_is_require_call(n) if r then @@ -6341,6 +6341,23 @@ function Errors:add_prefixing(w: Where, src: {Error}, prefix: string, dst?: {Err end end +local function ensure_not_abstract(t: Type, node?: Node): boolean, string + if t is FunctionType and t.macroexp then + return nil, "macroexps are abstract; consider using a concrete function" + elseif t is TypeDeclType then + local def = t.def + if def is RecordType then + return true + elseif node and node_is_require_call(node) then + return nil, "module type is abstract: " .. tostring(t.def) + elseif def is InterfaceType then + return nil, "interfaces are abstract; consider using a concrete record" + end + return nil, "cannot use a type definition as a concrete value" + end + return true +end + local record Unused y: integer x: integer @@ -6368,7 +6385,9 @@ local function check_for_unused_vars(scope: Scope, is_global?: boolean): {Unused end elseif var.used and t is TypeDeclType and var.aliasing then var.aliasing.used = true - var.aliasing.declared_at.elide_type = false + if ensure_not_abstract(t) then + var.aliasing.declared_at.elide_type = false + end end end if list then @@ -7298,20 +7317,6 @@ do end end - local function ensure_not_abstract(t: Type): boolean, string - if t is FunctionType and t.macroexp then - return nil, "macroexps are abstract; consider using a concrete function" - elseif t is TypeDeclType then - 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 - end - local function ensure_not_method(t: Type): Type if t is FunctionType and t.is_method then @@ -7768,10 +7773,9 @@ do return ret end - function TypeChecker:add_to_scope(node: Node, name: string, t: Type, attribute: Attribute, narrow: Narrow, dont_check_redeclaration: boolean): Variable - local scope = self.st[#self.st] - local var = scope.vars[name] - if narrow then + do + local function narrow_var(scope: Scope, node: Node, name: string, t: Type, attribute: Attribute, narrow: Narrow): Variable + local var = scope.vars[name] if var then if var.is_narrowed then var.t = t @@ -7792,42 +7796,44 @@ do return var end - if not dont_check_redeclaration - and node - and name ~= "self" - and name ~= "..." - and name:sub(1, 1) ~= "@" - then - self:check_if_redeclaration(name, node, t) - end + function TypeChecker:add_var(node: Node, name: string, t: Type, attribute?: Attribute, narrow?: Narrow): Variable + if self.feat_lax and node and is_unknown(t) and (name ~= "self" and name ~= "...") and not narrow then + self.errs:add_unknown(node, name) + end + if not attribute then + t = drop_constant_value(t) + end - if var and not var.used then - -- the old var is removed from the scope and won't be checked when it closes, - -- so check it here - self.errs:unused_warning(name, var) - end + if self.collector and node then + self.collector.add_to_symbol_list(node, name, t) + end - var = { t = t, attribute = attribute, is_narrowed = nil, declared_at = node } - scope.vars[name] = var + local scope = self.st[#self.st] + if narrow then + return narrow_var(scope, node, name, t, attribute, narrow) + end - return var - end + if node then + if name ~= "self" and name ~= "..." and name:sub(1, 1) ~= "@" then + self:check_if_redeclaration(name, node, t) + end + if not ensure_not_abstract(t) then + node.elide_type = true + end + end - function TypeChecker:add_var(node: Node, name: string, t: Type, attribute?: Attribute, narrow?: Narrow, dont_check_redeclaration?: boolean): Variable - if self.feat_lax and node and is_unknown(t) and (name ~= "self" and name ~= "...") and not narrow then - self.errs:add_unknown(node, name) - end - if not attribute then - t = drop_constant_value(t) - end + local var = scope.vars[name] + if var and not var.used then + -- the old var is removed from the scope and won't be checked when it closes, + -- so check it here + self.errs:unused_warning(name, var) + end - local var = self:add_to_scope(node, name, t, attribute, narrow, dont_check_redeclaration) + var = { t = t, attribute = attribute, declared_at = node } + scope.vars[name] = var - if self.collector and node then - self.collector.add_to_symbol_list(node, name, t) + return var end - - return var end local type CompareTypes = function(TypeChecker, Type, Type): boolean, {Error} @@ -11145,7 +11151,7 @@ do ) ) if not ty is TypeDeclType then - return a_type(n, "typedecl", { def = ty } as TypeDeclType) + return self.errs:invalid_at(n.e1, "'require' did not return a type, got %s", ty) end if ty.is_alias then return self:resolve_typealias(ty) @@ -11366,6 +11372,9 @@ do assert(var) self:add_var(var, var.tk, t, var.attribute, is_localizing_a_variable(node, i) and "localizing") + if var.elide_type then + self.errs:add_warning("hint", node, "hint: consider using 'local type' instead") + end local infertype = infertypes.tuple[i] if ok and infertype then @@ -11404,6 +11413,9 @@ do end self:add_global(var, var.tk, t, is_inferred) + if var.elide_type then + self.errs:add_warning("hint", node, "hint: consider using 'global type' instead") + end self:dismiss_unresolved(var.tk) end @@ -11666,14 +11678,15 @@ do local expected = self:find_var_type("@return") as TupleType if not expected then -- if at the toplevel - expected = self:infer_at(node, got) - local module_type = resolve_tuple(expected) + local module_type = resolve_tuple(got) if module_type is NominalType then self:resolve_nominal(module_type) - self.module_type = module_type.found + self.module_type = module_type.resolved else self.module_type = drop_constant_value(module_type) end + + expected = self:infer_at(node, got) self.st[2].vars["@return"] = { t = expected } end local expected_t = expected.tuple @@ -11723,7 +11736,7 @@ do tuple = flatten_tuple(tuple) for i, t in ipairs(tuple.tuple) do - local ok, err = ensure_not_abstract(t) + local ok, err = ensure_not_abstract(t, node[i]) if not ok then self.errs:add(node[i], err) end @@ -12150,6 +12163,15 @@ do before = function(self: TypeChecker, node: Node) self:widen_all_unions(node) self:begin_scope(node) + + local expected = node.expected + if expected and expected is FunctionType then + for i, t in ipairs(expected.args.tuple) do + if node.args[i] then + node.args[i].expected = t + end + end + end end, before_statements = function(self: TypeChecker, node: Node, children: {Type}) local args = children[1] @@ -12672,9 +12694,13 @@ do after = function(self: TypeChecker, node: Node, children: {Type}): Type local t = children[1] if not t then - t = self.feat_lax - and an_unknown(node) - or a_type(node, "any", {}) + if node.expected and node.tk == "self" then + t = node.expected + else + t = self.feat_lax + and an_unknown(node) + or a_type(node, "any", {}) + end end if node.tk == "..." then t = a_vararg(node, { t })