Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if/then false positive when if-property is undefined. #2439

Closed
LazerJesus opened this issue May 23, 2024 · 5 comments
Closed

if/then false positive when if-property is undefined. #2439

LazerJesus opened this issue May 23, 2024 · 5 comments

Comments

@LazerJesus
Copy link

LazerJesus commented May 23, 2024

    "ajv": "^8.13.0",
    "ajv-errors": "^3.0.0",
    const ajv = new Ajv({ allErrors: true, verbose: true, $data: true });

I define this schema of an annotation object.

        annotation: {
            type: "object",
            properties: {
                pos: { ref:"" },
                verbform: { ref:"" },
                tense: { ref:"" },
                gender: { ref:"" },
            },
            required: [ "pos", "verbform"],
            allOf: [
                {
                    if: { properties: { verbform: { const: "fin" } } },
                    then: {
                        required: ["tense" ],
                        properties: { gender: { not: {} } }
                    }
                },
              ]
          }

I pass the following annotation: { pos: 'verb' }

I expect only 1 error of 'required verbform`.

but also i get this error:

{
  instancePath: '',
  schemaPath: '#/allOf/0/then/required',
  keyword: 'required',
  params: { missingProperty: 'tense' },
  message: "must have required property 'tense'",
  schema: [ 'tense' ],
  parentSchema: {
    required: [ 'tense', ],
    properties: { gender: [Object] }
  },
  data: { pos: 'verb', lemma: 'poder' }
}

meaning that the if: { properties: { verbform: { const: "fin" } } } is somehow true for { pos: 'verb' } ?!

@LazerJesus
Copy link
Author

LazerJesus commented May 23, 2024

Reproduction:

import Ajv from "ajv";

const ajv = new Ajv({ allErrors: false, verbose: true, $data: true });

const annotationschema = {
    type: "object",
    properties: {
        pos: {type: "string"},
        lemma: {type: "string"},
        verbform: {type: "string", enum: ["fin", "inf"]},
        tense: {type: "string"},
        gender: {type: "string"}
    },
    required: ["pos", "lemma", "verbform"],
    allOf: [
        {
            if: {properties: {verbform: {const: "fin"}}},
            then: {required: ["tense"], properties: {gender: {not: {}}}}
        }
    ]
};

const annotation = {
    pos: "verb",
    lemma: "something"
    // verbform: "inf" // toggle this to see the error magically disappear, even tho the if condition is just as false.
};

const validate = ajv.compile(annotationschema);
const valid = validate(annotation);
console.log(valid);
console.log(validate.errors);

@jasoniangreen
Copy link
Collaborator

Hi @LazerJesus, thanks for brining this to our attention. After playing around with your example for a bit it does indeed look like a bug to me. I can even pass an empty object and it still complains about missing tense. I will look into it.

@epoberezkin
Copy link
Member

schema in "if" says that when verbForm is present it should be "fin" but it doesn't require that it's present.

So in the absense of verbForm the schema in "if" passes validation, and schema in "then" is applied.

meaning that the if: { properties: { verbform: { const: "fin" } } } is somehow true for { pos: 'verb' } ?!

That's exactly how JSON schema spec is supposed to work 🤷‍♂️

If you make this property required, it will be "false", but it's not implied.

@jasoniangreen
Copy link
Collaborator

Ah yes of course, I was thrown by the fact that the outer schema has "verbform" in the required array, but the if schema is treated separately so will need it's own required.

        {
            if: {
                properties: {verbform: {const: "fin"}},
                required: ["verbform"],
            },
            then: {required: ["tense"]},
        }

@LazerJesus
Copy link
Author

Indeed that solves it. thank you for the information.

i must say tho, that its very counterintuitive and seems to conflict with logic.
since the statementif: { properties: { verbform: { const: "fin" } } } is true, the logical branch to execute is then.
adding the qualifier required doesnt, for me, change the proposition.
the verbform either is, or is not fin.

that being said, ill just add the qualifier and thats that ✌️ all the best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants