Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Automatic code generation for formatter #1997

Closed
Tracked by #1996
ematipico opened this issue Jan 25, 2022 · 7 comments
Closed
Tracked by #1996

Automatic code generation for formatter #1997

ematipico opened this issue Jan 25, 2022 · 7 comments
Assignees
Labels
A-Formatter Area: formatter A-Tooling Area: our own build, development, and release tooling task A task, an action that needs to be performed

Comments

@ematipico
Copy link
Contributor

ematipico commented Jan 25, 2022

We should be able to create code automatically, every time we run the xtask syntax.

This task will be disruptive and will apply the following changes:

  • cst.rs will be automatically generated, the whole file. If a new node is created, this file should have a new node created (and imported)
  • the folder structure will change drastically and will have to follow the naming convention of the nodes. One file will have to implement one single AST node. No more, no less. This will be the golden rule that would allow us to generate nodes automatically.

This is my proposal:

Main folders

The root folder will be the language. For now we have all Js* nodes, they will live inside a js/ folder. Once we will introduce new languages, new folders will pop up:

  • ts/ folder for Ts* nodes;
  • jsx/ folder for Jsx* nodes;
  • etc.

Sub folders

All our nodes contains a "concept" in their name: statement, expression, object member, binding, etc. We could use those concepts as sub folders of the main folders. If a AST node name contains Expression, it will be placed inside the expression/ folder, if it contains ObjectMember, it will be placed inside object_member/ folder. And so on.

Any* element, JsAnyArrayElement

  1. the file will be inside the jsfolder
  2. the file will be inside js/any/ folder because it contains the Any keyword
  3. the file name will be array_element.rs and it will contain the following code:
use crate::{FormatElement, FormatResult, Formatter, ToFormatElement};
use rslint_parser::ast::JsAnyArrayElement;

impl ToFormatElement for JsAnyArrayElement {
    fn to_format_element(&self, formatter: &Formatter) -> FormatResult<FormatElement> {
        match self {
            JsAnyArrayElement::JsSpread(element) => element.to_format_element(formatter),
            JsAnyArrayElement::JsAnyExpression(element) => element.to_format_element(formatter),
            JsAnyArrayElement::JsArrayHole(element) => element.to_format_element(formatter),
        }
    }
}

Rest of elements

For the name of the file, I am a bit conflicted. I would personally leave the whole name.

  • JsDoWhileStatement => js/statement/do_while_statement.rs
  • JsReturnStatement => js/statement/return_statement.rs
  • JsComputedMemberExpression => js/expression/computed_member_expression.rs

Ideally we could remove the "concept" from their name, but I'm not sure if it could create more issues.

The generated file will contain

use crate::{FormatElement, FormatResult, Formatter, ToFormatElement};
use rslint_parser::ast::JsAnyArrayElement;

impl ToFormatElement for JsComputedMemberExpression {
    fn to_format_element(&self, formatter: &Formatter) -> FormatResult<FormatElement> {
        Ok(formatter.format_verbatim(self.syntax()))
    }
}
@ematipico ematipico changed the title Automatic code generation for formatterAt the moment, every time we create new grammar, we have to manually add those nodes to our formatter suite. It would be great if we can automate that part by creating the new stuff automatically Automatic code generation for formatter Jan 25, 2022
@leops
Copy link
Contributor

leops commented Jan 25, 2022

I opened an "RFC" draft PR that adds a tentative design for this in #1998

@ematipico
Copy link
Contributor Author

I opened an "RFC" draft PR that adds a tentative design for this in #1998

Does it tackle the first bullet point of the task?

@leops
Copy link
Contributor

leops commented Jan 25, 2022

It does so indirectly: cst.rs isn't autogenerated but calls a macros that is, to let this matching logic be reused across the codebase.

I haven't done anything to address the second part though, so in the meantime I still had to include a placeholder implementation of ToFormatElement on all the AstNodes that were missing one

@ematipico ematipico added A-Formatter Area: formatter task A task, an action that needs to be performed A-Tooling Area: our own build, development, and release tooling labels Jan 25, 2022
@leops
Copy link
Contributor

leops commented Jan 28, 2022

The match expression in cst.rs being complete is going to be a requirement for range formatting and format on type since the formatter needs to be able to start from any node in the tree, so I'll look into this next

@MichaReiser
Copy link
Contributor

The new code-gen is neat. I've a few usability suggestions

  • Automatically stage new generated files with git add (low)
  • Add a new line ending at the end of the generated files. Clippy doesn't like rust files without.
  • Delete outdated files. It's easy to end up with stale files when renaming a node because the old implementations are removed from the mod file and, therefore, the compiler won't generate any compile errors for the no-longer existing nodes.

@MichaReiser
Copy link
Contributor

@leops is there more work you plan to do on the codegen side or can we close this for the time being?

@leops
Copy link
Contributor

leops commented Mar 4, 2022

No there isn't any work I plan to do on the formatter codegen for now, I'll close this

@leops leops closed this as completed Mar 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter A-Tooling Area: our own build, development, and release tooling task A task, an action that needs to be performed
Projects
None yet
Development

No branches or pull requests

3 participants