Refactorings working towards nested destructuring

[matthewmcgarvey]

Related to #226

I don't want to drop a 1000-2000 PR, and I already have some questions so might as well pull out as much refactoring code as makes sense at this point.

A lot of this was pulled from #283 as I have been learning why it was written the way it was.

Hopefully these are good enough refactorings on their own aside from preparing for nested destructuring.

Changes

• extracted methods for getting destructuring variable in type_checkers/case_branch.cr since it will be moved to recursion to get nested variables
• Ast::EnumDestructuring, Ast::TupleDestructuring, and Ast::ArrayDestructuring all have parameters as [] of Ast::Node since they will hold more than one type for destructuring
• Extracted _compile methods for the destructurings as they will also have a recursive aspect for destructurings

Questions

• In type_checkers/scope.cr I'm not sure what that is doing and how that will support destructurings going forward. Is selecting only variables good enough? I'll have to dig into nested's once they're added I suppose?
• Type checking is not clear to my right now. Even with the current code I'm not sure how type_checkers/case_branch.cr does type checking. It kind of looks like type_checkers/case.cr is doing some Tuple counting. Is there a way we could refactor (if that is type checking) to a place where they could be called for nested destructurings?

Current Struggles

This is my first time working in Mint's compilers so everything's kind of new. Apart from that, some things that have caused me a hard time:

• A lot / most methods don't have a return type specified so when I change a return type the expected type vs got type compile-time errors are pretty confusing. Seeing an expected type that is the union of 3 Tuples with 5 items in each is really difficult to parse visually.
• I've been doing some "puts debugging" with tests and using the specs that read in the files are pretty handy but you have to write the name of the file you want into the spec if you want to run just one. Would it be possible to change the process that reads in the files and creates the specs to do it with a macro so that I could pass a line number to run a specific spec?
• No docs on anything in the compilers so a lot of times I have no idea what the thing is that I'm working with. For example, what's the condition here? I've just been ignoring anything I don't understand

[gdotdesign]

First of all thanks for putting your time in Mint! I really appriciate it 🙏

Also thank you for creating these detailed PRs 👍 they help a lot!

In type_checkers/scope.cr I'm not sure what that is doing and how that will support destructurings going forward. Is selecting only variables good enough? I'll have to dig into nested's once they're added I suppose?

That module is responsible for keeping track of the entities which are in scope. It's like a stack, for example given this code:

component Main {
  fun myFunction (input : String) : String {
    input
  }

  fun render : Html {
    <div>
      <{ myFunction("Test") }>
    </div>  
  }
}

the scope when resolving the body of the render function is:

Main component 
  render function

when trying to resolve the call to myFunction we search the scope (outward), this is where the find method comes in, it basically finds (matches) an entity (argument, function, computed property, etc..) to the variable string.

In this case it will try to find myFunction in the render function first (in where statements or arguments) then goes to the Main component where it finds it.

It has code for tuple destructuring because they can be used in statements where array and enum cannot, now that we are trying to do nested destructurings maybe it would be good do the matching here instead of the case branch by hand (let me know if doesn't make sense and I'll try to explain it further).

Type checking is not clear to my right now. Even with the current code I'm not sure how type_checkers/case_branch.cr does type checking. It kind of looks like type_checkers/case.cr is doing some Tuple counting. Is there a way we could refactor (if that is type checking) to a place where they could be called for nested destructurings?

type_checkers/case_branch.cr does the type checking against the condition (or target if it's a better name) of the case expression itself, while type_checkers/case.cr does the type checking on the branches overall - making sure that all possibilities are covered, etc.

A lot / most methods don't have a return type specified so when I change a return type the expected type vs got type compile-time errors are pretty confusing. Seeing an expected type that is the union of 3 Tuples with 5 items in each is really difficult to parse visually.

Yeah, I struggle with it some times, Crystal does not use aliases when an error happens as it would be a partial solution at least but I can understand that how difficult to achieve that. I'll try to add return types in the future. If you need any at the moment let me know and we'll add them.

I've been doing some "puts debugging" with tests and using the specs that read in the files are pretty handy but you have to write the name of the file you want into the spec if you want to run just one. Would it be possible to change the process that reads in the files and creates the specs to do it with a macro so that I could pass a line number to run a specific spec?

It's possible I guess, I'm not sure how to do it though as it would require extracting command line arguments from the spec command somehow, maybe @Sija has some ideas :D

No docs on anything in the compilers so a lot of times I have no idea what the thing is that I'm working with. For example, what's the condition here? I've just been ignoring anything I don't understand.

I guess that's fair, It's on me and I need to get better at it. If you have examples in other projects where its done right that would help me a lot in the future. Until then ask away and I'll answer and document along the way (probably on Discord or some other channel).

In this specific case the condition is the condition (or target) of the case itself case (condition) { ... }.

[Sija]

@matthewmcgarvey Please rebase on top of the current master to include CI-related fixes, thanks 🙏

[matthewmcgarvey]

@gdotdesign will this be merged or do you want me to keep committing to it?

[gdotdesign]

I'll merge it, I was just waiting for your input 🙂