WIP: Make convenience initializers with CodeGeneration

[natikgadzhi]

Summary

Work in progress, not ready for a full-on review, ideas and feedback welcome!

This pull request closes #1984. Instead of doing one-off initializers by hand, like in #2127 and #2112, we add a code generation clause that makes the initializers based on NodeInitRules.

Approach overview

• Node gets a new property: rules: [NodeInitRule].
• Each NodeInitRule has nonOptionalChildName and childDefaultValues: [String: Token].
• The generator makes initializers that take non-optional parameter nonOptionalChildName and set defaults to childDefaultValues.keys.
• There's an example of a rule in AttributeSyntax:

   rules: [
     NodeInitRule(
       nonOptionalChildName: "arguments",
       childDefaultValues: [
         "leftParen": .leftParen,
         "rightParen": .rightParen
       ])
   ],

Next steps

Proof of concept

• Generate the init declaration
• Generate the init documentation
• Generate the init body: call self.init with the parameters list.

Getting ready for the 1st review

• Write down a list of all nodes that will have convenience initializer in a separate commit
• Clean up the code
• Rebase and split into commits for easier review

Polish

• Add rules validation:
  • Check that nonOptionalChildName exists in node.children and that it is optional (otherwise the rule does not make any sense)
  • Check that each child in childDefaultValues exists in node.children
• Figure out how to test the initializers

[ahoppen]

I only glanced over it and the direction looks good. Before going into full-steam review on this, I would like to see a list of all the types that we want to apply rules to. Just to make sure that the rules are general enough to fit the needs and that there are sufficiently many that it’s easier to generate the code than to hand-write it.

[natikgadzhi]

@ahoppen, thank you! Yeah, I got excited, and first trying to make the end to end prototype and not care about how clean the code is.

Then, I'll post the list — I remember you're interested in looking at that first. I doubt I know all of the instances of where we'll want convenience initializers, so it's a possibility that the rules won't be flexible enough.

I think once I have the list and the prototype, I'll groom this for the first review. I expect about 10 initializers, but perhaps I will think of more on my own, too.

If at any point we decide to go with a different approach, no hard feelings. I'm here to learn 🙃

[ahoppen]

I think you’re doing a great job here and I’m all in favor of generating code instead of handwriting it.

[natikgadzhi]

@ahoppen, the proof of concept works! The code in this PR successfully generates one initializer for AttributeSyntax. Not ready for a review yet, just celebrating a little win 🥳

I've updated the initial comments with the list of next steps to emphasize getting the list of convenience init methods to generate first, then I'll clean up the code a little bit and split it into two commits — the infrastructure of the rules, and the list of actual nodes to process, so it's easier to review.

I'll take a day away from this to let this sit, I'll re-reading the code after will give me a fresh lens, I'll spot more mistakes.

[natikgadzhi]

@ahoppen, I've got some interesting findings!

• First off, I cleaned up the code a bit, and it's more tolerable now.
• I've added ConvenienceInitRules to replace my previous two PRs — so EnumCaseParameterSyntax and ClosureCaptureSyntax are now covered in generators instead of hand-written initializers, and those two work.
• Some others don't: MacroExpansionDeclSyntax and FunctionCallExprSyntax actually require their arguments to be non-optional, so while sure, we can add an initializer that also automatically sets the parentheses, we should not pass the arguments as optional to the default initializer.
  • Two possible paths forward: ignore them for now and do in a follow-up PR, or work in this one to improve the rule engine. I think that the initializer generator can check if the target child.isOptional originally to make things work.
  • Or we can get this to a state where we merge it, and improve later. I would recommend not doing that because seeing more edge cases will allow us to evaluate how good the rule engine is, and get it right from the start. But, my experience is limited 🙃

Next steps:

• The code needs a rebase
• Let's do a review of what I have. I am not entirely sure about naming, but I think it makes sense. Let's align on it.
• Once the code generally feels good, I will also add more rules for situations where colon is optional, and do that optionality check for the trigger argument. That's easy.
• If we can get this done in 1-2 weeks, I think that would be a good state to merge this to not spend too much time on rebasing often, and then just add rules as we see more situations where they are useful.

[ahoppen]

Some others don't: MacroExpansionDeclSyntax and FunctionCallExprSyntax actually require their arguments to be non-optional, so while sure, we can add an initializer that also automatically sets the parentheses, we should not pass the arguments as optional to the default initializer.

• Two possible paths forward: ignore them for now and do in a follow-up PR, or work in this one to improve the rule engine. I think that the initializer generator can check if the target child.isOptional originally to make things work.
• Or we can get this to a state where we merge it, and improve later. I would recommend not doing that because seeing more edge cases will allow us to evaluate how good the rule engine is, and get it right from the start. But, my experience is limited 🙃

I think it really depends how many cases like this we have. If MacroExpansionDeclSyntax and FunctionCallExprSyntax are the only two nodes where we want to add parentheses if a collection is empty, I think it’s totally fine to just hand-write the convenience initializers instead of setting up the infrastructure to generate them. If we find more cases like this and it’s, say, 5 or more, I think introducing a rule for that would make sense.