Fix/strfmt dispatch symmetry - #80
Merged
Merged
Conversation
… site
`swagger:strfmt` is honoured on a named type declaration and dropped on an
alias one. The defect is not confined to aliases: it is an inconsistency in
which dispatch sites consult a declaration's comments. The named arms do it
through five classifiers; the alias arms do it in two places, each added when
a specific context was reported.
This is the corpus that pins the current behaviour, ahead of any fix. Four
fixture packages, one per dispatch family, each scanned in all three alias
modes:
- core — the sites reachable through `buildFromType`: field, pointer,
slice and array element, map value, plus the model-annotation
gate
- composition — plain struct embed and allOf member
- simpleschema — non-body parameters and response headers, where OAS v2
forbids `$ref`
- stdlib — a format annotation over a type the builder also recognizes
Every cell is a pair of declarations differing only by the `=`, so the named
half is the control and the ledger compares the two halves rather than a
hand-written expectation. 56 of 69 cells diverge; each is listed with the
dispatch site responsible. The lists assert in both directions, so a cell that
starts agreeing fails too and the record cannot drift from the code.
Three findings the pairwise comparison alone would have missed. The alias half
does not merely lose a format keyword — a struct emits an object, a byte slice
an array of integers, a `json.RawMessage` an untyped schema. An alias over an
already-annotated named type still resolves, which places the defect precisely
at "the alias's own comments are never read". And a plain embed drops the
format on both halves, so that cell is a shared gap rather than an asymmetry,
recorded as a witness and left unasserted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A format annotation was honoured on a named type declaration and dropped on an alias one, at every use site: struct field, pointer, slice and array element, map value, allOf member, non-body parameter, response header. The alias half did not merely lose the format — a struct emitted an object, a byte slice an array of integers, an alias over `json.RawMessage` an untyped schema. Each builder looked the alias declaration up only to decide whether to emit a `$ref`, then dissolved to the right-hand side. Past that point nothing recalls an alias was involved, so the annotation had no reader. Two ordering rules make it observable again: - the declaration lookup moves above the `TransparentAliases` early return, which used to dissolve without reading the declaration at all; - the classifier runs before the right-hand side is reached, so an author's format beats a recognizer. `swagger:strfmt date` on an alias of `time.Time` now yields `date`, not the recognizer's `date-time` — the annotation exists for formats the library cannot infer, so a recognizer is a default for un-annotated code, never an override. The classifier dispatches on the alias's underlying kind so the format lands where the equivalent named declaration puts it, including the whole-schema `byte` and `bsonobjectid` specials that apply to slices and arrays instead of their items. It lives on the shared builder: the parameters and responses builders have their own dissolve paths and the same gap, and a header cannot carry a `$ref` at all, so for those the format has to inline even when the alias is a model. Declaration sites are untouched, as are aliases carrying no annotation; one that dissolves onto an annotated named type still picks that type's format up as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A `swagger:strfmt` on an array or slice can describe the sequence itself or each
of its elements. That choice was a two-name allowlist — `byte`, plus
`bsonobjectid` for arrays only — and every other format went to the items. Both
names are formats for a byte sequence, `bsonobjectid` being a strfmt library type
that happens to have an array underlying, so the list was standing in for a
question about the element type. Ask that question instead:
type ID [16]byte // swagger:strfmt uuid → {string, format: uuid}
type Emails []string // swagger:strfmt email → {array, items: {string, format: email}}
A byte or rune sequence is string-like and takes the format on the schema;
anything else takes it on the items. This generalises to formats no list ever
mentioned — `uuid` over a `[16]byte` id, `ulid` — and removes the array-vs-slice
asymmetry, which had no reason to exist. `rune` and `int32` are the same type to
go/types, so `[]int32` is treated alike; a string format on integer elements was
already a contradiction.
Only the mechanical half of the question is settled. A format on a sequence of
some other element type is genuinely ambiguous and stays on the items.
Separately, the declaration dispatch grew the array and slice arms it never had.
A model sequence carrying a format published its definition with the format
dropped, and nothing downstream compensated: the field site references such a
type by `$ref` and deliberately skips the inline classifiers, on the assumption
the declaration already applied the override. Alias declarations get the same
element-driven rule, so the two spellings publish the same definition.
One existing witness moves: a `[64]byte` signature annotated `password` emitted
an array of 64 password strings and now emits a password string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The annotation produced no `default` in the spec in any placement or form. On a named basic type it was worse than inert: the classifier returned "handled" on a target it had never written, so the declared type published a schema with no type at all, and every property referencing it came out typeless. Silently, and regardless of whether an argument was given. It is now an empty sink, following the precedent set by swagger:alias — parsed, reported as `validate.deprecated`, and otherwise ignored. An annotated type emits exactly what it would emit unannotated. Retiring it rather than implementing it, because every place OpenAPI 2.0 admits a default is already served. The `default:` keyword covers the Schema, Parameter, Items and Header objects — which is precisely its registered context set — and a `default` response code in a route's `Responses:` body covers the Responses object. That closes the surface; there was no meaning left to give the annotation, and no usage to preserve: the sole occurrence in the corpus was a fixture written for branch coverage, whose own comment described the empty schema as the expected result. The value argument becomes optional. Requiring it hard-errored on the bare form the annotation had been documented with for years, which is an unkind way to greet a deprecation. Doc site: the reference page, the tutorial section, the annotation index and the context matrix all described an annotation that had never done anything. They now carry the deprecation and point at the keyword; the tutorial gains the default-response spelling, which is the one sense of "default" the keyword does not cover. The published example is removed rather than rewritten — it demonstrated the bare form on a `var`, a placement no builder ever reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A declaration's comment block is dispatched before its Go type is resolved onto
the schema, so `default:`, `example:` and `enum:` were coerced against an empty
type and fell back to the author's raw text:
// swagger:model Port
// default: 8080
// enum: 1,2,3
type Port int
→ {"type":"integer", "default":"8080", "enum":["1","2","3"]}
The same keywords on a field, a parameter, a header or a body property were
always correct — there the type is known when the walk runs, and the identical
handler coerces properly. The enum row is the consequential one: an enum of
strings on an integer schema is a document no validator can satisfy. A JSON
array fared worse than a scalar, arriving as a string holding JSON source rather
than as a value of the wrong type.
`RecoerceDeclValues` re-types the three from their raw form, at the same "the
type is known now" seam where `RecheckSchemaShape` already re-gates the
validation keywords for exactly this reason. Re-coercion is sound because the
fallback preserved the text verbatim: a value still stored as a string is one
that was never typed. String-typed schemas are skipped, since there the fallback
and the correct answer coincide.
The three keywords travel together deliberately — they share
`validations.ParseDefault` and the same dispatch arms, so a divergence between
them would itself be a defect. The witness pairs each declaration cell with a
field-site control carrying the identical literal, and asserts they agree rather
than hard-coding a coercion result.
Also pinned: the default RESPONSE, which is a wholly separate mechanism from a
default VALUE. `default:` is not a legal keyword in a response block context, so
the two cannot collide, and the fixture keeps both senses side by side.
A model-level object literal now reaches the spec as real JSON, matching what a
field-level one already did — a difference that had been sitting inside a single
test's expectation, unnoticed.
Doc site: `swagger:enum` the annotation and `enum:` the keyword produce the same
spec keyword from opposite directions, which is easy to trip over. The tutorial
gains a side-by-side comparison and both reference pages cross-link it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A `default:`, `example:` or `enum:` value that does not fit the schema it sits on was handled three different silent ways. On a declaration the raw string survived, so `default: notanumber` reached the document as `"notanumber"` on an integer schema — output no validator accepts. On a field the value was dropped instead, discarding the author's intent without a word. And an `enum:` produced a mixed array, `[1, "two", 3]`, at both sites. All three now warn, and all three drop. Dropping beats keeping: a value of the wrong type yields a document that fails validation, whereas omitting it yields one that is merely incomplete — the lesser harm, once the author is told which value went and why. Declaration and field sites agree on behaviour and on reporting, which they did not before. For an enum the drop is per member and the warning names it. An enum is a closed set, so removing a member changes what the API accepts; a count would not be enough to act on. It fails closed, as the swagger:enum annotation already does when a const value does not fit. Coercion towards string never fails — every literal has a string form — so a string-typed schema is skipped outright and no diagnostic can arise there. That also makes the existing skip in the decl re-coercion a statement about correctness rather than an optimisation. Parameters and headers already reported these failures through their error sink; only the schema paths were silent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Two readings of the `json:"-"` tag diverged from the wire, in opposite directions. A promoted field re-declared with `json:"-"` was deleted from the schema. Go does not hide it: a `-` field is ignored entirely, never enters the name set, and so shadows nothing — the embedded field keeps marshalling. The schema understated what the API returns. The property stays now, and the existing hint continues to point at `swagger:omit`, which drops it for real; the intent behind the tag is real even though the mechanism does not work. The `json:"-,"` escape, which names a field literally `-`, was dropped instead of emitted. encoding/json compares the whole tag to `-`, so a trailing comma names rather than skips; splitting on the comma first conflated the two. `-` is now accepted as a name from the json tag alone — no other tag type has an encoding rule to appeal to. Both were pinned as contracts. `TestOverridingOneIgnore` asserted two properties where the wire carries three, and the corpus has carried `IgnoredOther string` under `json:"-,omitempty"` since long before this, its name recording the assumption rather than the behaviour. A fixture comment likewise credited Go's depth rule for shadowing a `-` re-declaration, which it does not do. All three now say what Go does. The witness is differential, because this corpus has an oracle: the right answer is not a design choice but whatever encoding/json produces. The fixture module marshals its own types and commits the resulting key sets; the integration test scans the same types and asserts the property sets match. Neither side writes an expectation down. Controls for the shapes that were already correct — a re-declaration under a real name, an ignore with nothing to shadow, and an embed tagged `-` — travel with them, so a future change cannot quietly trade one for another. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…AS 2.0 does
Completes the alias half of the classifier story that swagger:strfmt started.
A `swagger:type` on an alias declaration was dropped at every use site — struct
field, pointer, slice and array element, map value, non-body parameter, response
header — so the annotation decorated nothing while the same annotation on a named
declaration worked. Every form was affected: scalar names, `[]T` prefixes, and
references to other scanned types.
The parameters and responses builders needed the fix too, for the third time in
this series. Both unaliased the field before delegating, throwing the declaration
away before any classifier could read it; they now hand the alias itself to the
schema sub-builder, whose OptionFor already selects SimpleSchema mode from the
target's location.
Which matters, because swagger:type brought a constraint swagger:strfmt did not:
a non-body parameter and a response header are SimpleSchema locations, and an
object has no representation there. That case was silently broken. A bare
`object` argument reached the target and was wiped by the exit validator, leaving
a parameter with no type at all; a type-name reference resolved into the nil
Schema a non-body parameter exposes and vanished without even a diagnostic, the
validator never firing because the illegal value never materialised to be
checked. Since `type` is mandatory under SimpleSchema, an override that cannot be
represented is now refused before it is applied, with a diagnostic naming the
annotation, and the Go-derived type stands. The parameter stays valid.
`file` is no longer rejected as a swagger:type argument. It is an OAS v2 type
name like any other, so the annotation that names types should name it, and
`swagger:file` is the extraneous spelling — the preference is now the other way
round. Rather than add a second location rule, the argument raises the same
signal the older annotation raises, so both spellings pass through one gate: a
formData parameter and a response body, nothing else.
The SimpleSchema witness previously asserted that an offending parameter is wiped
to `{}`. It now covers both remedies, which had been conflated: an override the
location cannot honour is refused so the Go type survives, while a Go type with
no SimpleSchema form has nothing to fall back on and is still wiped.
One known difference is pinned rather than fixed: an allOf member honours no type
classifier, so the named half emits an empty member there. That breaks both
halves and is tracked separately.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…tion A classifier body is prose-only, so the parser warned on every keyword token it found there. `in:` and `name:` are not schema-body keywords, though: they say where a field goes and what it is called, and the parameters and responses builders read them straight from the doc text. A parameter field may carry both a classifier and `in:` — indeed `in:` is mandatory there — so the canonical file-upload idiom warned about its own required directive: // in: formData // // swagger:file MyFormFile *bytes.Buffer → warning: keyword "in" not valid under swagger:file The same field without the annotation never warned, so the check was inconsistent as well as wrong. It fired for `swagger:strfmt`, `swagger:type` and `swagger:file` alike — any classifier next to a field directive. Every keyword context-invalid diagnostic in the entire fixture corpus was an instance of this. After the fix there are none, which is why the malformed-input fixture gains a `maximum:` under the same annotation: a schema keyword there is genuinely invalid, and the TUI's column-translation test needs a diagnostic that lands exactly on a keyword. Its previous subject was the spurious `in:`. No golden moves: the rejected keywords were dropped, and dropping them silently produces what dropping them loudly did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Closes the last of the classifier annotations that an alias declaration dropped silently. `swagger:strfmt` and `swagger:type` were fixable: both decorate the emitted schema, and nothing about aliasing fights them. This one is not. An enum's members are found by collecting the constants declared with the annotated type. A type alias is erased by the type-checker, so in type Unsigned = uint64 const Zero Unsigned = 0 `Zero` is a `uint64` constant indistinguishable from every other `uint64` constant in the package. There is nothing to collect and no plumbing that would change it, so the remedy is to say so rather than to propagate. The diagnostic names the declaration and suggests the named form. It fires wherever the alias is reached, since that is where the members go missing, and it is deliberately silent for an alias to a NAMED enum type — that case works, because the named type survives the alias, and warning about it would tell authors their working annotation is broken. The witness pins all three: the named control, the alias that cannot work, and the alias to a named type that can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…t the next drift The `swagger:type` fix reached the non-body branch of both alias field handlers and not the body one, so the annotation worked on a model field and a query parameter while dropping on a body parameter and a response body. Both branches end in the same dissolve, and unaliasing there discards the declaration before any classifier can read it; the body branch now hands the alias to the schema sub-builder as the non-body branch already did. Notably `swagger:strfmt` on the same alias was unaffected, because its classifier sits above the branch. Two annotations fixed together, diverging one branch apart, is the shape of the problem rather than an instance of it. So this also adds the detector. The schema, parameters and responses builders each resolve Go types to spec constructs and each carries its own copy of rules the others need; nothing forced them to agree and nothing noticed when they stopped, which is why a fix verified on one path read as complete. The suite takes one Go shape reached from three full-schema positions — a model field, a body parameter, a response body — and asserts the three agree. It carries no expectations of its own: each subject is compared against the model field, whose behaviour is pinned by its own witnesses, so the suite only ever asks whether the builders still say the same thing. Two choices in it are load-bearing. The subjects are the field's type directly rather than nested in a struct: nesting delegates everything to the schema sub-builder, where the three agree trivially and the suite reports a comfortable all-clear, while the divergences live in the short-circuits that fire when a field is itself a named or alias type. And SimpleSchema positions are excluded, because a non-body parameter and a response header have a genuinely different legality surface — which is the reason the builders grew separate paths to begin with. Comparing those needs a declared projection, not equality, and mixing them in would bury real drift under expected difference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…and responses The two builders each carried their own walk for an alias reached as a field, and the copies drifted three times: the declaration lookup sat below the TransparentAliases return in one and not the other, `swagger:type` reached the non-body branch and not the body one, and a missing source errored at different points. Both walks ended in the same delegation to the schema builder, which is where every classifier an alias declaration can carry already lives. So the resolution moves there and the callers keep only what is genuinely theirs: which types they refuse, the error they wrap a missing source in, and — for responses — the cross-ref path a header anchors under. 158 lines of duplicated control flow become 13. Most of the walk turned out to be re-implementing what it delegated to. The schema builder already applies the classifiers, already honours the TransparentAliases rule, and already dissolves; handing it the alias rather than the right-hand side is the whole of what the callers needed to do. What remains is one genuine exception — a body field naming a `swagger:model` alias keeps its `$ref` identity at the use site. One behavioural difference goes with the duplication. Under TransparentAliases a `swagger:type` on an alias was honoured for a model field and dropped for a query parameter and a response header, because the schema builder reads the declaration before dissolving and the hand-written copies did not. The named half emits the overridden type in every mode, so the alias half now does too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…anchor
Extends two harnesses ahead of factoring the duplicated field dispatch out of the
parameters and responses builders, because neither could see the change.
The conformance suite's subjects were all named or alias types, which reach only
two arms of the dispatch. It gains struct, interface, map, pointer, plain basic,
and a slice with an inline element, so the arms about to be moved are compared in
all three positions rather than guarded by goldens alone.
The second gap was the dangerous one. The responses builder threads
`descendBody("items")` through its array arms, and that affects cross-ref
provenance pointers only — never the emitted spec. No golden can see it, and
neither can a suite that compares schemas, so dropping those two lines would have
passed everything. The provenance corpus gains an array-bodied response whose
element is inline, which pins
/responses/{name}/schema/items/properties/code
and fails without the threading. Inline is the operative word: a named element
anchors in its own definition instead, where the threading is unobservable, so a
witness built from one would prove nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Every field shape the parameters and responses builders do not handle themselves is resolved by deferring to the schema builder, and each had written the same three steps out per shape: construct a sub-builder, build, drain its post-declaration queue. Nine copies between them, differing only in the Options. The drain is the part worth centralising. A model reached solely through one field — no annotation, no other reference — arrives in the spec through that queue and nowhere else, so omitting the loop loses it silently. That is not hypothetical: the loop went missing from the parameters map arm during an earlier factor-out and needed a dedicated fixture to find. Two spellings, because there are two shapes: `Delegate` builds in the caller's own declaration context, `DelegateAs` in a resolved one and infers names from it. The Options stay at the call sites, since that is where the builders genuinely differ — the responses builder threads a cross-ref path, and a map value is targeted explicitly rather than through OptionFor. `buildFromFieldStruct` and `buildFromFieldInterface` disappear entirely; both were a delegation with no shape-specific content, and the dispatch arm now says so directly. The field dispatch switch itself stays duplicated. Sharing it needs callbacks for the named arm, the alias arm and the responses builder's items descend, which is more machinery than the two lines of difference it would remove. No golden moves and no provenance anchor moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
… scan
An `error` field in a `swagger:parameters` struct aborted the whole build. The
guard is right that an error has no meaning as a parameter — describing it as
`{type: string}`, which the schema builder would, tells a client to send
something it should never send — but refusing the entire spec over one field is
not. A struct shared between a parameter set and a response could not be scanned
at all, though losing the field on the parameter side is exactly what its author
would expect.
It now skips with a located diagnostic, which is the house rule and which its own
sibling two arms down already followed for a Go type with no SimpleSchema form
(go-swagger#2804); this guard predates that and never got the same treatment.
The conformance suite gains a golden. Comparing the three builders against each
other only asks whether they agree, so an answer all three get wrong would pass
silently; the golden makes every subject's emitted spec reviewable on its own.
It also gains its first pinned divergence, with the assertion running in both
directions so the list cannot rot: a named `[]string` carrying a non-special
format is rendered by the schema builder as an array of formatted elements and by
the other two as a single formatted string. They short-circuit on a local helper
that writes the format onto the whole schema unconditionally, which predates the
element-driven rule the schema builder applies — so the same Go type is described
to a client as a list of email addresses in one position and as one email address
in another.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…rule
The parameters and responses builders short-circuited a named field on a local
helper that wrote `Typed("string", format)` unconditionally. That predates the
element-driven items-vs-whole rule the schema builder applies, so a format on a
`[]string` landed on the whole schema rather than on its items: the same Go type
was described to a client as a list of email addresses in one position and as a
single email address in another.
Both short-circuits are gone — the format one because it was wrong, the
`time.Time` one because the delegation immediately below already recognises it.
The conformance suite's two pinned cells are unpinned and now agree, and no
golden moved, which is the evidence that removing them changed nothing except the
divergence.
Removing the consumers left a `seen` map threading through the field dispatch and
reaching nothing, so it goes too, along with the helper that is now unreferenced
in the parameters builder.
One related defect is deliberately NOT addressed here. The same two shortcuts
appear again on the response DECLARATION arm, where they are worse: both write
into a local schema and return without the call that attaches it, so a
`swagger:response` on a named `time.Time` emits a response with no schema at all.
Removing them there is not enough on its own — that arm also passes the
underlying type rather than the declared one, so the recognizer and the
declaration's own classifiers are bypassed, and correcting that moves five other
tests. It wants its own change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A `swagger:response` declared on a named type whose underlying is not a struct came out carrying a description and nothing else. The arm short-circuits on the stdlib time recognizer and on the declaration's format, and both branches wrote into a local schema and returned without the call that attaches it — so the schema was built and then dropped. The format branch was wrong as well as unattached: it wrote the format onto the whole schema, where a `[]string` annotated `email` is a list of email addresses rather than one of them. It now goes through the shared element-driven rule. Handing the sub-build the type's underlying rather than its declaration turns out to be deliberate, and worth stating since it reads like the defect: a `swagger:response` declares a response, not a model, so passing the named type would send it through the $ref machinery and publish it as a definition. Two existing witnesses exist to prevent precisely that, and they caught the attempt. The new witness pairs each subject with the same type reached as a model field — both are full-schema positions, so a declaration should not mean different things depending on which one reads it — and asserts that none of the response types becomes a definition. One cell stays pinned. `type Stamp time.Time` is not `time.Time`, so the recognizer declines, correctly. The model side resolves it anyway because it builds from the declaration's written right-hand side, where the recognizer fires one level in; this arm peels to the fully underlying struct instead and reads the response as a struct whose fields become headers, of which time.Time has none. That is a different defect and wants its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…ts underlying `type Stamp time.Time` declared as a response emitted no schema. `Underlying()` peels every named layer at once, so the arm received time.Time's STRUCT and read it as a response struct whose fields become headers — and time.Time exports none. The recognizer never got a chance: it keys on identity, and the `time.Time` layer it would have matched had already been peeled away. The schema builder does not have this problem because it builds from `Spec.Type`, the type a declaration was WRITTEN over, where the recognizer sees `time.Time` one level in. The responses arm now does the same, and the response body renders as the model field does — a `$ref` to the same definition. Only a NAMED right-hand side redirects. A struct literal, a slice or a basic type is already the shape this arm should build, and sending those through the sub-builder would publish the response type as a definition, which a `swagger:response` must never do. The witness cell that was pinned for this is unpinned, and its list is now empty: a response body and a model field are both full-schema positions, so every difference found between them has been a defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The schema builder applies one identity-recognizer set (any / time.Time / error / json.RawMessage / uuid) uniformly. The parameters and responses builders carried a hand-rolled subset per function, differing per function, and each subset ran AFTER the declaration lookup it should precede. A recognizer answers from a type's identity alone, so ordering it behind a lookup subordinates a rule that needs nothing to one that can fail. For the predeclared `error` it always fails: a predeclared object has no package, so no declaring source exists to find in any package graph. The responses field arm had no recognizer at all and went straight to the lookup, where reading the import path off a nil package took the whole scan down — a response body field typed `error` was a panic, not a degradation. The refusal on the parameters side was also spelling-dependent. An alias's object is the alias's own name, so a rule keyed on the object applied to `error` and not to `type Wrapped = error`: the bare spelling was dropped, the aliased one emitted, for the same Go type. It now resolves through the alias, and the alias arm skips the field where it used to abort the scan. Both parameters arms report the drop under their own reason rather than borrowing the SimpleSchema one. `error` is meaningless as an inbound value in every location including `in: body`, so a message naming a location sent the reader to change an `in:` that was never the problem. Coverage came first and is the reason the rest is visible: the conformance suite reached the stdlib types only through aliases, leaving the named arm — the one carrying the subsets — untested. It now reaches `time.Time`, `json.RawMessage`, `any` and `error` as named types in all three positions, with `error` as the suite's one declared difference: a response renders it, a parameter must refuse it and say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
responseTypable.AddExtension wrote onto the response in every case, so an
extension describing the body's Go type surfaced as a sibling of `schema`
and `description` instead of inside the schema it was talking about — a
response body typed `error` came out as {schema: {type: string},
x-go-type: error}.
The parameters typable has routed body-to-schema all along; this is the
same rule, kept privately by each builder, that only one of them applied.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Both names had to be a letter followed by at least one more character, so a single-character tag or operationId did not match. OAS 2.0 puts no such floor on either — both are free-form strings, and a one-letter tag is ordinary. The annotation did not merely lose the short name, it disappeared whole. The tags group is optional, so `swagger:route GET /pets e listPets` did not fail at `e`: it fell back to matching with no tags, leaving the operationId pattern to swallow `e listPets`, whose alphabet has no space in it. The line then matched nothing at all — and a swagger:route that matches nothing is not a malformed route, it is not a route, so the path simply never appeared and nothing anywhere said why. Applies to swagger:operation identically; both share the two patterns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A `swagger:route` or `swagger:operation` line that fails its regex produced nothing and said nothing. The failure mode is worse than an error: the annotation does not become malformed, it stops being an annotation, so it is indistinguishable from prose to everything downstream and the first sign of trouble is a path missing from the output. Every regex change to these patterns has carried that risk, and the path-parameter regex strip already carries a comment saying so. The line is now reported under scan.unparsed-path-annotation, with its position and the offending text. Telling a failed annotation from prose needs more than the keyword. Since annotations must start the comment line, a doc comment whose sentence opens `swagger:route response lines are …` starts with it too — three such lines exist in this repo's own fixtures. The check therefore matches the annotation HEAD: keyword, method, and a /-rooted path. A real annotation always has all three; prose after the keyword never reaches a path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
… positions The conformance suite compared a model field, a body parameter and a response body — three positions that converge on one field dispatch. An allOf member does not: it is resolved by buildNamedAllOf, a composition arm carrying its own copy of the classifier cascade, which makes it the position most likely to be left behind by a fix applied to the others. AllOfHost reaches every embeddable subject as a member of ONE allOf, in the subject table's order, with the composing struct's own field in a trailing member. One allOf rather than one host per subject: members resolving side by side also witness that no member's classifier leaks into its neighbours. The positional mapping is asserted rather than assumed — member count and the trailing member's shape — since a member that fails to build emits an empty member rather than none, and a count that still matches is what shows the indices did not shift. The position starts with three pinned cells against the one already on record: swagger:type is dropped there, swagger:enum likewise (a basic underlying then falls to the arm's warn-and-skip default and the member comes out empty), and a format on a string sequence lands on the whole member instead of on its items, the arm's strfmt classifier predating the element-driven rule. Subjects Go cannot embed under a usable name — a map, a slice of an inline struct, and the pointer/basic/predeclared arms — sit the position out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…scan An annotation nobody recognises is almost always a typo, and it aborted the whole scan: one mistyped keyword in one comment and an entire package graph produced nothing — the outcome least useful to whoever typed it. It is now reported under parse.invalid-annotation, with the name and the location, and classified as nothing so the comment reads as prose. Every other annotation in the tree still works. Skip-and-diagnose is the house rule, and this is the case that most deserves it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
`swagger:strfmt` and `swagger:type` in an embedded field's own comment are parsed into the field doc and then never read: no embed arm consults either. On a regular field both are honoured, so the same annotation in the same syntactic position works one line and does nothing the next. The silence was the worst part. The scanner reads that comment and rejects an unknown annotation in it, so the author got validation feedback implying the annotation was meaningful and no feedback at all that it had been discarded. They are still ignored — this reports the drop, it does not change it. An embed contributes its embedded type's shape, and what that shape is comes from that type's own declaration, so the message names that as the place the annotation belongs rather than merely refusing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
An embed contributes the shape of the type it embeds, and what that shape is comes from that type's own declaration. `swagger:strfmt` and `swagger:type` written in an embedded field's doc comment do nothing — which is easy to get wrong, because both are honoured on an ordinary field one line away. Covers the annotation reference for swagger:allOf, the allOf composition how-to, and the schema builder's own contract doc, which now names the five fieldDoc signals an embed actually consumes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
An allOf member is a full schema describing one type, exactly as a model
field is, but the composition arm resolving it kept its own copy of the
classifier cascade — one shape-BLIND strfmt check and nothing else. Three
annotations were wrong there:
- swagger:type was dropped, and a basic underlying then fell to the arm's
warn-and-skip default, so the member came out as an empty schema;
- swagger:enum likewise;
- swagger:strfmt on a string sequence landed on the whole member, so a
composed []string annotated `email` asserted the value IS an email
address rather than a list of them.
The shape-aware classifiers already existed, scattered through
buildNamedType's switch. They move to applyNamedShapeClassifier, which both
arms now call, so neither can answer one of these differently again.
The arm keeps its own $ref-or-inline policy. Delegating wholesale was tried
and reverted: buildNamedType publishes a definition for a non-model struct,
which would put types in the spec their author never asked to expose — the
default-allof-embeds fixture documents inline-for-non-model as deliberate.
The classifiers are the shared part; the reference policy is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
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
1 participant
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.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist