Allow interfaces and records to have tuple keys#1101
Allow interfaces and records to have tuple keys#1101hishamhm merged 10 commits intoteal-language:mainfrom
Conversation
|
Teal Playground URL: https://1101--teal-playground-preview.netlify.app |
hishamhm
left a comment
There was a problem hiding this comment.
Looking good! Thank you for this contribution! Could you add some tests under spec/ so we can exercise this new code?
I'll have to learn how the testing suite works, but I'll do my best |
As requested by maintainers
|
As requested, I have updated the code to hard error on incompatible array/tuple typings. Here is the updated example code: -------- Compatibility with array interfaces --------
local interface MySimpleTupleInterface is {string, string}
-- Tuple interfaces can be compatible with array interfaces only if all types are equal
end
------- Simple compatibility -------
local interface MyCompatibleArrayInterface is {string}
-- An array of strings is compatible with {string, string}
end
local interface MyIncompatibleArrayInterface is {boolean}
-- An array of booleans is not compatible with {string, string}
end
-- warn: "inherits overlapping array {string} and tuple {string, string}"
local record MyCompatibleTupleArrayRecord is MySimpleTupleInterface, MyCompatibleArrayInterface
end
-- error: "inherits incompatible array {boolean} and tuple {string, string}"
local record MyIncompatibleTupleArrayRecord is MySimpleTupleInterface, MyIncompatibleArrayInterface
end
------- Compatibility with union type arrays -------
local interface MyUnionArrayInterface is {boolean | string}
-- An array of {boolean | string} is compatible with {string, string}
end
-- warn: "inherits overlapping array {boolean | string} and tuple {string, string}"
local record MyUnionTupleArrayRecord is MySimpleTupleInterface, MyUnionArrayInterface
end
-- The typing of tuple fields overrides the typing of the array where applicable
local tuple_array_object: MyUnionTupleArrayRecord = {
[1] = "A",
[2] = "B"
}
-- error "in assignment: got boolean, expected string"
tuple_array_object[2] = true
-- valid assignments because the array interface accepts both booleans and strings
tuple_array_object[5] = false
tuple_array_object[6] = "G"
-- n1 has a (string) typing instead of (boolean | string) because it's within tuple bounds
local n1 = tuple_array_object[1]
-- n3 has a (boolean | string) typing because [3] is only an array index
local n3 = tuple_array_object[3]
|
|
Unrelated to the content of this pull request, but while I was writing the specs for tuple records I noticed that teal seems to consider negative integers as expressions instead of as constants: local object: {number, number, string} = {1, 2, "3"}
-- look at some constants
local x = object[1] -- All good, has (number) typing
local y = object[3] -- All good, has (string) typing
local var_a = object[4] -- error: index 4 out of range for tuple {number, number, string}
local var_b = object[0] -- error: index 0 out of range for tuple {number, number, string}
-- look at some expressions
local n: integer = get_some_integer()
local var_n = object[n] -- no error, has (number | string) typing
local var_x = object[(10 * 0 + 1)] -- no error, has (number | string) typing
local var_y = object[-1] -- no error, has (number | string) typing |
|
I believe all of the test specifications should now be in place |
|
@hishamhm it has been almost a week, is this still being considered? Are there any other changes that I need to make? |
oh, I think that's the first place where I saw this distinction mattering. Yes, I've been treating |
Yes, it is considered, definitely! I just haven't had the time to give it a proper review so far. |
|
This looks great! Next thing I think the next step would be to update the grammar.md document to match the syntax addition, but this can happen in a follow-up commit! Merging this, thank you! |
Also, if you'd like to do further code cleanups/changes, feel free to open a PR with them, and we can discuss whether to adopt them there! |
|
It's appreciated that you kept this particular PR focused on the functionality addition! |
I have implemented the functionality for recordlike types to have tupletable parent interfaces (#1029).
The basic syntax is very similar to how it is to define an interface to have an array parent interface:
If an recordlike type implements parents which are both tuples then it will attempt to combine the tuples, or error if the tuple types are incompatible:
I currently have opted for the most lenient behaviour when a recordlike type attempts to implement both an array superinterface and a tuple superinterface. Currently, a warning is generated and the tuple types take precedence over the array types where applicable.
I believe an improvement could be erroring by default, but letting the user manually override the error in some way; however, I wanted to avoid any unnecessarily large code changes in this pull request.
A couple pieces of my code could probably be made cleaner or more readable, but I had to leave them as-is to avoid significantly restructuring code unrelated to the tuple table interface addition (it would feel rude to forcefully inject my personal code tastes into places it isn't needed). I will leave the decision on that up to the maintainers who will actually be interacting with it.