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

feat(rome_analyze): add a codegen command for the analyzer #2646

Merged
merged 3 commits into from
Jun 9, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Jun 3, 2022

Summary

This PR adds a cargo codegen analyzer to automatically generate the analyzers.rs and assists.rs index modules in rome_analyze as well as the with_filter builder on RuleRegistry (through the impl_registry_builders macro), based on a list of analyzer rules generated automatically by listing all the files in the analyzers and assists directories

@leops leops temporarily deployed to aws June 3, 2022 09:19 Inactive
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 3, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2e10ac1
Status: ✅  Deploy successful!
Preview URL: https://c2dec2ad.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the CONTRIBUTING.md file with this new command.

@@ -0,0 +1,108 @@
use std::fmt::Write;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_analyzers.rs? (plural)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale was that this is generating the code for the analyzer (well technically the registry), but isn't touching the code for the analyzers / rules themselves

use quote::{format_ident, quote};
use xtask::{glue::fs2, project_root};

pub fn generate_analyzer() -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn generate_analyzer() -> Result<()> {
pub fn generate_analyzers() -> Result<()> {

@leops leops temporarily deployed to aws June 3, 2022 15:21 Inactive
Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you envision dynamic plugins/rules in the future?

@@ -1,3 +1,4 @@
mod flip_bin_exp;
//! Generated file, do not edit by hand, see `xtask/codegen`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we name this file generated or put into a generated directory so that it automatically gets "hidden" in reviews. Or add it to the list in the gitattributes file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's a bit complicated to rename just these files to generated since they're "index files" in the middle of other, hand-written modules (like the auto-generated mod.rs files in the formatter for instance). I could rely on a #[path] attribute to load the rename file but it's probably easier to just add entries in gitattributes

@leops leops temporarily deployed to aws June 7, 2022 12:23 Inactive
@leops
Copy link
Contributor Author

leops commented Jun 7, 2022

How would you envision dynamic plugins/rules in the future?

Overall these auto-generated factories are just convenience functions, internally the RuleRegistry is just holding onto an array of rules represented as function pointers, and more could be added dynamically if necessary

@leops leops merged commit 13ef9a0 into main Jun 9, 2022
@leops leops deleted the feature/analyzer-codegen branch June 9, 2022 13:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants