Skip to content

openapi+irfacts: functions signal none or unbounded with an in-band -1 #1088

Closed

Description

@OmarAlJarrah

Problem

The styleguide bans in-band errors: no -1 for "not found". Five functions return -1 to mean "none" or "unbounded", and their callers test the sign:

Site Function -1 means
compilers/openapi/internal/merge/admits.go:326 twoDigits either byte is not a digit
compilers/openapi/internal/schema/compose.go:328 selectAllOfBase no branch qualifies as Model.Base
ir/irfacts/typegraph.go:84 typeGraph.vertex the registry declares no such type
ir/irfacts/typegraph.go:98 typeGraph.reference the registry declares no such type
compilers/openapi/internal/load/reached.go:311 reached.remaining the budget is unbounded

typeGraph.link absorbs the irfacts pair: it skips an edge whose to is negative (typegraph.go:111). The callers at lines 64–73 hand it vertex and reference results unchecked, and line 76 tests v >= 0 itself.

git grep -nE 'return -1$' -- '*.go' ':!*_test.go'
git grep -nE '= -1$' -- '*.go' ':!*_test.go' ':!*/testdata/*'

The first command also finds annotation's sign, load's compareSites and jsonshape's compareConstants, where -1 is a sign or an ordering, not a sentinel. The second finds four named constants, each out of scope below, and two local loop markers that never leave their function (majorMinor's firstDot, firstWritten's at).

Direction

A draft.

  • Each function returns (value, ok), as Go's own lookups do. remaining returns its count with a bool saying whether the budget bounds it.
  • The typeGraph callers test ok before calling link, and link's sign guard goes.
  • Each site is its own commit, in a pull request of its own: the file splits beside it (openapi: split merge.go and admits.go by responsibility #1095 for twoDigits) stay pure moves.

Out of scope

The four named constants holding -1 are values inside state, not results a caller must remember to test:

  • ir.NoSource (ir/provenance.go) is the IR's declared "no source" value for Provenance.Source. It is part of the encoded document, settled in ir: pass and irverify disagree about Provenance.Source = -1 #127.
  • load's foreign (load/validate.go:92) marks a finding's provenance until it is placed. It has ir.NoSource's value and lives in that same field.
  • irfacts' unvisited (scc.go) is one index array's marker inside the strongly connected components search.
  • irfacts' sharedProp (exposure.go) is one index array's marker inside the exposure index.

Done when

  • git grep -nE 'return -1$' -- '*.go' ':!*_test.go' finds only the sign and ordering sites above.
  • Goldens unchanged; go test ./... passes without -update.

Priority: P4, rule 1 (housekeeping: changes no lowering, validation, IR or emit-contract behaviour). The first command → 8 lines, 5 of them sentinels (morphic @ff8330ba).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:P4Refactors, tests, CI, hostile-input hardening, cosmetic diagnostics, other formatstype:choreRefactor or cleanup, no behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions