Feature/inner destructuring #283
Conversation
|
@teggotic Ameba warnings need to be fixed, see failed CI builds. |
There was a problem hiding this comment.
Thanks for the PR 👍
There are some larger issues:
- Tuples can be destructured in statements (
try,where, etc...) but enums and arrays can't so we need to restrict the parser for this to only allow type variables - The type checker will correctly check things at the first level but not at lower levels, for example this
[a, b, [...x,...y,...z], ...c] => { }would not raise an error. We need to do this recursively as well.
| @@ -4,6 +4,8 @@ module Mint | |||
|
|
|||
| alias TypeOrVariable = Type | TypeVariable | |||
|
|
|||
| alias DestructuringType = ArrayDestructuring | EnumDestructuring | TupleDestructuring | |||
There was a problem hiding this comment.
This is not necessary and should be replaced with Node
| @@ -1,9 +1,11 @@ | |||
| module Mint | |||
| class Ast | |||
| class EnumDestructuring < Node | |||
| alias EnumDestructuringParameter = DestructuringType | TypeVariable | |||
|
|
||
| statements << "const #{js.variable_of(node.items.select(Ast::Spread).first.variable)} = __" | ||
| statements | ||
| condition = "Array.isArray(#{condition_var_name}) && #{condition_var_name}.length >== #{node.items.size - 1}" |
There was a problem hiding this comment.
This can be condition = if makes it easier to read.
| @@ -1,36 +1,64 @@ | |||
| module Mint | |||
| class Compiler | |||
| def _compile(node : Ast::ArrayDestructuring, value) | |||
| private macro compile_item!(initial_var, item = item, index = index1) | |||
There was a problem hiding this comment.
Why does this needs to be a macro?
| @@ -31,5 +31,8 @@ module Mint | |||
| input: data) | |||
| end | |||
| end | |||
|
|
|||
| def parse_destructuring_item | |||
| @@ -11,7 +12,16 @@ module Mint | |||
|
|
|||
| option = type_id! EnumDestructuringExpectedOption | |||
|
|
|||
| parameters = many { type_variable }.compact | |||
| parameters = (many do | |||
There was a problem hiding this comment.
I think we should have only one syntax, in this case with parentheses. This will be a breaking change, but would fix the else branch since there it's not possible to inner destructure.
There was a problem hiding this comment.
Maybe we want to have a comma between parameters?
|
|
||
| scope(variables) do | ||
| scope(variables.not_nil!) do |
There was a problem hiding this comment.
I suggest to_a instead of not_nil!
| @@ -33,6 +33,12 @@ module Mint | |||
| "option" => option, | |||
| "node" => param, | |||
| } unless option.parameters[index]? | |||
|
|
|||
| case param | |||
| when Ast::EnumDestructuring | |||
There was a problem hiding this comment.
Other branches should be resolved as well, since they can cause other errors.
|
@teggotic Hi, are you willing to continue working on this? |
You can take it |
|
Superseded by #593 |
No description provided.