feat(xtask): automatically generate documentation pages for lint rules

[leops]

Summary

This PR adds a new cargo lintdoc automatically generating markdown documentation pages on the website for all registered lint rules

Internally this command relies on the content of the doc-comments of each lint rule that's now available as a runtime metadata alongside the name of the rule itself. A new metadata() method is available on the RuleRegistry to get the name and documentation of each active rule, and this method is aliased as the plain function rome_js_analyze::metadata() for the default instance of the JS rule registry.

The xtask_lintdoc crate makes use of this metadata by parsing the content of the comments as markdown, running the detected code blocks through the analyzer (similar to the "doctest" feature of rustdoc), and inserting snapshots of the emitted diagnostics if the analysis returned errors (using a newly-introduced HTML printer for rome_console markup)

Test Plan

At the moment I've only run the new generation command manually, but it will probably need to be integrated to the automated test suite to ensure the generated files are kept up to date

[ematipico]

Add documentation to the remaining rules

I believe this can be done afterwards, we just need to create an issue and we should be fine.

[cloudflare-pages]

Deploying with    Cloudflare Pages

Latest commit:	60146c0
Status:	✅  Deploy successful!
Preview URL:	https://a4e9b1a6.tools-8rn.pages.dev
Branch Preview URL:	https://feature-xtask-lintdoc.tools-8rn.pages.dev

View logs

[github-actions]

Parser conformance results on ubuntu-latest

js/262

Test result	main count	This PR count	Difference
Total	45878	45878	0
Passed	44938	44938	0
Failed	940	940	0
Panics	0	0	0
Coverage	97.95%	97.95%	0.00%

jsx/babel

Test result	main count	This PR count	Difference
Total	39	39	0
Passed	36	36	0
Failed	3	3	0
Panics	0	0	0
Coverage	92.31%	92.31%	0.00%

symbols/microsoft

Test result	main count	This PR count	Difference
Total	5946	5946	0
Passed	350	350	0
Failed	5596	5596	0
Panics	0	0	0
Coverage	5.89%	5.89%	0.00%

ts/babel

Test result	main count	This PR count	Difference
Total	588	588	0
Passed	519	519	0
Failed	69	69	0
Panics	0	0	0
Coverage	88.27%	88.27%	0.00%

ts/microsoft

Test result	main count	This PR count	Difference
Total	16257	16257	0
Passed	12393	12393	0
Failed	3864	3864	0
Panics	0	0	0
Coverage	76.23%	76.23%	0.00%

[github-actions]

Playground for commit a4163cd

[MichaReiser]

Can we write some documentation for the "documentation" structure and what syntax is allowed. That documentation could either be part of a README or as part of declare_rule?

[leops]

I'd probably include it in the documentation for declare_rule as well as the (future) section on writing lint rules in the contribution docs, but the thing is I'm not entirely sure how should actually work anyway. Currently I've implemented the valid / invalid logic by detecting sections of the markdown document with the id valid and invalid, but I'm also considering having the expected status of a code block as an attribute on the language tag instead (for example ```js,should_fail) similar to rustdoc

[MichaReiser]

My understanding is that these pages get deployed to the website. Is there already an entry point? If so, what's your plan on handling the missing documentation?

[ematipico]

The entry point for now has been deleted but we can easily restore it: https://github.com/rome/tools/pull/2185/files#diff-1010593bf20dbee354fd42eb016d4589c20308e350009f7c013b1919e04ff0f5

The file was also auto generated

[leops]

For the entry point I've added a link to this lint rules index page in the linting section of the main docs similarly to what existed in the JS version, but maybe it would also make sense to it to have an entry in the navigation menu ?
As for the content of the pages, the new generation script generates the individual rule documentation pages and the rules index page at the same place and with the same code as what used to exist in the JS version of Rome, including the "MISSING DOCUMENTATION" message. But I could easily change the syntax of the declare_rule macro to make the documentation mandatory, and fail to build if there is no doc-comment on the rule

[MichaReiser]

I think it becomes important to add additional checks to our CI pipeline to make sure that this command is run for all PRs to prevent an outdated homepage. This isn't an issue specific to this command but common to many of our xtask commands.

One way we could accomplish this is by adding a CI step that generates the formatter, grammar, lint rules, etc. and verifies that thereafter are no git changes.

[leops]

I've added a "Codegen" step to the CI checks run on pull requests, it runs the grammar, analyzer and lintdoc codegen commands before checking the git status is unchanged. A test / example of failing run can be found here: https://github.com/rome/tools/runs/6900285745?check_suite_focus=true

[MichaReiser]

I love it! Thank you @leops for following up on this.

[MichaReiser]

What's the code size impact of including the markdown of all rules as part of the linter crate? Do you plan to expose the documentation as part of the CLI?

[ematipico]

What's the code size impact of including the markdown of all rules as part of the linter crate? Do you plan to expose the documentation as part of the CLI?

Probably that's what we want. It would help the usage of our CLI when people are offline

[leops]

What's the code size impact of including the markdown of all rules as part of the linter crate? Do you plan to expose the documentation as part of the CLI?

At the moment I don't expect it to be too significant, obviously that might change if we start to have many rules with extensive documentation. But yes the point of including the documentation in the binary is to allow it to be rendered by the CLI (I might put it behind a feature flag so it can be turned off in the LSP), although at the moment it would also require embedding a markdown parser to turn that code into markup.
One way of solving this is turning declare_rule into a procedural macro to perform that transformation at compile time instead, but it might also be a temporary "problem" if we eventually integrate support for the markdown language as part of the larger toolchain anyway

[ematipico]

Is this correct? Should it be js?

[leops]

The language ID emitted in the documentation page is the one used by the documentation generator and may not be exactly the same one specified in the original input: in this case the change is caused by .js files being interpreted as JSX here.

[ematipico]

Some feedback: https://feature-xtask-lintdoc.tools-8rn.pages.dev/docs/lint/rules/

If you check the description of a rule, the code block is not correctly rendered. But, if you open the rule page, it is correctly rendered: https://feature-xtask-lintdoc.tools-8rn.pages.dev/docs/lint/rules/noNegationElse/

[ematipico]

Also, should we allow people to see the rules now? The autofix hasn't been released yet and the new rules too, so I am not sure if it makes sense?

[ematipico]

This page rendered in a weird way: https://feature-xtask-lintdoc.tools-8rn.pages.dev/docs/lint/rules/noImplicitBoolean/ (it actually rendered the HTML instead of using it as a text)

[leops]

This page rendered in a weird way: https://feature-xtask-lintdoc.tools-8rn.pages.dev/docs/lint/rules/noImplicitBoolean/ (it actually rendered the HTML instead of using it as a text)

This should be easy to fix, I just need to escape HTML special characters in the markup writer the same way it's done in the terminal writer

If you check the description of a rule, the code block is not correctly rendered. But, if you open the rule page, it is correctly rendered: https://feature-xtask-lintdoc.tools-8rn.pages.dev/docs/lint/rules/noNegationElse/

That's interesting, in the JS version the description for the rules also included markdown in a similar way, so I'm not exactly sure of I'm missing that makes the generator for the website ignore this markup

Also, should we allow people to see the rules now? The autofix hasn't been released yet and the new rules too, so I am not sure if it makes sense?

Indeed having auto-generated documentation from the latest revision of the code is going to clash with the workflow we currently have of holding off update to the documentation until the corresponding release is ready.
While the linter is unstable it may not be too much of a problem since the main intended use-case for these documentation pages is explaining to users why they are seeing certain lint diagnostics, so if they're using a version of the analyzer that doesn't have those rules they wont need this documentation in the first place.
We could also display which version of the toolchain introduced each rule in the documentation, but for the long term it might make sense to expose archives of past and future versions of the full documentation website along with the current one

[ematipico]

Indeed having auto-generated documentation from the latest revision of the code is going to clash with the workflow we currently have of holding off update to the documentation until the corresponding release is ready. While the linter is unstable it may not be too much of a problem since the main intended use-case for these documentation pages is explaining to users why they are seeing certain lint diagnostics, so if they're using a version of the analyzer that doesn't have those rules they wont need this documentation in the first place. We could also display which version of the toolchain introduced each rule in the documentation, but for the long term it might make sense to expose archives of past and future versions of the full documentation website along with the current one

We can definitely come up with a solution. Definitely not in this PR. Thank you for the insights!