feat(regular_expression): add terms visitor to pattern

[camchenry]

To make it easier to iterate over the nodes in the regex AST, I attempted to add a pseudo-visitor method for patterns

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[camchenry]

• fix(linter): include actual span size in no-regex-spaces diagnostic #5957
• refactor(linter): use regex parser in eslint/no-regex-spaces #5952
• feat(regular_expression): add terms visitor to pattern #5951 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @camchenry and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #5951 will not alter performance

_{Comparing 09-21-feat_regular_expression_add_terms_visitor_to_pattern (cfb8510) with main (05f592b)}

Summary

✅ 29 untouched benchmarks

[leaysgur]

@DonIsaac I am not familiar with this area...! 😅

Should these be handwritten? Or we need some updates for tasks/ast_tools?

[camchenry]

I feel like this might be the wrong implementation, but we probably do need some sort of regular expression AST visitor. I think a better approach here would be to create a generic AST visitor that can visit every node type in the Regex: Term, Disjunction, Alternative, etc.

I'm going to mark this as draft for now

[leaysgur]

create a generic AST visitor that can visit every node type

I agree.

For the main oxc_ast ASTs, it seems adding #[ast(visit)] instructs tasks/ast_tools to automatically generate the visit_ implementation (sorry, maybe I'm wrong).

However, I don't know the details of how it works or whether we should do the same here.

@Boshen Would you please help me to review? 🙏🏻

[Boshen]

We can always start with a manual one to test the water, and then migrate to ast_tools.

[leaysgur]

@camchenry If that's the case, I think there's no problem.

Just 1 point, I think it would be nice to separate the file as visit.rs with comments referring to ast_tools.

[camchenry]

closing this in favor of #6055, which is the better implementation I think