Merge pull request #129 from fredbi/fix/loader-conformance

Fix/loader conformance

Author: fredbi

internal/builders/resolvers/assertions.go

@@ -228,8 +228,27 @@ func IsOpaqueStream(o *types.TypeName) bool {
 	return found
 }
 
+// IsStdJSONRawMessage reports whether o is [encoding/json.RawMessage], in either of the two shapes a
+// toolchain gives it.
+//
+// go1.27 turned RawMessage into an alias of [encoding/json/jsontext.Value], so unaliasing a field
+// written as json.RawMessage now lands on jsontext.Value, and a predicate that reads only
+// (encoding/json, RawMessage) stopped matching there: the type fell through to its []byte underlying
+// and rendered as an array of uint8 instead of "any JSON". Both names mean raw JSON text, so both
+// answer yes.
 func IsStdJSONRawMessage(o *types.TypeName) bool {
-	return o.Pkg() != nil && o.Pkg().Path() == "encoding/json" && o.Name() == "RawMessage"
+	if o == nil || o.Pkg() == nil {
+		return false
+	}
+
+	switch o.Pkg().Path() {
+	case "encoding/json":
+		return o.Name() == "RawMessage"
+	case "encoding/json/jsontext":
+		return o.Name() == "Value"
+	default:
+		return false
+	}
 }
 
 func IsAny(o *types.TypeName) bool {

internal/builders/resolvers/assertions_test.go

@@ -11,6 +11,45 @@ import (
 	"github.com/go-openapi/testify/v2/assert"
 )
 
+// TestIsStdJSONRawMessage covers both spellings of raw JSON the standard library has had.
+//
+// Synthesized rather than scanned, for the reason given on [TestIsStdUUID]: the predicate must
+// answer for a toolchain other than the one this test is built with. go1.27 turned
+// encoding/json.RawMessage into an alias of encoding/json/jsontext.Value, so a scan under go1.26
+// reaches the predicate with the named RawMessage and a scan under go1.27 reaches it with
+// jsontext.Value — through the same binary.
+func TestIsStdJSONRawMessage(t *testing.T) {
+	byteSlice := types.NewSlice(types.Typ[types.Byte])
+
+	named := func(path, name, typeName string) *types.TypeName {
+		pkg := types.NewPackage(path, name)
+		return types.NewTypeName(token.NoPos, pkg, typeName, byteSlice)
+	}
+
+	t.Run("encoding/json.RawMessage is recognized", func(t *testing.T) {
+		assert.TrueT(t, IsStdJSONRawMessage(named("encoding/json", "json", "RawMessage")))
+	})
+
+	t.Run("the jsontext.Value it aliases in go1.27 is recognized", func(t *testing.T) {
+		assert.TrueT(t, IsStdJSONRawMessage(named("encoding/json/jsontext", "jsontext", "Value")))
+	})
+
+	t.Run("another type in either package is NOT recognized", func(t *testing.T) {
+		assert.FalseT(t, IsStdJSONRawMessage(named("encoding/json", "json", "Number")))
+		assert.FalseT(t, IsStdJSONRawMessage(named("encoding/json/jsontext", "jsontext", "Pointer")))
+	})
+
+	t.Run("a third-party type of either name is NOT recognized", func(t *testing.T) {
+		assert.FalseT(t, IsStdJSONRawMessage(named("github.com/goccy/go-json", "json", "RawMessage")))
+		assert.FalseT(t, IsStdJSONRawMessage(named("example.com/mymod/jsontext", "jsontext", "Value")))
+	})
+
+	t.Run("a builtin (no package) does not panic", func(t *testing.T) {
+		builtin := types.NewTypeName(token.NoPos, nil, "error", nil)
+		assert.FalseT(t, IsStdJSONRawMessage(builtin))
+	})
+}
+
 // TestIsStdUUID covers the go1.27 stdlib uuid.UUID predicate.
 //
 // Deliberately synthesizes the *types.TypeName instead of scanning a fixture, so the predicate stays

internal/builders/responses/responses.go

@@ -239,10 +239,16 @@ func (r *Builder) buildFromType(otpe types.Type, resp *oaispec.Response, seen ma
 	}
 }
 
-// namedWrittenRHS reports a declaration whose written right-hand side is itself a NAMED type, together with that type.
+// namedWrittenRHS reports a declaration whose written right-hand side names a DECLARED type, together with that type.
 //
-// Only a named right-hand side redirects the build: a struct literal, a slice or a basic type is already the shape the
-// response arm should build, and sending those through the sub-builder would publish the response type as a definition.
+// Only a declared right-hand side redirects the build: a struct literal, a slice or a basic type is already the shape
+// the response arm should build, and sending those through the sub-builder would publish the response type as a
+// definition.
+//
+// An alias counts, since it too names a declared type: go1.27 made `encoding/json.RawMessage` an alias of
+// jsontext.Value, so `type DefinedRaw json.RawMessage` writes an alias on the right and a gate reading only
+// *types.Named stopped redirecting it — the response fell through to the []byte underlying and came out as an array of
+// uint8. The alias itself is handed on, so the schema builder applies the alias policy the declaration asks for.
 func namedWrittenRHS(ctx *scanner.ScanCtx, o *types.TypeName) (*scanner.EntityDecl, types.Type, bool) {
 	decl, found := ctx.DeclForType(o.Type())
 	if !found {
@@ -252,7 +258,7 @@ func namedWrittenRHS(ctx *scanner.ScanCtx, o *types.TypeName) (*scanner.EntityDe
 	if !ok {
 		return nil, nil, false
 	}
-	if _, isNamed := rhs.(*types.Named); !isNamed {
+	if _, isNamed := types.Unalias(rhs).(*types.Named); !isNamed {
 		return nil, nil, false
 	}
 

internal/builders/schema/README.md

@@ -214,16 +214,28 @@ distinction is the whole reason both exist:
 Neither beats an explicit `swagger:strfmt` / `swagger:type` — the
 classifier runs first, see [§textmarshal-order](#textmarshal-order).
 
-**Not reached by either: a struct embed.** `uuid.UUID` has an array
-underlying type, so `buildNamedEmbedded` falls to its `default` arm
-(only the *interface* arm consults `applyStdlibSpecials`) and skips
-the embed with a `CodeUnsupportedGoType` warning. That asymmetry is
-long-standing and uuid-agnostic — any `strfmt.UUID` / `time.Time`
-embed behaves the same, and Go itself renders such a struct as a bare
-string via the promoted `MarshalText`. Tracked as Q33; the identity
-recognizer deliberately does not change it. Witnessed by
+**A struct embed reaches them through the property path.**
+`uuid.UUID` has an array underlying type, so embedding it promotes no
+member: `embedPromotes` sends it to `applyFieldCarrier` as an ordinary
+property named after the type, and the recognizers run there like on
+any other field — `Embedder` carries `UUID: {string, uuid}`. Go's
+default marshaller renders the whole struct as a bare string instead,
+via the promoted `MarshalText`, which codescan does not model; see
+[§embed-marshaller](#embed-marshaller) and Q33. Witnessed by
 `TestStdlibUUID`'s embed sub-test.
 
+**`recognizeRawMessage` answers to two names.** go1.27 turned
+`encoding/json.RawMessage` into an alias of
+`encoding/json/jsontext.Value`, so unaliasing a field written as
+`json.RawMessage` now lands on `jsontext.Value`.
+`resolvers.IsStdJSONRawMessage` matches both — a predicate reading
+only `(encoding/json, RawMessage)` let the type fall through to its
+`[]byte` underlying and render as an array of `uint8`. The same
+change makes an alias the ordinary shape of a written right-hand side
+(`type DefinedRaw json.RawMessage`), which is why the responses
+builder's `namedWrittenRHS` unaliases before testing for a declared
+type.
+
 The seven call sites of `applyStdlibSpecials` (`buildFromDecl`,
 `buildDeclAlias` RHS, `buildAlias`, `buildNamedType`,
 `buildNamedEmbedded` interface arm, `processEmbeddedType` named arm,

internal/integration/response_specials_test.go

@@ -34,12 +34,14 @@ func TestResponseSpecials(t *testing.T) {
 		}
 	})
 
-	t.Run("the two spellings agree", func(t *testing.T) {
+	t.Run("the three spellings agree", func(t *testing.T) {
 		for defined, aliased := range map[string]string{
 			"definedStamp":  "aliasedStamp",
 			"definedRaw":    "aliasedRaw",
 			"definedStream": "aliasedStream",
 			"definedWhole":  "aliasedWhole",
+			"viaAliasStamp": "aliasedStamp",
+			"viaAliasRaw":   "aliasedRaw",
 		} {
 			d, a := doc.Responses[defined].Schema, doc.Responses[aliased].Schema
 			require.NotNil(t, d)
@@ -71,5 +73,21 @@ func TestResponseSpecials(t *testing.T) {
 		assert.Empty(t, raw.Type)
 	})
 
+	t.Run("a declaration written over an ALIAS reaches the recognizers", func(t *testing.T) {
+		// `type ViaAliasStamp Stamped` writes an alias on the right, and the redirect that carries the
+		// named layer to the recognizers used to demand a *types.Named there. go1.27 makes this the
+		// ordinary case rather than a curiosity: encoding/json.RawMessage became an alias of
+		// jsontext.Value, so `type DefinedRaw json.RawMessage` takes this path too.
+		stamp := doc.Responses["viaAliasStamp"].Schema
+		require.NotNil(t, stamp)
+		require.Len(t, stamp.Type, 1)
+		assert.Equal(t, "string", stamp.Type[0])
+		assert.Equal(t, "date-time", stamp.Format)
+
+		raw := doc.Responses["viaAliasRaw"].Schema
+		require.NotNil(t, raw)
+		assert.Empty(t, raw.Type)
+	})
+
 	scantest.CompareOrDumpJSON(t, doc, "enhancements_response_specials.json")
 }

internal/integration/schema_uuid_go127_test.go

@@ -105,29 +105,28 @@ func TestStdlibUUID(t *testing.T) {
 		assertUUID(t, tagged.Properties["alias"])
 	})
 
-	t.Run("an EMBEDDED uuid.UUID is dropped, with a warning — quirks-open.md Q33", func(t *testing.T) {
-		// Current behaviour, not desired behaviour. uuid.UUID has an array underlying type, so
-		// buildNamedEmbedded's switch falls to its default arm (only the *interface* arm consults
-		// applyStdlibSpecials) and skips the embed. Go itself renders the whole struct as a bare
-		// string via the promoted MarshalText, so "name" is not on the wire either.
+	t.Run("an EMBEDDED uuid.UUID becomes a property named after the type", func(t *testing.T) {
+		// uuid.UUID is an array underneath, so embedding it promotes no member and Go keeps the value
+		// as an ordinary key named UUID — which is what the schema says, built through the identity
+		// recognizer like any other member. See embed-basic-underlying for the general rule.
 		//
-		// This is a pre-existing, uuid-agnostic quirk shared with every strfmt.UUID / time.Time
-		// embed; the identity recognizer deliberately does not change it. When Q33 is decided, this
-		// sub-test and the golden move together.
+		// The promoted MarshalText makes the DEFAULT marshaller render the whole struct as a bare
+		// string, dropping "name" from the wire. codescan does not model that, by decision: an embed
+		// means composition, and a composed model round-trips through a hand-written marshaller. See
+		// schema/README.md#embed-marshaller.
 		embedder, ok := sp.Definitions["Embedder"]
 		require.TrueT(t, ok)
 		assert.TrueT(t, embedder.Type.Contains("object"))
-		assert.Len(t, embedder.Properties, 1)
+		assert.Len(t, embedder.Properties, 2)
 		assert.Contains(t, embedder.Properties, "name")
+		require.Contains(t, embedder.Properties, "UUID")
+		assertUUID(t, embedder.Properties["UUID"])
 
-		var warned bool
+		// The embed is emitted, so nothing about it is skipped or reported.
 		for _, d := range diagnostics {
-			if d.Severity == codescan.SeverityWarning && strings.Contains(d.Message, "buildNamedEmbedded") {
-				warned = true
-				break
-			}
+			assert.FalseT(t, d.Severity == codescan.SeverityWarning && strings.Contains(d.Message, "buildNamedEmbedded"),
+				"an embed that is emitted must raise no unsupported-type warning: %s", d.Message)
 		}
-		assert.TrueT(t, warned, "the dropped embed must not be silent")
 	})
 
 	scantest.CompareOrDumpJSON(t, sp, "go127_uuid_spec.json")

testdata/enhancements/response-specials/api.go

@@ -18,4 +18,6 @@ package response_specials
 //	212: aliasedStream
 //	213: aliasedWhole
 //	214: aliasedFraction
+//	220: viaAliasStamp
+//	221: viaAliasRaw
 func getSpecial() {}

testdata/enhancements/response-specials/types.go

@@ -55,3 +55,19 @@ type AliasedWhole = big.Int
 
 // swagger:response aliasedFraction
 type AliasedFraction = big.Rat
+
+// Local-alias spelling — the declaration is written over an alias, which is what go1.27 made of
+// json.RawMessage: `type DefinedRaw json.RawMessage` now has an ALIAS on its right-hand side. The
+// redirect that carries the named layer to the recognizers has to follow that too.
+
+// Stamped is a local alias of time.Time.
+type Stamped = time.Time
+
+// Rawish is a local alias of json.RawMessage.
+type Rawish = json.RawMessage
+
+// swagger:response viaAliasStamp
+type ViaAliasStamp Stamped
+
+// swagger:response viaAliasRaw
+type ViaAliasRaw Rawish

testdata/goparsing/go127/uuid/model.go

@@ -55,8 +55,13 @@ type Tagged struct {
 // Embedder embeds the stdlib uuid.UUID.
 //
 // Witness for the embedded arm, which the fuzzy name heuristic never reached (it is caller-gated to
-// buildFromTextMarshal). Note the promoted MarshalText makes encoding/json render the WHOLE struct
-// as a bare string, dropping Name from the wire — see quirks-open.md Q33.
+// buildFromTextMarshal). uuid.UUID is an array underneath, so the embed promotes nothing and Go
+// keeps it as an ordinary key named after the type: the schema carries a UUID property, built
+// through the identity recognizer like any other member.
+//
+// The promoted MarshalText makes the DEFAULT marshaller render the whole struct as a bare string,
+// dropping Name from the wire. codescan does not model that — an embed means composition here — see
+// the schema builder README, section embed-marshaller.
 //
 // swagger:model Embedder
 type Embedder struct {

testdata/integration/golden/enhancements_response_specials.json

@@ -35,6 +35,12 @@
           },
           "214": {
             "$ref": "#/responses/aliasedFraction"
+          },
+          "220": {
+            "$ref": "#/responses/viaAliasStamp"
+          },
+          "221": {
+            "$ref": "#/responses/viaAliasRaw"
           }
         }
       }
@@ -99,6 +105,17 @@
         "type": "integer",
         "x-go-type": "math/big.Int"
       }
+    },
+    "viaAliasRaw": {
+      "description": "",
+      "schema": {}
+    },
+    "viaAliasStamp": {
+      "description": "",
+      "schema": {
+        "type": "string",
+        "format": "date-time"
+      }
     }
   }
 }