Add "schemaPath" to verbose output, showing which subschema triggered each error.

[deBhal]

This PR adds a 'schemaPath' to the verbose output that can be used to get back to the failed rule in the schema in order to associate metadata with failures.

This provides a solution to https://github.com/mafintosh/is-my-json-valid/issues/102 and https://github.com/mafintosh/is-my-json-valid/issues/122 by making it possible to pull the appropriate additionalProperties or enum out of the schema.

For example, given this schema:

{   "id": "birds",
    "allOf": [
        {   "metadata": "ravens_are_black",
            "not": {
                "properties": {
                    "animal": { "enum": [ "raven" ] },
                    "color": {
                        "not": {
                            "enum": [ "black" ] } } } } },
        {   "metadata": "nested_string",
            "properties": {
                "outer": {
                    "properties": {
                        "inner": {
                            "metatdata": "deep",
                            "type": "string" } } } } },
        {   "metadata": "doves_are_white",
            "not": {
                "properties": {
                    "animal": { "enum": [ "dove" ] },
                    "color": {
                        "not": {
                            "enum": [ "white" ] } } } } } ] }

You can validate trace back failures to the and this test program:

const validator = require( './index.js' );
const birdsSchema = require('./bird-schema.json');

const badRaven = { animal: 'raven', color: 'rainbow', outer: { inner: 12 } };
const badDove = { animal: 'dove', color: 'pink' };

const validate = validator( birdsSchema, { greedy: true, verbose: true } );
const get = require( 'lodash/get' );

function printError( error ) {
    console.log( 'error:', JSON.stringify(error) );
    const failedRule = get( birdsSchema, error.schemaPath );
    console.log( 'failedRule:', JSON.stringify( failedRule ) );
    console.log();
}

validate( badRaven );
console.log('badRaven:');
validate.errors.map( printError );

console.log();

validate( badDove );
console.log("badDove:");
validate.errors.map( printError );

We can get the correct rule and it's metadata back out of the schema:

badRaven:
error: {"field":"data","message":"negative schema matches","value":{"animal":"raven","color":"rainbow","outer":{"inner":12}},"schemaPath":["allOf",0]}
failedRule: {"metadata":"ravens_are_black","not":{"properties":{"animal":{"enum":["raven"]},"color":{"not":{"enum":["black"]}}}}}

error: {"field":"data.outer.inner","message":"is the wrong type","value":12,"type":"string","schemaPath":["allOf",1,"properties","outer","properties","inner"]}
failedRule: {"metatdata":"deep","type":"string"}

badDove:
error: {"field":"data","message":"negative schema matches","value":{"animal":"dove","color":"pink"},"schemaPath":["allOf",2]}
failedRule: {"metadata":"doves_are_white","not":{"properties":{"animal":{"enum":["dove"]},"color":{"not":{"enum":["white"]}}}}}

The patch adds only the extra static string to the generated validation functions, so it should have virtually no effect on performance. (There's a very modest amount of extra work being done at compile time).

I added paths for anyOf and oneOf, but then I pulled it out again. I think it doesn't make sense to dive into them.

I've added paths to every call to visit, but I haven't exercised any of it, so they're probably wrong right now and need some tests.

[LinusU]

I'll give @mafintosh a few days to look, otherwise I'll merge it 👍

Feel free to ping me if (when) I forget 😄

[deBhal]

@LinusU: Thanks for looking at it so quickly! I wasn't quite finished :p

I've added some tests, fixed a bug they turned up, updated the docs and squashed, so I think I'm done now :)

I've just realized that squashing is going to make it harder for you to see the changes :( The only semantic difference is the bugfix, where I added added a check for tuple before pushing properties onto the path here: https://github.com/mafintosh/is-my-json-valid/pull/148/files#diff-168726dbe96b3ce427e7fedce31bb0bcR548 (this is because we re-use properties for arrays here, and so I was adding an incorrect properties field to the path before every array index).

I did also have a look at https://github.com/mafintosh/is-my-json-valid/issues/38 and https://github.com/mafintosh/is-my-json-valid/issues/22 which both talked about something like this, but I don't think it fits here.

When oneOf fails, whether it's because 2 rules passed or none you can't point to any of those sub rules and it was the wrong one. The failure is in the oneOf. Same with anyOf.

I'm a fan of the the feature those issues describe, and I'd be keen to see it implemented in some form, I'm just saying that I don't think it fits into the path.

[LinusU]

I've just realized that squashing is going to make it harder for you to see the changes :(

No problem at all 👌

I'm a fan of the the feature those issues describe, and I'd be keen to see it implemented in some form, I'm just saying that I don't think it fits into the path.

👍

[deBhal]

Feel free to ping me if (when) I forget 😄

I know that feeling so well. Ping :)

[LinusU]

Published as 2.17.0 🚀

Thank you for your contribution!

[groupsky]

node 5.12.0, same error: TypeError: Cannot read property 'concat' of undefined

[LinusU]

Shit, working on this!

[LinusU]

If anyone could give me a stacktrace I would appreciate it!

[jonanone]

@LinusU Check the open PRs regarding the issue #150 #151 #152 It will give you some hints for sure.

[LinusU]

Yeah, just say 😄

Fix published as 2.17.1, so sorry for any inconvenience caused!

[deBhal]

so sorry for any inconvenience caused!

Me too, Sorry! :'(

I feel pretty stupid about it having seen the fix - I added a bunch of tests but not the right ones :(

Breaking downstream packages is a first for me, and I'd love to never do it again - is there something I could/should have done to catch this ahead of time?

[LinusU]

I feel pretty stupid about it having seen the fix

Don't beat yourself up, I missed it too!

is there something I could/should have done to catch this ahead of time?

Not really, we should definitely look at getting code coverage up and running though, and I would love to migrate to TypeScript which would have caught this error as well. Other than that it's just trying to add as many tests as possible 😄