Make parsers more easily pluggable

[nzakas]

I've been looking at some of the hoops we jump through (and babel-eslint jumps through) to get parsers to work in ESLint. It seems like we could dramatically improve things with just a few simple steps. Here's what I'm proposing:

1. Parsers should export a Syntax object that contains the names of all AST nodes they support. Esprima and Espree already do this.
2. Parsers should also export a VisitorKeys object containing the traversal information for the nodes they support. estraverse exports this currently.

When a custom parser is specified, ESLint will augment the Syntax and VisitorKeys estraverse objects with those that are exported from the parser itself, ensuring ESLint can work properly.

The benefits, as far as I can see them, are:

1. Custom parsers won't have to monkey patch estraverse to get traversal to work correctly (as babel-eslint does today).
2. The syntax information now lives in one place (the parser) rather than being split between the parser and ESLint. That means we can add more to Espree and ESLint will Just Work (tm).

This would also benefit typescript-eslint-parser, which is sure to have some nonstandard nodes to deal with.

Thoughts?

@hzoo

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[hzoo]

That would certainly help with patching up the visitorKeys - so instead of monkeypatching it in babel-eslint, it would export the visitorKeys and eslint would patch it instead? This would help to remove https://github.com/babel/babel-eslint/blob/master/index.js#L44-L62

[nzakas]

Yup! Exactly.

[hzoo]

Don't know if there is an easy way (or if you've thought of) doing the same for escope? There is a lot of patching done for flow, experimental features regarding no-unused-vars for that.

[nzakas]

I haven't done much thinking around that, as it seemed like traversal was the lowest hanging fruit. Do you have a suggestion?

[hzoo]

It's a similar thing but instead we have to patch the escope visitor methods somehow to also visit the nodes it doesn't know about (example with decorators)

function visitDecorators(node) {
    if (!node.decorators) {
      return;
    }
    for (var i = 0; i < node.decorators.length; i++) {
      if (node.decorators[i].expression) {
        this.visit(node.decorators[i]);
      }
    }
  }

And right it's definitely not as simple as traversal

[nzakas]

It looks like you're doing more than just adding methods to the referencer, though. I'm not sure there's an easy way to do this generically.

[hzoo]

Most of it is either visiting certain nodes by rewriting the function to do so and then calling the original referencer methods or creating a scope variable if needed. Maybe this is more for escope

[nzakas]

Yeah, it seems like more of an escope thing. Though, I wonder if we could go through and use VisitorKeys to create default visitors for things to at least get part of the way there. We can keep thinking about it going forward, though I would like to see escope become more extensible so we're not reaching into files and hacking things in.

[nzakas]

Closing in favor of #5476