Figure out the next step toward plugin system.

[sebv]

See the ugly (but working) #2077 PR to get an idea what is needed. @rwaldron was mentioning the need for a common design effort.

In short we need to expose a lot of internals, plugins would need to do stuff like registering prefix, statement call error/warnings, so the question is what is the best way to do that?

[sebv]

On top of my head I see 2 options:

1/ expose an plugin hook similar to api within the itself object.
2/ expose method directly on the JSHINT object.

[caitp]

Hooks you need:

1. scanner hooks
2. Hooks for each production

Other things you need:

1. Linting rules rewritten as items which match patterns of productions (if new productions are added, they don't break the existing lobbying rules)

At least, in my head, that's the most elegant and maybe workable solution, using the fewest hacks.

[sebv]

What is a production?

[caitp]

A symbol representing a sequence of symbols scanned from the source code, part of the formal grammar of the language.

To add new parsing rules, you'd need to add rules for new productions. That would be easily pluggable. To add new linting rules, you'd add new rules for generating errors from grammar productions (or more complex analysis). I'm not sure there is a better way to make linting pluggable.

For simple rules, you could even define a domain specific Language for creating the rules for generating errors.

It would be a pretty substantial rewrite of the current architecture, but we've discussed it a bit

[sebv]

@caitp sounds good, but how long is this going to take? A couple of things that struck me when I wrote the async extension:

• We need prefix("async") and stmt("async"), when both seem to do the same thing (at least for async case). The issue here seem to be that Jshint doesn't look enough token ahead.
• The whole state.tokens.curr/state.tokens.next, being overuled by peek, sounds like state.tokens functionality should be extended.
• The whole fat arrow logic seems out of place, It may be possible to put the the function detection in one place (same thing, probably have to look more than one token ahead.).

[caitp]

We need prefix("async") and stmt("async"), when both seem to do the same thing (at least for async case). The issue here seem to be that Jshint doesn't look enough token ahead.

Arguably, this is another reason to move away from the current parsing strategy ;) It's hard for people to understand, and potentially leads to code duplication.

The whole state.tokens.curr/state.tokens.next, being overuled by peek, sounds like state.tokens functionality should be extended.

Well sure, but ideally you shouldn't be dealing with tokens directly anyways.

The whole fat arrow logic seems out of place, It may be possible to put the the function detection in one place (same thing, probably have to look more than one token ahead.).

I believe the handling of arrow functions has already been unified by @jugglinmike, although I didn't pay much attention to his patch, so it might not be as unified as I believe.

[sebv]

I believe the handling of arrow functions has already been unified by @jugglinmike, although I didn't
pay much attention to his patch, so it might not be as unified as I believe.

What I meant is that it is not unified with the function logic. Should be possible to write some sort of centralized and extendable detectFunction method.

[caitp]

I understood you ;) I don't think it's necessarily worth doing that. There are two cases which can be arrow functions: BindingIdentifier => and ( arguments... ) =>, so there are two places to detect arrow functions. Processing of the arrow function is done in a central place (the same as regular functions), so it's really not that bad

[sebv]

@caitp yeah I think this is what is fundamentally wrong in a lot of places the current code, should be only in one place, because when it combine with other patterns, you have to multiply by 2 so complexity increases quickly.

[sebv]

FYI, this is the raw api I've extracted for the asyncAwait code (The part exposed to the actual plugin implementation). It's crap (I have to do with the current code), but it works and passes all the tests. I am going to iterate on this and see where it goes.

    var pluginApi = {
      addlabel: addlabel,
      advance: advance,
      doFunction: doFunction,
      blockstmt: blockstmt,
      error: error,
      expression: expression,
      funct: function () { return funct; },
      inblock: function() { return inblock; },
      isEndOfExpr: isEndOfExpr,
      nobreaknonadjacent: nobreaknonadjacent,
      nolinebreak: nolinebreak,
      optionalidentifier: optionalidentifier,
      peek : peek,
      prefix: prefix,
      warning: warning,
      warningAt: warningAt
    };