eslintabstract-syntax-treestatic-analysisautomated-refactoringast-grep

Can I reproduce eslint's "prefer-object-spread" rule using ast-grep?


I am trying to compare different linters and their performance with custom rules. Ast-grep is promising, and likely runs much faster than eslint, as it is written in Rust instead of javascript. However, eslint has a ton of established rules available and ast-grep has ... not so many. So I'm trying to get some experience and create some common ground between the two tools by writing a typical rule from eslint's catalog: "prefer-object-spread".

In general, "prefer-object-spread" will convert

Object.assign({},defaultConfig,customizedConfig) to {...defaultConfig,...customizedConfig}

I've mostly implemented this rule in ast-grep but I'm struggling to match top-level arguments (defaultConfig and customizedConfig) without also matching all of their descendants. There aren't any descendant nodes in the above simple example, but consider this example:

Object.assign({},defaultConfigGenerator(16))

or worse

Object.assign({},Object.assign(a,b))

I'm getting a result like {...defaultConfigGenerator(16),...16} because I'm matching the descendant node of 16 as well as its ancestor. I can't figure out how to use a meta variable to refer only to the nodes that are direct children of the arguments node and none of their descendants.

I've tried following the example from https://dev.to/herrington_darkholme/find-patch-a-novel-functional-programming-like-code-rewrite-scheme-3964 where we can match a meta variable to multiple AST nodes and transform each one of them.

The example shows how to make this conversion

// from
import {a, b, c} from './barrel';
// to
import a from './barrel/a';
import b from './barrel/b';
import c from './barrel/c';

With this ast-grep rule:

# Example barrel import rewrite rule
rule:
  pattern: import {$$$IDENTS} from './barrel'
rewriters:
- id: rewrite-identifer
  rule:
    pattern: $IDENT
    kind: identifier
  fix: import $IDENT from './barrel/$IDENT'
transform:
  IMPORTS:
    rewrite:
      rewriters: [rewrite-identifer]
      source: $$$IDENTS
      joinBy: "\n"
fix: $IMPORTS

So after reading this example many times I tried to apply the same principles to use a meta variable to match the arguments of Object.assign and transform them.

# First part of my rule
rule:
  pattern: Object.assign($FIRST_ARG_OBJ_LITERAL,$$$REMAINING_ARGS)
  has:
    kind: object
    stopBy: end
    pattern: $FIRST_ARG_OBJ_LITERAL

So far this matches the correct nodes and even properly enforces that the first argument has to be an object literal. The rest of the code is all about trying to modify the matched nodes properly. Here it is:

# Second part of my rule
rewriters:
- id: remove_empty_obj
  rule:
    kind: object
    not:
      has: 
        pattern: $ANYCONTENTS
  fix:
    ""

- id: add_ellipsis
  rule:
    pattern: $ANYCONTENTS
  fix:
    ...$ANYCONTENTS,

- id: argument_spreader
  rule:
    pattern: $ANY
    inside:
      kind: arguments
      inside:
        kind: call_expression
        pattern: Object.assign($$$REMAINING_ARGS)
  fix:
    "...$ANY"


transform:
  OPTIONAL_FIRST_OBJ:
    rewrite:
      rewriters: [remove_empty_obj,add_ellipsis]
      source: $FIRST_ARG_OBJ_LITERAL
  SPREAD_REMAINDERS:
    rewrite:
      rewriters: [argument_spreader]
      source: $$$REMAINING_ARGS
      joinBy: ","
  

fix:
  "{$OPTIONAL_FIRST_OBJ$SPREAD_REMAINDERS}"

The remove_empty_object and add_ellipsis rewriters just help handle the first argument. My issue is with the $$$REMAINING_ARGS that get fed into the argument_spreader rewriter. I expected the $$$REMAINING_ARGS to just consist of siblings to the first argument, but those nodes' descendants are included as well.

I'm able to reduce some of these excess nodes by insisting that we should only match nodes that are arguments and even arguments in an Object.assign call, but it doesn't handle the case if we have the nested Object.assign({},Object.assign(a,b)). There appears to be no connection between the $$$REMAINING_ARGS variable inside argument_spreader to the rest of the rule. Otherwise, I think it would work.

You can play around with this rule at the ast-grep playground to test any of your ideas.


Solution

  • ast-grep's rewriter is a new and experimental feature! Thank you for trying out and it is quite natural if you cannot figure it out. Let's break down the issue and see how it can be done.

    First, let's elaborate our goal. If the JavaScript has an Object.assign call using an object literal as the first argument, we want to change it to object spread syntax.

    The pattern below can achieve this goal.

    rule:
      pattern: Object.assign({$$$FIELDS}, $$$REMAINING_ARGS)
    

    The pattern above takes advantage that {$$$FIELDS} is an expression that can be readily parsed by tree-sitter. It only matches object literal and the fields inside are captured by the meta variable $FIELDS.


    Now let's try add add_ellipsis

    rewriters:
    - id: add_ellipsis
      rule:
        pattern: $ANYCONTENTS
      fix:
        ...$ANYCONTENTS,
    

    and then apply it to $$$REMAINING_ARGS

    transform:
      SPREAD_REMAINDERS:
        rewrite:
          rewriters: [argument_spreader]
          source: $$$REMAINING_ARGS
    

    I expected the $$$REMAINING_ARGS to just consist of siblings to the first argument, but those nodes' descendants are included as well.

    Rewriter will be applied to all descendants. However, a rewriter should only match the first node in the tree and stops rewriting its children. There is a bug that descendant nodes are repeatedly rewritten. And it is fixed here.


    Now let's assemble these pieces together! The most simple way is just spreading all objects.

    fix: '{...{$$$FIELDS}, $SPREAD_REMAINDERS}'
    

    See the playground


    Note we have not optimized the rewriting above. We also want to:

    1. Put object fields directly in the rewritten object
    2. Rewrite Object.assign inside of Object.assign recursively

    This is too long to be included in this answer. While it is possible to do it in YAML, using js-api may be a better choice.