Introduce alternative to SkippableTokenAwareRuleWalker and scanAllTokens

[ajafff]

PR checklist

• Addresses an existing issue: comment-format TSX false positive #2007
• New feature, bugfix, or enhancement
  • Includes tests
• Documentation update
• Enable CircleCI for your fork (https://circleci.com/add-projects)

What changes did you make?

Scanning the whole source file is very fragile, because the grammar is not context free. You had to skip certain ranges, that were known to cause trouble. For example regex literals or template expressions.
This approach is prone to errors, as it didn't cover all cases and has to adopt new grammar as the language adds it.

The new function hands the scanning and parsing off to the parser and only iterates over the tokens of all AST nodes. Skipping ranges is no longer necessary.

This fixes #2007 and replaces #2009 (I took 2 commits from that PR to include the contained tests)

For more information please refer to the description of each individual commit.

Is there anything you'd like reviewers to focus on?

This is a rather big PR. I could split it into several smaller ones if you want.

WhitespaceRule used to skip TemplateExpression, JsxElement and JsxSelfClosingElement. That's no longer necessary.
That means not all whitespace errors were found with the previous implementation. Now we find all errors, even inside JsxExpression and TemplateExpression.
If you think this is a breaking change, I would fall back to skipping the aforementioned nodes for now and submit a separate PR to include them for the next major version.

I'm currently investigating what the performance impact of this change is. I'll keep you posted.

[ajafff]

Performance

Linting tslint's code with itself, repeated several times.

master: 10.9 sec to 11.4 sec
this branch: 9.7 sec to 10.0 sec

Seems like a 10% overall speedup as a side effect of a bugfix isn't that bad, is it?

Where's that coming from?

• We no longer need SkippableTokenAwareRuleWalker. See Simplify scanning with skippable tokens #2006 for some perf numbers
• node.createChildren only needs to rescan "synthetic tokens" - tokens that are not included in the AST - instead of scanning the whole SourceFile.
• node.getChildren only calls node.createChildren once and then reuses this array. So EnableDisableRuleWalker does the "heavy lifting" of creating those arrays for the whole AST. As long as the AST is not recreated (e.g. because a fix was applied), all rules using node.getChildren (and similar methods like getChildAt, getFirstToken, ...) benefit from the cached child array.

[nchen63]

this is pretty amazing

[nchen63]

as far as the new errors it finds, I'd consider those as fixes which aren't breaking. @adidahiya for comment

[nchen63]

looks good, minor nits

[nchen63]

tivia -> trivia

[nchen63]

doesn't belong here, move inside hasCommentAfterPosition or just use anonymous callback

[nchen63]

@ajafff thanks!