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

specify if, then, and else together using annotations as interaction mechanism #1451

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 22 additions & 49 deletions jsonschema-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -1714,62 +1714,35 @@ against the schema defined by this keyword.

#### Keywords for Applying Subschemas Conditionally {#conditional}

Three of these keywords work together to implement conditional application of a
subschema based on the outcome of another subschema. The fourth is a shortcut
for a specific conditional case.
These keywords contain subschemas which are conditionally applied based on the
Copy link
Member

@Julian Julian Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this is a perfect example to me of #1445 (comment) (I didn't look at this PR first, promise :)

But to me, this makes things way less clear than what was here I think.

Just my opinion of course.

(EDIT: To be clearer on the review -- I'm -0 on the change, but if you choose to stick with the language from #1445 that I don't like, feel free to ignore the -0 and consider me OK with this regardless.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a comment on this line or the concept of the PR as a whole?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR as a whole, sorry (for being unclear).

Copy link
Member Author

@gregsdennis gregsdennis Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this change is actually to make the interaction of if with then and else more consistent.

All other keyword interactions (except contains/minContains, which I'm planning a PR for) are described using annotations as the communication mechanism. Implementations aren't required to actually do it that way, but it serves as a good description. That's what #1445 is about.

As they're currently defined, then and else depend on the evaluation result of if. Depending on an evaluation result of a sibling keyword doesn't exist anywhere else in the spec, and there's not a defined communication mechanism to describe that behavior. But there is such a mechanism using annotations. So redefining then and else to depend on the annotation result of if (which is the evaluation result) makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me -- again just me -- it is undoubtedly going to be clear to current readers of the spec what the intended behavior is, regardless of formality. It's the way any programmer really thinks about these things -- annotations are (I think incontrovertibly) not how anyone already knows if/then/else to work.

I do hear you on consistency -- my personal preference would be to go in the other direction and reduce the number of other keywords defined using annotations. Again though, it's a stylistic thing, I find things generally clearer when they're not mentioned, but it's obviously easier case by case to look at something and compare.

Copy link
Member Author

@gregsdennis gregsdennis Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my personal preference would be to go in the other direction and reduce the number of other keywords defined using annotations.

I'm open to this, but we'd need to either:

  • define that keywords can depend on the evaluation result of other keywords
  • describe this interaction in some other way

Currently, only "presence, absence, and value" dependencies are defined (which means I need to actually add "annotation result" to that list in #1445).

If we decide to backtrack on the annotations-as-communication-mechanism thing, I'd want to be sure that all keywords can be described without them. The difficult ones are unevaluated*.

I'm going to hold these PRs and start a discussion (linked in opening comment).

instance.

`if`, `then`, and `else` MUST NOT interact with each other across subschema
boundaries. In other words, an `if` in one branch of an `allOf` MUST NOT have an
impact on a `then` or `else` in another branch.
##### `if`, `then`, and `else`

There is no default behavior for `if`, `then`, or `else` when they are not
present. In particular, they MUST NOT be treated as if present with an empty
schema, and when `if` is not present, both `then` and `else` MUST be entirely
ignored.
These three keywords work together to implement conditional application of a
subschema based on the outcome of another subschema.

##### `if`
The value for each of these keywords value MUST be a valid JSON Schema.
gregsdennis marked this conversation as resolved.
Show resolved Hide resolved

This keyword's value MUST be a valid JSON Schema.

This validation outcome of this keyword's subschema has no direct effect on the
overall validation result. Rather, it controls which of the `then` or `else`
keywords are evaluated.

Instances that successfully validate against this keyword's subschema MUST also
be valid against the subschema value of the `then` keyword, if present.

Instances that fail to validate against this keyword's subschema MUST also be
valid against the subschema value of the `else` keyword, if present.

If [annotations](#annotations) are being collected, they are collected from this
keyword's subschema in the usual way, including when the keyword is present
without either `then` or `else`.

##### `then`
The `if` keyword produces an annotation which is the boolean validation result
of its subschema against the instance. The validation outcome of the `if`
keyword's subschema MUST NOT directly effect the overall validation result.
gregsdennis marked this conversation as resolved.
Show resolved Hide resolved

This keyword's value MUST be a valid JSON Schema.

When `if` is present, and the instance successfully validates against its
subschema, then validation succeeds against this keyword if the instance also
successfully validates against this keyword's subschema.

This keyword has no effect when `if` is absent, or when the instance fails to
validate against its subschema. Implementations MUST NOT evaluate the instance
against this keyword, for either validation or annotation collection purposes,
in such cases.

##### `else`

This keyword's value MUST be a valid JSON Schema.
The `then` and `else` keywords each consume the annotation from an adjacent `if`
keyword and conditionally apply their subschemas to the instance as follows:

When `if` is present, and the instance fails to validate against its subschema,
then validation succeeds against this keyword if the instance successfully
validates against this keyword's subschema.
- When the annotation from the `if` keyword is `true`, the `then` keyword's
subschema MUST be applied to the instance and the `else` keyword's subschema
MUST be ignored.
- When the annotation from the `if` keyword is `false`, the `else` keyword's
subschema MUST be applied to the instance and the `then` keyword's subschema
MUST be ignored.
- When there is no adjacent `if` keyword, no annotation can be produced, so both
the `then` and `else` keywords' subschemas MUST be ignored.

This keyword has no effect when `if` is absent, or when the instance
successfully validates against its subschema. Implementations MUST NOT evaluate
the instance against this keyword, for either validation or annotation
collection purposes, in such cases.
If [annotations](#annotations) are being collected, they are collected from the
subschemas which are evaluated, including from the `if` keyword's subschema when
it is present without adjacent `then` or `else` keywords.

##### `dependentSchemas`

Expand Down
Loading