Fix/aliased types#32
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (77.14%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #32 +/- ##
==========================================
+ Coverage 79.84% 80.18% +0.34%
==========================================
Files 71 71
Lines 5786 5739 -47
==========================================
- Hits 4620 4602 -18
+ Misses 875 859 -16
+ Partials 291 278 -13 ☔ View full report in Codecov by Harness. |
fredbi
force-pushed
the
fix/aliased-types
branch
from
febbbc2 to
599e453
Compare
The buildDeclAlias function has three branches selected by the
RefAliases / TransparentAliases flags. Before this commit, the
stdlib-special recognizers (time.Time, error, json.RawMessage,
any) fired in the Transparent branch but not in the others — so
an annotated `type Timestamp = time.Time` produced its canonical
{string, date-time} shape under TransparentAliases but emitted
the wrong shape (the walked Underlying struct) under Default,
and crashed with a nil-package panic under RefAliases for the
predeclared `error` type.
Three changes consolidate the dispatch:
- guard the predeclared (nil package) case in the *types.Named
arm of the $ref branch so `type Err = error` no longer
nil-panics on the GetModel lookup;
- consult applyStdlibSpecials at the top of the Expand branch
before walking Underlying() — same recognizer call the
Transparent branch already used, applied per-decl;
- lift the recognizer call above the nil-pkg conditional in
the $ref branch so predeclared and packaged stdlib types
both inline their canonical shape directly on the alias decl
(no chain hop to a separately-built Time / RawMessage
definition).
After this commit all four stdlib aliases (Timestamp / Err / Raw
/ SilentTime) produce their canonical recognizer shapes
uniformly across the three modes.
Adds two calibration fixtures — `alias-calibration-primitive`
and `alias-calibration-stdlib` — that exercise the three-mode
dispatch matrix for primitive and stdlib-special RHS types. The
integration test `coverage_alias_predeclared_ref_test.go` pins
the post-fix shape under both Default and Ref modes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A struct that embeds an alias of a named struct used to produce
allOf composition with a $ref to the alias — a different shape
from a struct that embeds the named type directly (flat inline
properties). The two embeds reference the same Go type
(`types.Unalias(BaseAlias) == Base`); the spec-side asymmetry
came from `*types.Alias` taking a separate dispatch arm in
`buildEmbedded` and `scanEmbeddedFields`.
Two changes align the paths:
- `scanEmbeddedFields` no longer treats aliased embeds as
implicitly opted into allOf composition (the `!isAliased`
gate is removed). Plain (non-`swagger:allOf`) aliased embeds
now inline their properties just like direct-named embeds.
- `buildEmbedded` routes the `*types.Alias` arm through
`types.Unalias` and then re-enters the named-embed path.
The same code subsequently handles allOf composition when
the embed carries `swagger:allOf`.
The documented contract is now uniform across embed kinds:
"embeds always inline properties, never $ref unless the embed
carries `swagger:allOf`". The annotation is the sole gate for
allOf composition.
Adds the `alias-calibration-embed` fixture covering all four
embed permutations (direct named, alias of struct, pointer,
named interface) plus the bidirectional `swagger:allOf` opt-in
witnesses for both alias and direct-named embeds. The
`coverage_alias_embed_inline_test.go` test pins both halves of
the contract.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
In the schema builder, an alias reached as a field, element, or
allOf-member type used to MakeRef the alias decl unconditionally
(outside TransparentAliases mode). This manufactured a
`definitions` entry for the alias even when the user never opted
in via `swagger:model`, leaking an internal Go implementation
detail into the spec and producing awkward output where a $ref
to an unannotated chain target had to be wrapped in `allOf`
whenever a validation constraint sat next to it.
The schema-level rule is now:
- annotated alias (`swagger:model`) → first-class spec entity:
field/element use sites keep `$ref: <AliasName>`; the alias
carries its own `definitions` entry under its own name.
- unannotated alias → Go implementation detail: dissolves to
its unaliased target at every use site, no `definitions`
entry.
The patch site is `buildAlias` in
`internal/builders/schema/schema.go`: after the `GetModel`
lookup, gate the `MakeRef` on `decl.HasModelAnnotation()`. When
the gate fails, route through `buildFromType(tpe.Rhs())` — the
same dissolve path `TransparentAliases` already uses, but
applied per-decl based on user intent rather than per-mode.
`TransparentAliases` still supersedes annotation at use sites
(everything dissolves regardless of the annotation), preserving
the existing mode semantics. At the decl level, `swagger:model`
still forces registration unconditionally, so an annotated alias
keeps its own definition under Transparent.
Test coverage:
- the `alias-calibration-embed` fixture grows the `Envelope` /
`EnvelopeAnnotatedAlias` pair to pin both halves of the rule;
- bidirectional companions land across the affected fixtures
(alias-expand model side, the go123 historic corpus,
alias-response-shapes, alias-findmodel-witness) so each
demoted $ref has its annotated counterpart showing the
alias-name $ref shape is recoverable via `swagger:model`;
- new tests assert both the unannotated dissolve and the
annotated preservation across Default / Ref / Transparent
modes;
- cross-fixture goldens shift consistently — the most visible
cleanup is `order.id` in `go123_aliased_spec.json`, where
the `allOf: [{$ref: UUID}, {minimum: 1}]` workaround
collapses to a clean `{integer, minimum: 1}` inline shape
now that UUID dissolves.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…ites
Two rules land in the parameters builder, mirroring the
schema-side use-site rule and adding a top-level clause that
has no schema analogue.
1. Top-level alias annotated `swagger:parameters` is transparent
re: model creation. Neither the alias decl nor any chain link
of its backing struct surfaces as a `definitions` entry — the
fields of the unaliased target become the operation's
parameters. The `swagger:parameters` annotation declares a
parameter SET, not a model, and must not promote the backing
chain to definitions either explicitly (the alias) or
transitively (the target). The previous `AppendPostDecl`
calls in `parameters.buildAlias` did both, producing the
unexported-backing-struct leak and a dangling
`AliasedTopParams`-style entry that duplicated the parameter
set under a model name.
No mode-specific behaviour: TransparentAliases, RefAliases
and Default all share the same path. The alias is transparent
for the parameters layer regardless of mode.
2. Body field aliases follow the schema-side rule. Annotation
gates first-class identity:
- annotated alias → `$ref` preserves the alias name, alias
gets its own definition (the schema layer then shapes
that definition per the mode flag — chain under Ref,
structural under Default and Transparent);
- unannotated alias → dissolve via `types.Unalias` to the
unaliased target (full chain collapse in one step), no
`definitions` entry for the alias.
The mode flag (RefAliases vs Default) has no effect at body
field sites under this rule — the annotation is the only gate.
This corrects a curious pre-patch property where
`swagger:model` had NO effect at body field sites under
either mode (the parameters builder unconditionally expanded
the alias regardless of annotation).
TransparentAliases still supersedes the annotation gate at
use sites (every body alias dissolves to its unaliased
target). Non-body parameters are SimpleSchema targets and
cannot carry `$ref`; they always expand to the unaliased
target regardless of annotation.
Adds the `alias-parameters-calibration` fixture exercising all
three reach contexts (top-level alias, body field, non-body
field) under all three modes, with bidirectional annotated /
unannotated companions for the body-field case. Coverage tests
pin the contract per mode with both inline assertions and
goldens.
Side-effect cleanup across existing fixtures: the
`enhancements_alias_expand.json` and
`enhancements_alias_ref.json` goldens shrink as the historic
`AliasedTopParams` / `AliasedTopParams2` / `exportedParams` /
unannotated `PayloadAlias` chain leaks vanish.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Mirrors the parameters-builder rule and the schema-builder
use-site rule, applied to the responses layer. Two rules land:
1. Top-level alias annotated `swagger:response` is transparent
re: model creation. Neither the alias decl nor any chain link
of its backing struct surfaces as a `definitions` entry — the
fields of the unaliased target become the response's body and
headers. The `swagger:response` annotation declares a
response, not a model.
This commit also fixes two pre-existing bugs surfaced by the
calibration fixture:
- Default mode crashed on top-level response aliases with
"anonymous types are currently not supported for
responses" — the old code walked
`tpe.Unalias().Underlying()` and handed a *types.Struct
to buildFromType, which has no case for anonymous structs.
- RefAliases mode produced an EMPTY body schema on the
top-level response — the old MakeRef branch created a
throwaway typable that never connected to the response
object.
Routing through `buildFromType(tpe.Rhs())` dispatches to
buildNamedType -> buildFromStruct for the backing Named
struct, which is the same path a directly-declared
`swagger:response` struct takes. Both bugs vanish.
2. Body field aliases follow the same gate as parameters and
schema:
- annotated alias → `$ref` preserves the alias name; the
alias gets its own definition via MakeRef's
AppendPostDecl side effect;
- unannotated alias → dissolve via `types.Unalias` to the
unaliased target (full chain collapse in one step), no
`definitions` entry for the alias.
TransparentAliases still supersedes the annotation gate at
use sites. Non-body fields are SimpleSchema targets and
cannot carry `$ref`; they always expand to the unaliased
target regardless of annotation.
Adds the `alias-responses-calibration` fixture exercising all
three reach contexts under all three modes. The coverage tests
pin the contract per mode with both inline assertions and
goldens.
Cross-fixture cleanup is substantial: the
`enhancements_alias_response_ref.json` golden now carries real
`schema: $ref: Envelope` on both top-level response aliases
(previously empty schemas), and the `exportedResponse` /
`AliasedTopResponse` / `EnvelopeAlias` chain leaks vanish
across `enhancements_alias_response_shapes_ref.json`,
`enhancements_alias_findmodel_witness.json`, and
`enhancements_alias_ref.json`.
Annotated companions land alongside the unannotated witnesses
across `alias-response-shapes`, `alias-expand` (response side),
and `alias-findmodel-witness` (both parameter and response
sides) — each post-fix golden now carries both halves of the
contract on the same canvas: the unannotated path that
dissolves and the annotated path that preserves the alias name
via `swagger:model`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Before this commit, a `swagger:strfmt` annotation on a
`*types.Alias` declaration was silently dropped at every
dispatch layer:
- `classifierNamedTypeOverride` (the decl-entry classifier in
`buildFromDecl`) consumes only `swagger:type`, so a
`// swagger:strfmt date` annotation never matched there;
- the alias-specific dispatch into `buildDeclAlias` then fired
its stdlib recognizer (for `type X = any`) or walked the
underlying primitive (for `type X = int64`), in both cases
bypassing the classifier path that handles `swagger:strfmt`
for Named decls (`classifierNamedStructStrfmt` /
`classifierNamedBasic`).
Empirical pre-patch shapes:
- `type Datestamp = any` with `swagger:strfmt date` → `{}`
(annotation dropped)
- `type UserIDStrf = int64` with `swagger:strfmt uuid` →
`{integer, int64}` (annotation dropped; primitive shape
leaked through)
After this commit:
- Datestamp → `{type: string, format: date}`
- UserIDStrf → `{type: string, format: uuid}`
The patch is intentionally surgical — a one-line check at the
top of `buildDeclAlias` (before the TransparentAliases / Expand
/ $ref branches) that consults `swagger:strfmt` and writes the
canonical `{string, format}` shape when present. This keeps the
alias-specific fix isolated from Named-decl handling, where
strfmt is already correctly applied via the underlying-kind
classifiers fired from `buildNamedType`. An earlier attempt at
extending `classifierNamedTypeOverride` itself broke
`SomeTimesType []time.Time` and TestAliasedTypes because the
early strfmt fire at the use-site Named call also scalarised
slice-Named decls; the buildDeclAlias-only location avoids that
regression entirely.
Adds probe decls to the existing `ref-alias-chain` fixture
covering both the broken and the working cases (alias of any
with strfmt, alias of primitive with strfmt, alias of any with
swagger:type as a baseline that already worked). The coverage
test pins the four cells.
Out of scope:
- `swagger:enum` on alias decls — empirically also dropped,
but the semantic question of what an enum-of-int alias even
means (constants must be typed against the alias's
underlying, which is no longer the alias itself) deserves
its own scope.
- The default-shape question for unannotated `type X = any` —
the current empty body is the documented Swagger 2.0
representation of "any value allowed" (Swagger 2.0 has no
equivalent of JSON Schema `true`). Users wanting a typed
surface can now annotate with `swagger:strfmt` or
`swagger:type`. Status quo accepted.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…ocabulary
Documents the unified alias-handling contract across the three
builders (schema, parameters, responses) in their README files and
scrubs the planning-phase vocabulary that accumulated during the
work but does not belong in the disclosed surface.
Contract documentation:
- schema/README.md §aliases — extended to cover both decl shape
(mode-dependent: dissolve / $ref / expand) and use-site shape
(annotation-gated: $ref alias name with `swagger:model`,
dissolve otherwise; TransparentAliases supersedes). Adds notes
on SimpleSchema reach contexts (non-body params, response
headers), top-level swagger:parameters / swagger:response
alias declarations (never produce model definitions), and the
Rhs / Underlying / Unalias distinction across the three
branches.
- parameters/README.md §alias-handling — rewritten to defer to
the schema contract and describe the parameters-specific
reach contexts (top-level alias, body field, non-body field).
- responses/README.md §alias-handling — same treatment for the
responses builder. The historical `quirks-history` section is
untouched.
Vocabulary scrub across code, fixtures, and tests:
- Rule-letter labels and workshop-cycle references removed from
inline comments. The rules they named are now documented in
the relevant README sections, so the comments point at the
contract instead of carrying their own scaffolding.
- `.claude/plans/` path references removed (those documents are
not part of the disclosed repo surface). The two non-alias
occurrences (the YAML dedupe documentation and the
securitydefs witness) gain neutral phrasing.
- Fixture godoc packages rewritten to describe what the fixture
exercises rather than which workshop cycle produced it; the
contract pointer cross-links to the relevant builder README.
Test impact: goldens regenerate for the fixtures whose godoc text
was rewritten (the description fields land in the captured spec).
No behavioural changes — the asserted shapes are unchanged. Full
suite green; lint clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi
force-pushed
the
fix/aliased-types
branch
from
599e453 to
5c55ebb
Compare
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