Should identifier declarations be respected in non-schema locations?

[jdesrosiers]

The following quote from the spec was brought up in a slack discussion.

From JSON Schema Core - Section 9.4.2

Similarly, a reference target under a known keyword, for which the value is known not to be a schema, results in undefined behavior in order to avoid burdening implementations with the need to detect such targets.

This brings into question a few of the tests in the suite. (There may be others. These are the ones I'm aware of.)

• https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/tests/draft2020-12/id.json#L211-L261
• https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/tests/draft2020-12/anchor.json#L84-L141
• https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/tests/draft2020-12/unknownKeyword.json

These tests require that identifier declarations ($id or $anchor) in locations not known to be schemas are ignored. However, the quoted passage above would seem to indicate that the behavior is undefined in this situation and therefore we should not have tests that require a behavior in this case.

Full Disclosure: My implementation doesn't pass these tests and I don't intended to fix it. Although the removal of these tests is in my best interest, I'm only interesting in getting to the bottom of what is required by the spec. I've been content to skip those tests and if we decide they actually do belong in the suite, I'll just continue skipping them.

[Julian]

Your reading sounds right to me seeing that line quoted. "Reference target" in this case is "the thing with $id in it" (not the schema with $ref in it, which is probably how I've read that paragraph un-carefully before) -- and the spec is saying "referencing that thing is undefined behavior", so it indeed would sound like to me they should go in optional unless someone points out what we're missing about that paragraph.

[gregsdennis]

I'm happy to put them in optional.

For the record, my implementation does pass these. (Not a boast, just a consequence of how I process schemas.)

[jdesrosiers]

@karenetheridge said in Slack,

I think the only change we could make is to remove those ref-to-nonschema tests entirely — leaving them in optional/ may be confusing, if it’s intepreted as “this is something you could implement but you don’t have to”.

I'm inclined to agree with Karen on this. Unless someone can point to something in the spec that indicates that this is expected-but-not-required behavior, rather than undefined behavior, then it probably doesn't belong in "optional" either.

[Julian]

Why would it appear in the spec? That isn't what optional means (c.f. of course the PR which clarifies what it means explicitly). Optional is any behavior that isn't required by the spec that an implementer may implement, and allows any implementers doing so to share the tests for such a thing rather than rewriting them themselves. These kinds of tests have been in the suite since literally day 1 when I wrote it, what possible benefit is there in removing them? There's no evidence anyone has been confused in the way indicated there, can you recall any case ever of someone showing up and saying "hey should I implement this, I see it in the folder"?

[gregsdennis]

This calls back to the idea that we need to split the tests according to requirement level. It sounds like a single "optional" is insufficient.

[jdesrosiers]

Fair enough. I've always ignored the optional tests, so I haven't kept up on what they're for. Sorry, I misunderstood the purpose.

[karenetheridge]

I don't think this is optional behaviour - I think we should (and we do, iirc) have non-optional tests that check that such identifiers are NOT recognized. Properties that look like identifiers in non-schemas are absolutely not keywords, they are just plain data.

[gregsdennis]

For reference, the conversation in Slack stemmed from #697.

Also from 9.4.2 (linked above):

Therefore, having a reference target in such an unrecognized structure cannot be reliably implemented, and the resulting behavior is undefined.

This is the sentence before the one that @jdesrosiers quoted. His pertains to known locations, and this pertains to unknown locations. Both indicate that the behavior is undefined.

Under @Julian's perspective that any undefined behavior that's actually implemented should be an optional test, I think this test specifically should be an optional test.

Other tests that only include direct references into unknown locations should be non-optional.

I think we should (and we do, iirc) have non-optional tests that check that such identifiers are NOT recognized.

I can't find any language in the spec that says implementations MUST NOT support this.

[mwadams]

This kind of optional test essentially defines an undefined behaviour for a particular implementation, for specific input of a kind we (sadly but inevitably) meet in the real world.

Is there any reason for not having the orthogonal optional tests that verify whether it does work in that particular implementation (in those specific circumstances)?

It feels like a set of tests that could live in their own folder in the "optional" hierarchy, and it allows consumers to track if "undefined" behaviour that they depend on for their real-world use case has changed in their implementation.

It also encourages implementers to understand when they are straying into the realms of the Undefined, and to understand the implications.

[Julian]

This kind of optional test essentially defines an undefined behaviour for a particular implementation, for specific input of a kind we (sadly but inevitably) meet in the real world.

(I'm not sure if by "particular implementation" you mean one specific concrete implementation of JSON Schema, i.e. mine, but I definitely don't think I'm the only one who has this behavior. Perhaps you mean one way of implementing JSON Schema, in which case I agree).

Is there any reason for not having the orthogonal optional tests that verify whether it does work in that particular implementation (in those specific circumstances)?
It feels like a set of tests that could live in their own folder in the "optional" hierarchy, and it allows consumers to track if "undefined" behaviour that they depend on for their real-world use case has changed in their implementation.

That's essentially what #590 does, or sets us up for, which simply needs more people saying they do or don't understand where files are ending up.

[mwadams]

(I'm not sure if by "particular implementation" you mean one specific concrete implementation of JSON Schema, i.e. mine, but I definitely don't think I'm the only one who has this behavior. Perhaps you mean one way of implementing JSON Schema, in which case I agree).

That's exactly what I mean, yes.

That's essentially what #590 does, or sets us up for, which simply needs more people saying they do or don't understand where files are ending up.

And yes - that's exactly the "more granular" situation I was thinking of. I hadn't spotted #590.

[jdesrosiers]

Properties that look like identifiers in non-schemas are absolutely not keywords, they are just plain data.

Unfortunately, I think the spec is a little more complicated than that. I think Section 9.4.2 is acknowledging that any reference target could be interpreted as a schema, but the behavior is undefined for locations not known to be a schema.

{
  "anyOf": [
    { "$ref": "#/$defs/foo/const" },
    { "$ref": "/$defs/foo" }
  ],
  "$defs": {
    "foo": {
      "const": { "type": "string" }
    }
  }
}

This is a valid schema, but it contains some undefined behavior. When /$defs/foo is evaluated, the value of const is just data, but when /$defs/foo/const is evaluated, it could be interpreted as a schema. The "type" property in const is sometimes just data and sometimes a keyword depending on the context in which it's being evaluated. The behavior here is technically specified as undefined, but I think it's expected that this would work. I think the intention of section 9.4.2 is to recognize that things get messy when identifiers are introduced and that's why the behavior is left undefined.

{
  "$id": "https://example.com/schema",
  "$ref": "/foo",
  "$defs": {
    "foo": {
      "const": {
        "$id": "/foo",
        "type": "string"
      }
    }
  }
}

In the previous example, there was no ambiguity about the target of the reference, but in this case, it could be argued that the reference target doesn't exist because the $id in const isn't a keyword until a reference resolves to that location and the reference can't resolve to that location until $id is a keyword. It's a chicken and egg problem.

What do you all think? According the spec (not any particular implementation), does this case fall under the "undefined" behavior specified in 9.4.2, or must this be interpreted as a bad reference? My take is that this should be considered undefined behavior. I think it's reasonable for an implementation to consider this a bad reference, but it's also reasonable for an implementation to identify /$defs/foo/const/$id as a possible reference target (even though it's in const) and allow $ref to target that location as a schema.

[Julian]

I agree with all of your above analysis.

[jdesrosiers]

@gregsdennis

@karenetheridge would you then expect only the tests which have pointer $refs that go into non-schema locations be moved to optional? That would leave any tests which use $id-based $refs in required.

I'm not following the distinction you're making here. Can you give an example?