Skip to content

fix: resolve a $ref through an import alias in a map - #120

Merged
seanogdev merged 13 commits into
masterfrom
fix/map-ref-import-alias
Sep 24, 2026
Merged

seanogdev merged 13 commits into
masterfrom
fix/map-ref-import-alias

Conversation

@seanogdev

@seanogdev seanogdev commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

A map value type that the Go source reaches through an import alias got a $ref under the alias. Kommentaar keeps the definition under the package name, so the reference pointed at nothing, and only dbg reported it.

Changes

  • resolveMap names the $ref after the definition that GetReference stored. An import alias or a base name clash no longer gives a reference to nothing or to the wrong type.
  • A named slice value such as map[string]bars gives an array of its element, the same as fieldToSchema does.
  • resolveMap reuses a stored definition before it calls GetReference.
  • A value that GetReference cannot resolve, such as map[string]pkg.StatusType, gives an open object, not a dangling $ref.
  • resolveMap still sends the GetReference error to dbg alone. A separate change makes it a failure.
  • A base name clash in a slice element or a plain struct field is not fixed here. resolveArray and fieldToSchema still name the $ref by base name.
  • The resolveMap tests assert that every $ref in the schema has a definition in prog.References.

Focus areas

  • find.go: GetReference takes its key from the new referenceLookup. The key rule did not change.
  • jsonschema.go: resolveMap calls referenceLookup first, because GetReference does not find a stored definition by its full import path. Without the check, a self-referencing map value recursed until the stack overflowed.
Files changed in docparse/
File Change
find.go Key rule moved into referenceLookup
jsonschema.go Map value names, named slices, stored lookup
jsonschema_test.go Tests for resolveMap
testdata/src/a/a.go Map fixtures, aliased and dotted imports
testdata/src/c/c.go Types for the aliased import
testdata/src/d/c/c.go Types for the clash and a self reference
testdata/src/example.com/m/m.go Types on a dotted import path

Screenshots

resolveMap had no tests. The tests cover primitive, pointer, any, struct,
cross-package struct, slice and nested map value types.

Each test also makes sure that prog.References defines every $ref that the
schema gives. A reference that nothing defines makes an unusable document.
lookupTypeAndRef built the reference from the selector as written in the Go
source. For a type that the file reaches through an import alias, that is the
alias, but GetReference keeps the definition under the package name. The
reference then pointed at a definition that does not exist.

The reference name and the GetReference lookup now come from the import path
that findType resolves.
The tests now cover map[string][]aliased.Nested, which goes through
resolveArray. lookupTypeAndRef no longer gives a JSON Schema type, because no
caller uses it.
A scratch probe for map[string]b.StatusType reached the tree by accident. The case fails, because resolveMap gives a reference for a named primitive.
Two packages that share a base name give one definition the base name and the
other a numbered name. resolveMap built the name on its own, so both maps
pointed at the first definition. An alias goes with this case, because a name
clash is a reason to write one.

The reference now comes from what GetReference stored. The name that
lookupTypeAndRef gives stays for the error path.
@seanogdev
seanogdev requested a review from shane-tw September 22, 2026 14:12
@coveralls

coveralls commented Sep 22, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 36029501958

Coverage increased (+2.0%) to 59.71%

Details

  • Coverage increased (+2.0%) from the base build.
  • Patch coverage: 5 uncovered changes across 1 file (28 of 33 lines covered, 84.85%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
docparse/jsonschema.go 20 15 75.0%
Total (2 files) 33 28 84.85%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2832
Covered Lines: 1691
Line Coverage: 59.71%
Coverage Strength: 39.25 hits per line

💛 - Coveralls

Comment thread docparse/jsonschema.go Outdated
Comment thread docparse/jsonschema.go Outdated
Comment thread docparse/jsonschema.go Outdated
resolveMap now sets additionalProperties only after GetReference succeeds, so a failed lookup no longer leaves a $ref that points at nothing.
resolveMap sent a named slice such as bars to GetReference. That gave a $ref to one element, and for a package on a dotted import path it gave a $ref to nothing. It now resolves the element through resolveArray, the same as fieldToSchema does.
resolveMap sent the full import path to GetReference. GetReference stores a definition by base name, so the full lookup never found it. It parsed the type again and replaced its context. resolveMap now checks for the stored key first.
@seanogdev

Copy link
Copy Markdown
Contributor Author

A local review of this branch raised these points.

  • Full import path sent to GetReference: a named slice value now resolves through resolveArray (5caba0e). resolveMap now finds a stored definition by package before it calls GetReference (8044f31, 0de9f99).
  • vref.IsSlice ignored: map[string]bars now gives an array of a.bar (5caba0e).
  • Base name clash in a slice value: out of scope. fieldToSchema has the same fault for a plain struct field, so the fix belongs in a separate change. The PR body records it.
  • Weak clash test: the test now asserts c.Nested from c and c.Nested2 from d/c (d087f35).
  • lookupTypeAndRef and its dead return: inlined, and additionalProperties is set only after GetReference succeeds (c57d91f).
  • Duplicated alias resolution: declined. The filepath.Base copy went with lookupTypeAndRef. A shared helper would change fieldToSchema and resolveArray, which the clash change will rework.
  • sRef, lref and vref: removed with lookupTypeAndRef (c57d91f).
  • Long and contradictory comments: replaced with one two-line comment (0de9f99).
  • Unchecked assertions in the clash test: the test now checks the map assertion and additionalProperties (d087f35).
  • Repeated test setup: declined. The four older tests in jsonschema_test.go repeat the same findType setup.

@seanogdev
seanogdev merged commit b74d3f7 into master Sep 24, 2026
3 checks passed
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.

3 participants