Only treat schema bearing positions as identifier locations - #954

Fixes #949

Problem

SchemaStorage walks a document looking for id / $id so it can register subschemas under their canonical URI. It descends into every member it meets, so an $id written in a position that holds data rather than a schema gets registered as though it identified a schema. When that value collides with a real $id elsewhere in the document, the fake one can win, and a $ref to that URI then resolves to the wrong schema.

That is exactly what unknownKeyword.json in the official suite pins down: an $id buried under an unknown keyword must not shadow the real one.

Cause

scanMembersForSubschemas() skipped enum and const by name. That covers two data positions out of many: default, examples and any unknown keyword were still descended into.

Fix

Instead of listing the positions to avoid, which is open ended, the scanner now only descends into positions that actually hold schemas. Two classification lists sit beside the existing SCHEMA_MAP_KEYWORDS:

• SCHEMA_KEYWORDS, whose value is a schema (not, if, additionalProperties, and so on)
• SCHEMA_ARRAY_KEYWORDS, whose value is a list of schemas (allOf, anyOf, prefixItems, and so on)

Everything else is data, and an id inside it identifies nothing.

Members of a schema map (properties, $defs, definitions, ...) remain schemas whatever their name, which is what the existing $parentProperty check already guaranteed: a subschema legitimately named enum is still traversed.

The classification is deliberately draft agnostic, since SchemaStorage is never told which dialect it is reading. A keyword counts if it holds schemas in any draft. That is also the safer direction: treating a keyword unknown to the current draft as a schema position is simply the behaviour every keyword had before.

Tests

Six cases added to subschemaRegistrationProvider(). Four fail without the source change (default, examples, an unknown keyword holding a list, an unknown keyword holding a map). The remaining two guard against over narrowing in the other direction, checking that an id under not and under an items tuple is still registered.

The six now obsolete unknownKeyword.json entries in shouldNotYieldTest() are removed.

Notes

expandRefs() carries the same enum / const special case. I left it untouched: the fixture does not exercise it, and measuring confirmed the scanner change alone is sufficient. Glad to align the two walkers in a follow up if you would prefer they agree.

One caveat on how far I could verify this. JsonSchemaTestSuiteTest currently yields no tests at all, because the upstream suite added a tests/v1/ directory that DraftIdentifiers::fromConstraintName() rejects, and PHPUnit reports that as a warning while still exiting 0. I opened #953 about it. To exercise the unknownKeyword cases here I skipped v1 locally, which is not part of this PR. With the suite unblocked that way, this change takes the failure count from 364 down to 354, repairing 6 suite cases plus the 4 new unit cases, and breaking none.

PHPStan reports no errors. Thanks for the library, and for the very readable SchemaStorage, which made this one pleasant to chase. 🙂