fix(core): don't parse 'requires' from strings or comments or regexes

[markwhitfeld]

This resolves issue #2392.
During this PR I push the commit with the failing tests to showcase the issue, and then I push the commit to fix the issue. The initial issue was specific to minified files as described in the original issue, but as I dug deeper I found many edge cases that have been plaguing this feature previously. For example:

• Issue cjs extension falsely matches "require" strings #222 - the original issue reporting the problem
• Issue 'require(...)' calls erroneously parsed even within strings in CommonJS #311 attempting to address the 'require's in string issue
  • Attempted fix with PR Avoid parsing require("dep") inside strings in CJS #312 but later reverted due to RactiveJS and SystemJS 0.11.2 - Module process not declared as a dependency #322
• Issue RactiveJS and SystemJS 0.11.2 - Module process not declared as a dependency #322 where regex expressions were messing with the string detection
• Issue CommonJs loader does not ignore requires() in comments, if they appear at the last index of comments  #919 related to avoiding detecting 'require's in comments
  • This was fixed by PR Proposed fix for https://github.com/systemjs/systemjs/issues/919 #920, but regexes could still interfere with the comment detection

The initial code that excluded these checks from minified files, just for comments is here, but in a later commit the code checking strings was moved into this block as well. This was potentially done erroneously because comments are usually removed from minified files, but strings still remain in the code. My initial attempt to fix my issue just moved the string check out of the block which was skipped for minified files, but later my solution evolved as described below.

Update:
I eventually realised that the existing approach could be simplified by using a regex that would handle the mutual exclusivity of the different ways that JS code can have text that should not be parsed. This new approach has resolved the original issue as well as a number of known issues documented in the tests.
This approach covers what was attempted (and abandoned) with PR#326 for tokenisation, but leverages the mutual exclusivity inside the regex to optimise the matching.

I updated the benchmark code and found that the new way is more than twice as fast!

• Old way: 144ms for 1000 runs (on my machine)
• New way: 64ms for 1000 runs (on my machine)

[markwhitfeld]

NodeJS 4 tests are making me sad 😢. Sorry about the multiple force pushes.

[markwhitfeld]

Ok, looks like node 4 is actually broken at the v0.21.6 head. Removing node 4 from CI build.

[markwhitfeld]

Bug demonstrated here as can be seen in the failing tests added in the commit above:
https://app.travis-ci.com/github/systemjs/systemjs/jobs/568734711#L386

[markwhitfeld]

The commit above adds tests to prove that string literals are also not handled correctly and then that is is resolved by the improved regex approach.
See test output: https://app.travis-ci.com/github/systemjs/systemjs/jobs/568746162#L386

[markwhitfeld]

Ok, I'm sure happy with this now.
Better regex, simpler code, handles more cases as well as previous know issues.

[markwhitfeld]

@joeldenning I know that the 0.21.x branch is real legacy stuff, but hopefully this PR is clear enough for you to easily review and release a new 0.21.7 version with this fix.

[markwhitfeld]

Hooray! New approach is also more than twice as fast too!

[markwhitfeld]

I have picked up an issue with the regex detection regex.
This article details why this is hard:
https://localcoder.org/when-parsing-javascript-what-determines-the-meaning-of-a-slash