Linting tests

[brettz9]

• Fix: Ensure long unicode-specific check occurs before regular unicode matches it
• Refactor: Use constant ES6 template in place of accumulated let
• Refactor: Use switch to clarify paths are not successive
• Refactor: Avoid unreachable alternate branch for if (for coverage)
• Linting: Comment out unused variable; mixed tabs/spaces; missing semicolons and "use strict"
• Testing: Ensure Linting and coverage are run in test for CI (though provide mocha and nyc scripts for development)
• Testing: Temporarily Add thresholds matching current percentages (to ensure not regressing in coverage until we may get to 100%)
• Testing: Add (positive) unquoted keyword tests
• Testing: Invalid comments
• Testing: Incomplete Unicode wide escape

Btw, there seems to be one regression, though not reflected in tests. For some reason, when the package.json file is built, it adds a section about rollup-plugin-terser which I don't see any reason it should be built. But I'd appreciate if you could look at this short PR first as I think the testing procedures here ought to be helpful in preventing some regressions going forward (at least if CI is heeded; one could also add a commit hook to lint there if you like). Filed test for this in #33 which, if fixed, should fix this underlying package.json issue as well.

[brettz9]

I just added a couple unquoted keyword tests.

[brettz9]

also updated to add:

• Fix: Ensure long unicode-specific check occurs before regular unicode matches it
• Testing: Invalid comments
• Testing: Incomplete Unicode wide escape

[brettz9]

I've also just added:

• Testing: Add CR test which covers block but is failing

---

• Refactor: Avoid unreachable alternate branch for if (for coverage)

[brettz9]

Btw, were you saying that for line 452, if( hex_char_len < 2 || ( stringUnicode && hex_char_len < 4 ) ) {, the else path here couldn't be taken? (and I could therefore remove or comment out that check)?

[d3x0r]

Btw, were you saying that for line 452, if( hex_char_len < 2 || ( stringUnicode && hex_char_len < 4 ) ) {, the else path here couldn't be taken? (and I could therefore remove or comment out that check)?

I was not saying that... there was an octal check < 255 that was superfluous.

								hex_char_len++;
								if( stringUnicode ) {
									if( hex_char_len == 4 ) {
										val.string += String.fromCodePoint( hex_char );
										stringUnicode = false;
										stringEscape = false;
									}
								}
								else if( hex_char_len == 2 ) {
									val.string += String.fromCodePoint( hex_char );
									stringHex = false;
									stringEscape = false;
								}

the else if will happen, when the second hex (\x) character is found...

[d3x0r]

		it('should recover with carriage return escape at end of string', function () {
			var o = JSON6.parse( '"\\\r"' );
			console.log( "o is", o, typeof o );
---			expect(o).to.equal('\r');
+++			expect(o).to.equal('');
		});

the fixed cr_escape handling recognizes it was consumed....
The sorts of things that failed then were like
\\rA (because cr_escaped was still set and the A got eaten... it should be "A" now

[brettz9]

Thanks for the merge!

As far as the else... it is actually not visited despite having a two-character hex.

I think this is because, within that if( hex_char_len < 2 || ( stringUnicode && hex_char_len < 4 ) ) { block, there is this:

								else if( hex_char_len == 2 ) {
									val.string += String.fromCodePoint( hex_char );
									stringHex = false;
									stringEscape = false;
								}

By setting stringHex to false, when revisiting the block which contains both of these blocks, else if( stringHex || stringUnicode ) { will no longer be true, so the else path of the middle check will not be reachable.

[d3x0r]

That above was really because hex_char_len < 2 || ( stringUnicode && hex_char_len < 4 ) is always true...

							if( stringUnicode ) {
							}else if( hex_char_len == 2 ) {

doesn't complain.