Skip to content

Fix/pointer gaps - #281

Merged
fredbi merged 4 commits into
go-openapi:masterfrom
fredbi:fix/pointer-gaps
Aug 9, 2026
Merged

Fix/pointer gaps#281
fredbi merged 4 commits into
go-openapi:masterfrom
fredbi:fix/pointer-gaps

Conversation

@fredbi

@fredbi fredbi commented Aug 9, 2026

Copy link
Copy Markdown
Member

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

fredbi and others added 3 commits August 9, 2026 14:04
Several checks walk a map looking for fault: definitions, paths, status
codes, response headers, properties. Go randomises map iteration, so the
same document could be reported differently from one run to the next — in
which order the findings came out, and, where a check stops on the first
fault it meets, which of two equally faulty definitions was named at all.

Those walks now take their keys in sorted order. "First" therefore means
first in definition-name (or path, or status-code) order. How many findings
a document yields, and when a check stops, are unchanged.

The circular-ancestry check in validateDuplicatePropertyNames returned on
the first offender whatever the options said, so a second circular
definition could never be heard from. It now returns only when
ContinueOnErrors is false, and otherwise moves on to the next definition
without descending into the loop it just found.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Located.Pointer is documented as a JSON pointer into the validated document,
and in four families of finding it addressed nothing.

Values checked by a borrowed validator. The parameter and header validators
are the ones a generated client uses at runtime, where the name of the
parameter is all a caller has, so they locate a finding by that name. Spec
validation borrows them to check a default or an x-example, where the name
addresses nothing: those findings now sit on the value's own node. A body
parameter is entered through its schema, as a response already was.

The path template. A parameter missing from a path template was reported
under the variable name rather than the path holding it.

Findings that knew their site but did not say it. A duplicate operationId is
counted over the operations rather than a flat list of identifiers, so it can
name the first operation declaring it. An unresolved reference is located at
the first local $ref that addresses nothing, and the separate message raised
while expanding a response carries that response.

Faults reached through a $ref. Checks walk an expanded document, so a finding
below a reference came out with a pointer descending into a node holding
nothing but "$ref". Such a pointer is now followed to what the reference
leads to, as many times as it takes; one that stops at the reference itself
is left where it is.

Three smaller gaps close with them: a document holding no paths is reported
at the root rather than under /paths, the format validator records the
location it holds, and a parameter too broken to be identified keeps its name
in the message while the pointer stops on the array that holds it — pathToken
gains a cosmetic kind, the converse of structural, for that.

Finally, a location is trimmed to the deepest node the document holds. This
runs once every check has had its say and only ever shortens, which is what
makes the pointer always resolve.

Over the fixture corpus, findings whose pointer addresses nothing go from 132
to 0, and exact locations from 1955 to 2161 of 2213.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A definition is not the only place a document says an object must hold a
property: so does the schema of a property, of an array item, of an
additionalProperties. Each is a self-contained object definition, and a
required entry naming something it never declares is the same modelling slip
wherever it sits. Only the top of a definition was checked, so the slip went
unreported one level down.

The check now walks the schemas a definition holds inline, and locates each
finding on the offending entry of the required array it walked into, e.g.
/definitions/A/properties/inner/required/0. A finding about such a schema
names it by the way down to it — schema "A.inner" — rather than naming the
definition holding it, which would send a reader to the wrong place. A
definition still names itself, in the words it always used.

Two boundaries keep the walk honest. A schema written as a $ref is left
alone: it is checked where it is defined, and following it would report the
same slip twice and would not terminate on a recursive definition. And inside
allOf, anyOf, oneOf or not, a member is a fragment of a constraint rather
than a complete definition — its required entries speak of the instance the
whole composition describes, are legitimately met by a sibling member or by
no declaration at all, and are enforced when data is validated. Their own
member schemas are still walked.

That second boundary also fixes a false positive that predates the walk: a
definition requiring a property contributed by one of its allOf members, or
by a base definition it refers to, was reported as requiring something it
does not declare.

Over the fixture corpus, which includes the Kubernetes and Bitbucket APIs,
this reports neither more nor less than before: the specifications that do
carry nested required entries declare them correctly.

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi
fredbi force-pushed the fix/pointer-gaps branch from 2d2c7a3 to 7cdf3d9 Compare August 9, 2026 12:14
No -race with that flag, as the extra resource consumption with pool
debug breaks CI.

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi
fredbi force-pushed the fix/pointer-gaps branch from 15e5233 to 2bf3c27 Compare August 9, 2026 13:02
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.25806% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.32%. Comparing base (02b8f94) to head (2bf3c27).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
required_walk.go 72.72% 9 Missing and 9 partials ⚠️
result.go 88.23% 1 Missing and 1 partial ⚠️
spec.go 97.75% 1 Missing and 1 partial ⚠️
helpers.go 80.00% 1 Missing ⚠️
ref_redirects.go 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #281      +/-   ##
==========================================
+ Coverage   93.13%   95.32%   +2.19%     
==========================================
  Files          27       31       +4     
  Lines        3395     3618     +223     
==========================================
+ Hits         3162     3449     +287     
+ Misses        156      106      -50     
+ Partials       77       63      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@fredbi
fredbi merged commit 39098ef into go-openapi:master Aug 9, 2026
23 checks passed
@fredbi
fredbi deleted the fix/pointer-gaps branch August 9, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant