Skip to content

irverify: two different properties minted on one PropID go unreported #280

Description

@OmarAlJarrah

Summary

irverify.checkDuplicateIDs holds every declared identity to being declared once, except
ir.PropID, which is skipped outright. The skip is there for a real reason, but it is wider than
that reason, and what it leaves out is the same defect class the check exists for.

Why the skip exists

A Response is embedded by value rather than interned in a registry. Since #107 the PropID of a
response header is derived from the declaration's pointer, so a component response referenced from
several operations materializes into each of them carrying the one ID its defining occurrence
derives. testdata/conformance/openapi/component-reuse.yaml is the live instance —
p/openapi/components/responses/Listed/headers/X-Rate-Unit appears at three paths:

services[0].groups[0].operations[0].responses[0].headers[0]
services[0].groups[0].operations[1].responses[0].headers[0]
services[0].groups[0].operations[2].responses[0].headers[0]

Those three are copies of one declaration. A lookup for that ID is unambiguous, and reporting them
would fail every document that reuses a component. That much the skip gets right.

What it leaves unheld

Two different properties minted at one PropID are equally unreported, and that is a genuine
ID-minting defect — invariant 3's "two nodes on one pointer" at property granularity. Built as a Go
document and run through both checkers:

doc := &ir.Document{Types: ir.TypeRegistry{
    "t/A": &ir.Model{TypeCommon: ir.TypeCommon{ID: "t/A", Name: ir.Naming{Source: "A"}},
        Properties: []ir.Property{{ID: "p/dup", Name: ir.Naming{Source: "alpha"}, Type: ir.TypeRef{Target: "t/A"}}}},
    "t/B": &ir.Model{TypeCommon: ir.TypeCommon{ID: "t/B", Name: ir.Naming{Source: "B"}},
        Properties: []ir.Property{{ID: "p/dup", Name: ir.Naming{Source: "beta"}, Type: ir.TypeRef{Target: "t/B"}}}},
}}

pass.Validate returns no diagnostic and irverify.Verify no violation about p/dup. Every
PropID lookup downstream — PropPath.Segments, HTTPParamBinding.ParamPath/BodyPath,
Content.Encoding keys, and pass.checkPropIDRefs's own membership test — then resolves to
whichever the reader reaches first, with nothing in the document saying which that is.

Expected

Distinguish the two. The copies of one declaration agree on every field; two properties minted at
one ID do not, so a cheap fingerprint read off the walk — Name.Source and Type.Target are
enough to separate the classes above — tells them apart without converting values back to their Go
types. irverify already reads struct fields that way (checkNaming's namingChannels, and
pass's collectPropIDs reads Property.ID off the same walk), so the mechanism exists.

Until then the skip is a debt rather than a settled carve-out, and checkDuplicateIDs should say so
with this issue's number beside it — the discipline nameOptional in ir/irverify/naming.go
already records for the same shape of exemption.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions