-
Notifications
You must be signed in to change notification settings - Fork 149
Fix __call self #1063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix __call self #1063
Conversation
|
Teal Playground URL: https://1063--teal-playground-preview.netlify.app |
|
Looks like it will need to be fixed for EDIT: Actually reading further it is due to it being a method call rather than __call. local record Foo
end
local record Selfish
me: function(self): self
end
function Foo:bar(_a: Selfish)
end
local x: Selfish
local foo: Foo = {}
local myself = x:me()
foo:bar(myself) --> argument 1: Foo is not a Selfish |
|
For a fix for records the below patch seems to work on top of yours. (I've skipped the precompiled env files in the patch below) diff --git a/teal/check/context.tl b/teal/check/context.tl
index 3b4d3bc..aacbda0 100644
--- a/teal/check/context.tl
+++ b/teal/check/context.tl
@@ -1676,7 +1676,16 @@ function Context:match_record_key(t: Type, rec: Node, key: string): Type, string
assert(t.fields, "record has no fields!?")
if t.fields[key] then
- return t.fields[key]
+ local ft = t.fields[key]
+ local ftrep = ft
+ if ft is FunctionType then
+ ftrep = types.map(nil, ft, {
+ ["self"] = function(_: nil, _typ: SelfType): Type, boolean
+ return t, true
+ end,
+ })
+ end
+ return ftrep
end
local str = a_type(rec, "string", {})
diff --git a/teal/check/context.lua b/teal/check/context.lua
index 5379c5b..3c20285 100644
--- a/teal/check/context.lua
+++ b/teal/check/context.lua
@@ -1676,7 +1676,16 @@ function Context:match_record_key(t, rec, key)
assert(t.fields, "record has no fields!?")
if t.fields[key] then
- return t.fields[key]
+ local ft = t.fields[key]
+ local ftrep = ft
+ if ft.typename == "function" then
+ ftrep = types.map(nil, ft, {
+ ["self"] = function(_, _typ)
+ return t, true
+ end,
+ })
+ end
+ return ftrep
end
local str = a_type(rec, "string", {})
diff --git a/spec/lang/call/record_method_spec.lua b/spec/lang/call/record_method_spec.lua
index 5e000dc..a63a64c 100644
--- a/spec/lang/call/record_method_spec.lua
+++ b/spec/lang/call/record_method_spec.lua
@@ -273,9 +273,9 @@ describe("record method call", function()
]],
{
- { y = 9, msg = "argument 1: got integer, expected Foo" },
- { y = 11, msg = "argument 1: got integer, expected Foo" },
- { y = 15, msg = "argument 1: got integer, expected Foo" },
+ { y = 9, msg = "argument 1: got integer, expected record Foo" },
+ { y = 11, msg = "argument 1: got integer, expected record Foo" },
+ { y = 15, msg = "argument 1: got integer, expected record Foo" },
}))
it("for function declared in record body with self as different type from receiver", util.check_type_error([[
diff --git a/spec/lang/metamethods/call_spec.lua b/spec/lang/metamethods/call_spec.lua
index c2b3f6e..98100f1 100644
--- a/spec/lang/metamethods/call_spec.lua
+++ b/spec/lang/metamethods/call_spec.lua
@@ -234,8 +234,8 @@ describe("metamethod __call", function()
]]))
end)
- describe("interface with __call returning self", function()
- it("resolves self to the interface type, not the caller", util.check([[
+ describe("method returning self", function()
+ it("resolves self to the proper type, not the caller, with __call", util.check([[
local record Foo
end
@@ -250,7 +250,26 @@ describe("metamethod __call", function()
local x: ICallable
local foo: Foo = {}
- foo:bar(x(1))
+ local res = x(1)
+ foo:bar(res)
+ ]]))
+
+ it("resolves self to the proper type, not the caller, with method", util.check([[
+ local record Foo
+ end
+
+ local record Selfish
+ me: function(self): self
+ end
+
+ function Foo:bar(_a: Selfish)
+ end
+
+ local x: Selfish
+ local foo: Foo = {}
+
+ local myself = x:me()
+ foo:bar(myself)
]]))
end)
end)
diff --git a/spec/lang/stdlib/pcall_spec.lua b/spec/lang/stdlib/pcall_spec.lua
index 3c6e3af..a9f87e3 100644
--- a/spec/lang/stdlib/pcall_spec.lua
+++ b/spec/lang/stdlib/pcall_spec.lua
@@ -114,6 +114,6 @@ describe("pcall", function()
print(err)
end
]], {
- { msg = "argument 2: got integer, expected Text" }
+ { msg = "argument 2: got integer, expected record Text" }
}))
end)
diff --git a/spec/lang/stdlib/xpcall_spec.lua b/spec/lang/stdlib/xpcall_spec.lua
index 13dc37d..76cbfea 100644
--- a/spec/lang/stdlib/xpcall_spec.lua
+++ b/spec/lang/stdlib/xpcall_spec.lua
@@ -157,7 +157,7 @@ describe("xpcall", function()
local myText: Text = {}
xpcall(myText.print, msgh, 12)
]], {
- { msg = "argument 3: got integer, expected Text" }
+ { msg = "argument 3: got integer, expected record Text" }
}))
end) |
|
Thank you both for looking into this! I haven't looked closely at it yet, but from reading the PR and patch, I wonder if the root cause is that we're missing a call to |
|
Both what we've added aren't quite the same as I also did try to just have the field access do Also I've found another contrived test case that breaks now because it resolves self to the wrong type. local interface ReturnsGeneric<T>
getgen: function(self): T
metamethod __call: function(self): T
end
local record Test
end
function Test:testgen(g: ReturnsGeneric<self>, s: self): Test
local checkself: Test = s -- this is to verify that self here resolves correctly
return g:getgen() -- in return value: got interface ReturnsGeneric, expected Test
end
function Test:testgencall(g: ReturnsGeneric<self>, s: self): Test
local checkself: Test = s -- this is to verify that self here resolves correctly
return g() -- in return value: got interface ReturnsGeneric, expected Test
endI'm happy to accept these existing changes as is for now as it is fairly well-defined (the |
|
Thanks, @mbartlett21! I've incorporated your recommendations into the PR. |
|
I've just done a push after running |
|
Oh and that now looks like it conflicts with my changes in 59deb86 . @mtdowling Are you happy for me to rebase or you to rebase? (Generally I reset the tl.lua and other changes that conflict and then run |
|
@mbartlett21 @mtdowling feel free to merge this when you get to a resolution regarding the rebase 👍 |
|
@mbartlett21 no worries— if you don’t mind handling the rebase, that would great! |
When accessing a function field on a record, `self` in the function signature now correctly resolves to the record type being accessed, rather than the enclosing method's receiver type. (thanks to mbartlett21 for most of this commit)
c7222ab to
8531a83
Compare
|
Thank you and sorted. |
Closes #1060