feat(irverify): check IDs, prim kinds, decimals and auth kinds - #319
Merged
Conversation
checkDeclaredIDs repeats ir.declaredID's predicate, and the comparison beside it cannot hold the copy in step: dropping either of the two narrowing clauses — the promoted-field guard or the named-string guard — leaves every test in this package green, corpus included. No Document separates them. Every promoted ID is a TypeID, which this rule skips as registry-keyed before the clause matters, and the IR declares no plain-string ID at all. TestDeclaredID_ClassifiesEachShape asks the predicate directly, over the four shapes it has to tell apart. Re-planting each mutation now reddens the subtest named for it. The clause's rationale is corrected with it. Counting promoted IDs would make ir.DeclaredIDs record a type twice, which is why ir carries the clause and why mutating it there reddens eleven tests; in this copy the registry skip gets there first, so the clause is fidelity with ir rather than an effect of its own.
OmarAlJarrah
added a commit
that referenced
this pull request
Aug 9, 2026
The version rule holds every hand-built document to carrying a stamp, and this branch stamped the fixtures that existed when it was written. #319 landed afterwards with new ones, so nine tests in ir/irverify failed once the two met: bigval_test.go and kinds_test.go build documents with no irVersion, and the new rule reports ir/ir-version-absent on each. Nothing warned. The merge is textually clean and the tree compiles; only running the suite shows it. Stamped the eight fixtures in those two files, for the reason the branch already gives for the ones it stamped: a fixture built to exercise one rule must contribute no violations of its own. The other unstamped documents in the package are left alone — their tests assert a particular violation is present rather than an exact set, so the stamp changes nothing there and adding it would be churn.
OmarAlJarrah
added a commit
that referenced
this pull request
Aug 9, 2026
The version rule holds every hand-built document to carrying a stamp, and this branch stamped the fixtures that existed when it was written. #319 landed afterwards with new ones, so nine tests in ir/irverify fail once the two meet: bigval_test.go and kinds_test.go build documents with no irVersion, and the rule reports ir/ir-version-absent on each. Nothing warned. The merge is textually clean and the tree compiles; only running the suite shows it. Resolved by stamping those eight fixtures, for the reason this branch already gives for the ones it stamped: a fixture built to exercise one rule must contribute no violations of its own. The package's other unstamped documents are left alone — their tests assert that a particular violation is present rather than an exact set, so the stamp would change nothing and adding it would be churn.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
irverifyis the oracle that holds a compiled document to the IR's structural invariants, and fiveof them were not stated anywhere. Each is a claim no other layer can make:
pass.Validatereportsspec problems, the JSON round-trip carries a bad value faithfully because most of them are strings,
and a document decoded from JSON or produced by a compiler outside this tree never meets the
constructors that would have refused it.
A primitive names a declared kind.
PrimKindis a bare string enum andir.PrimTypeIDderivesa consistent ID from any string, so an invented kind agrees with its own ID and reads as sound —
checkPrimIDssays as much, and asks a different question on purpose. Newir.PrimKind.Valid()andcheckPrimKinds, emittingir/unknown-prim-kind.An auth scheme names a declared mechanism.
Document.Authwas reached only by the rules thatread a key and an ID, so a scheme whose mechanism was empty or misspelled verified exactly like one
naming
oauth2. Newir.AuthKind.Valid()andcheckAuthKinds, emittingir/unknown-auth-kind.This is the class-level guard for the shape the OpenAPI compiler already refuses at the source; the
compiler-side refusal could not be regression-tested through
internal/harness, which returns atthe first error diagnostic before
Verifyruns.AuthScheme.Inand the OAuth flow URLs aredeliberately left alone: they are documented shapes rather than declared constant sets, and holding
them here would be re-validating the source.
Every numeric literal is one JSON reads back as a number.
ir.BigValis a defined string typewhose grammar is enforced only by
ir.NewBigVal, and it carries noUnmarshalJSON, so a conversionor a decode skips the constructor entirely. New
checkBigValsover everyBigValthe walk reaches—
Constraints.Min/Max/MultipleOfandValue.Numtoday — emittingir/bigval-not-numericfora value the constructor rejects and
ir/bigval-not-canonicalfor one it accepts but rewrites, sincea leading
+or a redundant leading zero is a number JSON does not admit. A bound is carried bypointer and so is present because something set it;
Value.Numis not, and is the zero string onevery value that is not a number, so the two are told apart rather than both skipped.
A node meant to have an identity carries one.
checkRegistryKeysreads an empty ID from aregistry key, and an operation, a service and a property have none;
ir.DeclaredIDsdrops anempty ID before the duplicate check could see it. New
checkDeclaredIDswalks for what thatderivation drops and reports
ir/empty-<noun>-id, the code the map-keyed classes already use.It covers
PropIDas well asOpIDandServiceID— a property's ID is minted from a sourcepointer like any other and every
PropIDreference in the document resolves against it, so leavingit out would have been a third class excluded by omission. The four classes
checkRegistryKeysalready covers are skipped so one defect stays one report, and which those are is read off
Document's own shape rather than listed.Two different properties do not share one
PropID. The uniqueness check skipped the wholePropIDclass, and the skip was wider than its reason: a component response materialized intoseveral operations legitimately repeats one ID, but two genuinely different properties went
unreported with the copies.
checkDuplicateIDsnow fingerprints the property — source name, wirename, and the ID its
TypeReftargets — and reports only when two declarations of one ID disagree.Nothing wider is read: copies agree on all three because they are copies, and two properties
agreeing on all three are indistinguishable to a consumer looking one up by ID, which is the only
thing a duplicate costs.
Both new
Valid()methods are tied to their const blocks by parsing theirsources, in the shapetypedef_completeness_test.goalready uses for theTypeKindsum, so a constant cannot be addedwithout joining the set. That parser was the one piece of
typedef_completeness_test.goworthsharing, so it moved to
helpers_test.goand grew a type parameter; no behaviour changed with it.The rules are additive and no committed golden moves. Two adjacent gaps found while writing them are
filed rather than fixed:
Naming.Aliasesis read by no rule at all (#317), and nothing rejects aUnionthat declares no variants (#318).Test plan
ir.Documentand a clean-documentcounterpart, since a check that cannot stay silent is no better than one that cannot fire.
BigValis planted at all six carrier positions separately and each asserted on its own path, andthe
PropIDfingerprint has one subtest per component so a component it stopped reading failsrather than narrowing what the check sees.
were checked the same way by planting a constant
Valid()does not know.testdata/conformance/openapi/component-reuse.yamlis the live negative control for thefingerprint: making the fingerprint differ per position reddens
TestVerify_Corpuson thatfixture and no other, so the carve-out is exercised rather than merely written down. Its three
copies are equal in every field today, so the corpus cannot hold the fingerprint to being narrow —
a dedicated test does.
TestCheckDeclaredIDs_ReachesEveryIDDeclaringNodecompares the new walk againstir.DeclaredIDson one fixture read two ways, so the two cannot disagree about which nodes exist.
TestDeclaredID_ClassifiesEachShapepins the copied predicate itself, which that comparisoncannot: dropping either narrowing clause leaves every Document-driven test here green, corpus
included, because no Document separates them — every promoted ID is a
TypeIDthis rule skips asregistry-keyed, and the IR declares no plain-string ID. Re-planting each mutation reddens the
subtest named for it.
gofmt,go vet,golangci-lint,go build, andscripts/check-coverage.shat100%.
Closes #240
Closes #280
Closes #282
Closes #289
Closes #295