Skip to content

Commit

Permalink
Fix/remove pcall method warning (#668)
Browse files Browse the repository at this point in the history
Fixes #656.

### What?
- Adds tests for the incorrect warnings and errors raised in #656 
- Updates `special_pcall_xpcall` so that the function type of the first argument is treated as not a method, even if it is a method

### Why?
Resolves #656 

`pcall` (and `xpcall`) invokes its first argument as a regular function with the provided arguments - it has no way of implicitly passing the `self` parameter from the method owner of its first argument to the method.
Therefore, it should not warn `invoked method as a regular function: use ':' instead of '.'` when its first argument is a method, or raise this as the error message when passed an argument of incorrect type.

### Anything else?
I was slightly worried that setting `is_method` to `false` on this type might cause unintended side-effects, however I have checked through the sequence of function calls from `special_pcall_xpcall` where `ftype` is passed, and it appears that `is_method` is exclusively used for determining whether to show these warnings / errors.

Squashed commits:

* feat(spec): adds tests to pcall_spec and xpcall_spec for method call warnings
* feat(spec): adds additional tests to pcall_spec and xpcall_spec for method call errors
* fix: sets is_method to false for the function type passed to pcall or xpcall
* chore: adds modified tl.lua from previous commit
* chore: clarifies comment
  • Loading branch information
JR-Mitchell authored Jun 9, 2023
1 parent cc96c76 commit 8829655
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 0 deletions.
36 changes: 36 additions & 0 deletions spec/stdlib/pcall_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,40 @@ describe("pcall", function()
]], {
{ msg = "xyz: got boolean, expected number" }
}))

it("does not warn when passed a method as the first argument", util.check_warnings([[
local record Text
text: string
end
function Text:print(): nil
if self.text == nil then
error("Text is nil")
end
print(self.text)
end
local myText: Text = {}
local ok, err = pcall(myText.print, myText)
if not ok then
print(err)
end
]], {}, {}))

it("checks the correct input arguments when passed a method as the first argument", util.check_type_error([[
local record Text
text: string
end
function Text:print(): nil
if self.text == nil then
error("Text is nil")
end
print(self.text)
end
local myText: Text = {}
local ok, err = pcall(myText.print, 12)
if not ok then
print(err)
end
]], {
{ msg = "argument 2: got integer, expected Text" }
}))
end)
36 changes: 36 additions & 0 deletions spec/stdlib/xpcall_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,40 @@ describe("xpcall", function()
{ msg = "argument 3: got integer, expected string" },
}))

it("does not warn when passed a method as the first argument", util.check_warnings([[
local record Text
text: string
end
local function msgh(err: nil) print(err) end
function Text:print(): nil
if self.text == nil then
error("Text is nil")
end
print(self.text)
end
local myText: Text = {}
xpcall(myText.print, msgh, myText)
]], {}, {}))

it("checks the correct input arguments when passed a method as the first argument", util.check_type_error([[
local record Text
text: string
end
local function msgh(err: nil) print(err) end
function Text:print(): nil
if self.text == nil then
error("Text is nil")
end
print(self.text)
end
local myText: Text = {}
xpcall(myText.print, msgh, 12)
]], {
{ msg = "argument 3: got integer, expected Text" }
}))

end)
4 changes: 4 additions & 0 deletions tl.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8448,7 +8448,11 @@ tl.type_check = function(ast, opts)
return TUPLE({ BOOLEAN })
end


local ftype = table.remove(b, 1)
ftype = shallow_copy_type(ftype)
ftype.is_method = false

local fe2 = {}
if node.e1.tk == "xpcall" then
base_nargs = 2
Expand Down
4 changes: 4 additions & 0 deletions tl.tl
Original file line number Diff line number Diff line change
Expand Up @@ -8448,7 +8448,11 @@ tl.type_check = function(ast: Node, opts: TypeCheckOptions): Result, string
return TUPLE { BOOLEAN }
end

-- The function called by pcall/xpcall is invoked as a regular function, so we wish to avoid incorrect error messages / unnecessary warning messages associated with calling methods as functions
local ftype = table.remove(b, 1)
ftype = shallow_copy_type(ftype)
ftype.is_method = false

local fe2: Node = {}
if node.e1.tk == "xpcall" then
base_nargs = 2
Expand Down

0 comments on commit 8829655

Please sign in to comment.