regal icon indicating copy to clipboard operation
regal copied to clipboard

Figure out best way to work with the AST in linter rules

Open anderseknert opened this issue 1 year ago • 1 comments

For the purpose of getting started, our rules currently leverage a (perhaps too) simple model of the input, where each linter rule iterate over rules, expressions and terms looking for whatever is relevant for the rule to enforce. This however means that each rule traverses almost the entire AST. While I don't think performance is a top concern at the moment, it probaly could be later when used in larger projects. A more immediate concern however, is how the "flat" iteration model will miss some significant parts of the AST, anytime another level of nesting is encountered. There are at least two (and possibly more I haven't thought of?) types of constructs where this happens:

  • comprehensions
  • the body of an every statement

Both of these create a new nested context, which in turn of course could contain even more nested comrehensions or bodies. While I don't think we'll need to worry too much about supporting "infinte" levels of nesting — as anything more than 2-3 levels would arguably be a code style violation itself, we'll definitely will want the linter to cover this code! How? Without recursion, this is non-trivial. Some options I can think of:

  1. Each rule use something like the walk builtin to traverse the AST — even more expensive.
  2. Have each rule declare its "dependencies" — i.e. the type of constructs that the rule applies to, like "function calls", "expressions", "imports" and so on... and Regal would prepare a "view" of the input based on that.
  3. Have regal run an "indexing" step before executing any rules, where the AST is traversed and groups are created for each type of construct, which the rules could then make use of.
  4. Maybe have Go code (via a custom builtin function or whatnot) provide a way to "flatten" nested constructs, or by other means allow linter rules to keep with the "simple" model, but have a tool to help with nesting. I'm not sure what that would look like though.

I think option 3 is appealing, as it would allow rules to deal only with the constructs that are relevant to them, and without having to know much about the context in which they're found. But we'd need to give this some more thought.

anderseknert avatar Feb 05 '23 22:02 anderseknert