Skip to content

irverify: an empty Operation or Service ID is reported by nothing #289

Description

@OmarAlJarrah

Summary

An ir.Operation or an ir.Service carrying an empty ID is reported by nothing. The equivalent
defect in every ID class Document keys a map by is reported, so the gap is specific to the two
classes that have no registry key to disagree with.

Reproduction

Built as Go documents and run through both checkers:

document reported
two operations, both ID: "" nothing (only the unrelated ir/naming-absent)
two services, both ID: "" nothing (only ir/naming-absent)
a channel with ID: "" under key c/x ir/channel-id-mismatch
doc := &ir.Document{Services: []ir.Service{{
    ID:     "s/x",
    Groups: []ir.OperationGroup{{Operations: []ir.Operation{{ID: ""}, {ID: ""}}}},
}}}
// irverify.Verify(doc) → no violation about the IDs
// pass.Validate(doc)   → nothing

Why nothing catches it

checkRegistryKeys is what reports an empty ID today, and it reads registry keys:
registryKey reports ir/empty-<noun>-id for an empty key and ir/<noun>-id-mismatch when a key
disagrees with its node. An operation is declared inside the ServiceOperationGroup tree and a
service sits in Document.Services, so neither has a key for either half to read.

ir.DeclaredIDs skips an empty ID deliberately, and should keep doing so: nothing can reference
one, and treating several nodes that carry one as duplicates of each other would name the wrong
defect. What is missing is the separate claim that a node which is supposed to have an identity
carries a non-empty one.

Why it matters

An ID is derived from the source pointer of its defining occurrence, so an empty one means a
compiler minted nothing where it was supposed to mint an identity — an irverify.Violation rather
than an ir.Diagnostic, on the same reasoning as ir/duplicate-<noun>-id. Downstream, every
reference to that operation resolves against a registry that does not contain it, which reads as a
dangling reference at the referring site rather than as the missing declaration it is.

Suggested shape

A check over ir.DeclaredIDs' input rather than its output — the walk already reaches every
ID-bearing node, but DeclaredIDs drops the empty ones before a caller sees them. Either it grows a
second return for the nodes it skipped, or the check does its own pass over the same walk. Whichever
way, ir/empty-<noun>-id is the code the map-keyed classes already use, so one defect keeps reading
under one code.

Found while reviewing #271, which added the reference and uniqueness checks for these two classes
and left this one open.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions