JSONTestSuite test results

[l1bbcsg]

There's this Parsing JSON is a Minefield article which details various issues (mostly in edge cases) across different JSON parsers.

Its author produced a test suit to test various implementations. I've run this test suit against Clarinet (on node 9.6.1), results compared to native JSON.parse are below (successful tests not shown).

Generally, I think the results are good with Clarinet being pretty close to native implementation and never failing too hard. But there are two things it still does wrong: It does not fail on some wrong inputs and fails on legitimate results consisting of one literal (i.e. null). The former is pretty minor I think, while the latter may lead to compatibility issues.

I'm raising this issue more for the purposes of discussion. What do you think, should those be addressed?

Results screenshot

Legend

Snippet I used to run Clarinet (derived from their JSON.parse):

var fs = require('fs');
var path = process.argv[2];
var clarinet = require('clarinet');
var parser = clarinet.parser();

try {
	var data = fs.readFileSync(path, 'utf-8');

	parser.onerror = function(e) {
		throw e;
	}
	parser.write(data).close();
	
} catch (e) {
	console.log("--", e.message)
	process.exit(1);
}

process.exit(0);

Note that at the time of writing the test runner has a really weird bug preventing it from running which isn't hard to patch out though.

[dscape]

This is amazing, thank you so much for this contribution!! Unfortunately I stopped coding a while back and don't find myself with the time to do the corrections.

We need to find a new maintainer to the project to make these changes, and I'm unsure if the module is used enough for this to be a good idea!

[evan-king]

Apologies from me, I saw this issue but responding to it slipped through the cracks. I somewhat expected @dscape's response to strongly favor backwards compatibility (including permissive handling of non-compliant input), but if that's not the case I'll be happy to assist.

My own availability is a bit up in the air currently but may include some time to implement corrections. Either way however, I can definitely accept patches for them.

[dscape]

I think as long as we do a major semver release it should be fine to be compliant like this? Or perhaps we can classify the fixes in two buckets: * Bugs * Functionality that previously wasn't expected And fix all the bugs, in a new major release, and then implement a strict flag for the strict behaviour describe above?

…

On 17 May 2018 at 14:11, Evan King ***@***.***> wrote: Apologies from me, I saw this issue but responding to it slipped through the cracks. I somewhat expected @dscape <https://github.com/dscape>'s response to strongly favor backwards compatibility (including permissive handling of non-compliant input), but if that's not the case I'll be happy to assist. My own availability is a bit up in the air currently but may include some time to implement corrections. Either way however, I can definitely accept patches for them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#49 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAhgdlgGtRBhzCm9_UF5R5FH6T_Ke8Fks5tzXcOgaJpZM4TyunV> .

[corno]

For anyone interested: I forked clarinet into 'bass-clarinet' and that one complies to the full JSONTestSuite testset