Skip to content

Commit

Permalink
feat: improve check over uses of self in forward-declared methods (#642)
Browse files Browse the repository at this point in the history
### What?
- Adds new `Type.is_self` property for identifying a function/method parameter that is `self`
- Adds new `string` parameter `name` for passing the name of the body / type being parsed to:
    - the `ParseBody` type
    - `parse_newtype`
    - `parse_record_body`
- Passes the record name using the `name` parameter for all cases where `parse_record_body` is called
- Updates `parse_argument_type` so that if the argument is named `self`, the type is marked as `is_self = true`
- Updates `parse_function_type` so that if the first argument has `is_self == true`, the function type is marked as `is_method = true`
- Updates `parse_record_body` so that, before a method is added to the record's fields, it is verified that the first argument matches the type of the record, and if it does not then it is marked as `is_self = false` and the function type as `is_method = false`
- Updates `recurse_type` so that the first argument is also recursed in a method if that argument is marked as `is_self == true`
- Consolidates the near-indentical `shallow_copy` and `shallow_copy_type` methods into one

### Why?
See #638 - this PR allows for forward declaration of methods in records, based on the first argument being named `self` and being correctly typed.
This allows for warnings and clearer errors when using types defined in `.d.tl` files, as well as allowing methods to be declared in one scope and then defined in another without losing these warnings.

### Anything else?
There is an edge case: the code
```
local record MyRecord<T>
    add: function(self: MyRecord<integer>, other: MyRecord<integer>)
end
local first: MyRecord<integer> = {}
local second: MyRecord<integer> = {}
first.add(second)
```
does not raise a warning, since `add` is not treated as a method for `MyRecord`, even though it could be argued that it should be treated as a method of the type realisation `MyRecord<integer>`.

Squashed commits:

* feat: adds work-in-progress logic for methods declared inside a record

* feat: ensures that functions declared in record bodies are only treated as records if the self parameter is the correct type

* chore: consolidates nearly identical shallow_copy and shallow_copy_type methods into one
  • Loading branch information
JR-Mitchell authored Apr 27, 2023
1 parent 0bdec77 commit 67ea51b
Show file tree
Hide file tree
Showing 3 changed files with 321 additions and 131 deletions.
314 changes: 231 additions & 83 deletions spec/call/record_method_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -64,94 +64,242 @@ describe("record method call", function()
}))
end)

it("catches wrong use of self. in call", util.check_type_error([[
local record Foo
end
function Foo:method_a()
end
function Foo:method_c(arg: string)
end
function Foo:method_b()
self.method_a()
self.method_c("hello")
end
]], {
{ y = 8, msg = "invoked method as a regular function: use ':' instead of '.'" },
{ y = 9, msg = "invoked method as a regular function: use ':' instead of '.'" },
}))
describe("catches wrong use of self. in call", function()
it("for methods declared outside of the record", util.check_type_error([[
local record Foo
end
function Foo:method_a()
end
function Foo:method_c(arg: string)
end
function Foo:method_b()
self.method_a()
self.method_c("hello")
end
]], {
{ y = 8, msg = "invoked method as a regular function: use ':' instead of '.'" },
{ y = 9, msg = "invoked method as a regular function: use ':' instead of '.'" },
}))

it("reports potentially wrong use of self. in call", util.check_warnings([[
local record Foo
x: integer
end
function Foo:copy_x(other: Foo)
self.x = other.x
end
function Foo:copy_all(other: Foo)
self.copy_x(other)
end
]], {
{ y = 8, msg = "invoked method as a regular function: consider using ':' instead of '.'" }
}))
it("for methods declared inside of a record", util.check_type_error([[
local record Foo
method_a: function(self: Foo)
method_b: function(self: Foo)
method_c: function(self: Foo, arg: string)
end
Foo.method_b = function(self: Foo)
self.method_a()
self.method_c("hello")
end
]], {
{ y = 7, msg = "invoked method as a regular function: use ':' instead of '.'" },
{ y = 8, msg = "invoked method as a regular function: use ':' instead of '.'" },
}))

it("accepts use of dot call for method on record typetype", util.check_warnings([[
local record Foo
x: integer
end
function Foo:add(other: Foo)
self.x = other and (self.x + other.x) or self.x
end
local first: Foo = {}
Foo.add(first)
local q = Foo
q.add(first)
local m = {
a: Foo
}
m.a.add(first)
]], {}, {}))
it("for methods declared inside of a nested record", util.check_type_error([[
local record Foo
record Bar
method_a: function(self: Bar)
method_b: function(self: Bar)
method_c: function(self: Bar, arg: string)
end
end
Foo.Bar.method_b = function(self: Foo.Bar)
self.method_a()
self.method_c("hello")
end
]], {
{ y = 9, msg = "invoked method as a regular function: use ':' instead of '.'" },
{ y = 10, msg = "invoked method as a regular function: use ':' instead of '.'" },
}))

it("reports correct errors for calls on aliases of method", util.check_type_error([[
local record Foo
x: integer
end
function Foo:add(other: integer)
self.x = other and (self.x + other) or self.x
end
local first: Foo = {}
local fadd = first.add
fadd(12)
global gadd = first.add
gadd(13)
local tab = {
hadd = first.add
}
tab.hadd(14)
it("for methods declared inside of a generic record", util.check_type_error([[
local record Foo<T>
method_a: function(self: Foo<T>)
method_c: function(self: Foo<T>, arg: string)
end
local function_b = function<T>(self: Foo<T>)
self.method_a()
self.method_c("hello")
end
]], {
{ y = 6, msg = "invoked method as a regular function: use ':' instead of '.'" },
{ y = 7, msg = "invoked method as a regular function: use ':' instead of '.'" },
}))

end)

]],
{
{ 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" },
}))
describe("reports potentially wrong use of self. in call", function()
it("for methods declared outside of the record", util.check_warnings([[
local record Foo
x: integer
end
function Foo:copy_x(other: Foo)
self.x = other.x
end
function Foo:copy_all(other: Foo)
self.copy_x(other)
end
]], {
{ y = 8, msg = "invoked method as a regular function: consider using ':' instead of '.'" }
}))

it("reports no warnings for correctly-typed calls on aliases of method", util.check_warnings([[
local record Foo
x: integer
end
function Foo:add(other: Foo)
self.x = other and (self.x + other.x) or self.x
end
local first: Foo = {}
local fadd = first.add
fadd(first)
global gadd = first.add
gadd(first)
local tab = {
hadd = first.add
}
tab.hadd(first)
it("for methods declared inside of the record", util.check_warnings([[
local record Foo
x: integer
copy_x: function(self: Foo, other: Foo)
copy_all: function(self: Foo, other: Foo)
end
Foo.copy_all = function(self: Foo, other: Foo)
self.copy_x(other)
end
]], {
{ y = 7, msg = "invoked method as a regular function: consider using ':' instead of '.'" }
}))

it("for methods declared inside of a nested record", util.check_warnings([[
local record Foo
record Bar
x: integer
copy_x: function(self: Bar, other: Bar)
copy_all: function(self: Bar, other: Bar)
end
end
Foo.Bar.copy_all = function(self: Foo.Bar, other: Foo.Bar)
self.copy_x(other)
end
]], {
{ y = 9, msg = "invoked method as a regular function: consider using ':' instead of '.'" }
}))

it("for methods declared inside of a generic record", util.check_warnings([[
local record Foo<T>
x: integer
copy_x: function(self: Foo<T>, other: Foo<T>)
end
local copy_all = function<T>(self: Foo<T>, other: Foo<T>)
self.copy_x(other)
end
copy_all()
]], {
{ y = 6, msg = "invoked method as a regular function: consider using ':' instead of '.'" }
}))

end)

describe("accepts use of dot call", function()
it("for method on record typetype", util.check_warnings([[
local record Foo
x: integer
end
function Foo:add(other: Foo)
self.x = other and (self.x + other.x) or self.x
end
local first: Foo = {}
Foo.add(first)
local q = Foo
q.add(first)
local m = {
a: Foo
}
m.a.add(first)
]], {}, {}))

]], {}, {}))
it("for function declared in method body with self as different type from receiver", util.check_warnings([[
local record Bar
end
local record Foo
x: integer
add: function(self: Bar, other: Bar)
end
local first: Foo = {}
local second: Bar = {}
first.add(second)
]], {}, {}))

it("for function declared in method body with self as different generic type from receiver", util.check_warnings([[
local record Foo<T>
x: T
add: function(self: Foo<integer>, other: Foo<integer>)
end
local first: Foo<string> = {}
local second: Foo<integer> = {}
first.add(second)
]], {}, {}))

it("for correctly-typed calls on aliases of method", util.check_warnings([[
local record Foo
x: integer
end
function Foo:add(other: Foo)
self.x = other and (self.x + other.x) or self.x
end
local first: Foo = {}
local fadd = first.add
fadd(first)
global gadd = first.add
gadd(first)
local tab = {
hadd = first.add
}
tab.hadd(first)
]], {}, {}))

end)

describe("reports correct errors", function()

it("for calls on aliases of method", util.check_type_error([[
local record Foo
x: integer
end
function Foo:add(other: integer)
self.x = other and (self.x + other) or self.x
end
local first: Foo = {}
local fadd = first.add
fadd(12)
global gadd = first.add
gadd(13)
local tab = {
hadd = first.add
}
tab.hadd(14)
]],
{
{ 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" },
}))

it("for function declared in method body with self as different type from receiver", util.check_type_error([[
local record Bar
end
local record Foo
x: integer
add: function(self: Bar, other: Bar)
end
local first: Foo = {}
first.add(first)
]],
{
{ y = 8, msg = "argument 1: Foo is not a Bar" },
}))

it("for function declared in method body with self as different generic type from receiver", util.check_type_error([[
local record Foo<T>
x: T
add: function(self: Foo<integer>, other: Foo<integer>)
end
local first: Foo<string> = {}
first.add(first)
]],
{
{ y = 6, msg = "argument 1: type parameter <integer>: got string, expected integer" },
}))

end)

end)
Loading

0 comments on commit 67ea51b

Please sign in to comment.