Fix/clone spec through json

.claude/rules/technical-writing.md

@@ -0,0 +1,67 @@
+---
+paths:
+  - "**/*.go"
+  - "**/*.md"
+---
+
+# Technical writing style (go-openapi)
+
+Applies to every committed comment, commit message, README and doc-site page.
+
+The standard is Ernest Gowers, *Plain Words*: **be short, be simple, be human.**
+His worked example is the whole rule:
+
+    DON'T   Was this the realisation of an anticipated liability?
+    DO      Did you expect to have to do this?
+
+The abstract nouns carry no information; the concrete verb carries all of it.
+
+## Two tests
+
+**The grep test.** Does the sentence contain something a reader can search for — an
+identifier, a file, a flag, an error, a number with a unit? Prose that names nothing has
+described the code without pointing at it.
+
+**The quotability test.** A sentence that would survive being quoted on its own is too
+pleased with itself. Rewrite it until it merely sounds true.
+
+## Never define by inversion
+
+The worst and most frequent fault. A copula whose subject or predicate is a wh-clause
+promises a definition and delivers a metaphor. Both directions are banned:
+
+    DON'T   Coverage is what says which templates a suite never reaches.
+    DON'T   What is lost is the doc comment.
+    DO      Coverage records which templates the suite never executed.
+    DO      A synthesized type loses its doc comment.
+
+The rewrite is mechanical: find the verb hiding inside the wh-clause and make it the main
+verb of the sentence.
+
+`which is why` pointing back at a fact just stated is legitimate, and rationed — one per
+comment is plenty.
+
+## The rest
+
+- **Name the thing.** `WithRoots`, not "the option that scopes a repository". Name the
+  error, the file, the flag, the upstream package, the constant.
+- **Statement, not aphorism.** State mechanism and effect. Never close a paragraph on a
+  maxim: the reflex lands hardest on a closing sentence.
+- **Keep a subject.** "New returns an error if the source is unreadable", not "What a
+  source leaves out is settled where it is declared".
+- **Plain verbs.** add, fix, return, parse, reject, cap, prune, record. Code does not say,
+  judge, grant, refuse, know, mean to, or reach for. `report` is fine when something
+  genuinely reports.
+- **Keep the numbers.** Sizes with units, counts, ratios, advisory ids. `286 -> 178 KiB`,
+  `GHSA-v2xp-g8xf-22pf`. Dropping them for a smoother sentence loses information.
+- **Be human.** Address the reader where there is advice: "Use `WithRoot` to confine local
+  loading." Admit the awkward thing rather than smoothing it over.
+
+## Self-check
+
+    # definition by inversion, both directions
+    grep -rnE '\b(is|are) (what|where) [a-z]' --include='*.go' --include='*.md' .
+    grep -rnE '(^|\. )What [a-z][a-z ,-]{3,50} (is|are) ' --include='*.go' --include='*.md' .
+
+Subtract the legitimate `which/that/this/it is what` before judging the first one.
+Neither grep is a verdict — they find one fault out of six. The others need reading.

clone_test.go

@@ -0,0 +1,100 @@
+// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
+// SPDX-License-Identifier: Apache-2.0
+
+package loads
+
+import (
+	_ "embed"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/go-openapi/testify/v2/assert"
+	"github.com/go-openapi/testify/v2/require"
+)
+
+//go:embed testdata/json/zero-valued-bounds.json
+var zeroValuedBoundsJSON []byte
+
+// TestCloneSpecCarriesZeroValuedBounds covers what gob used to drop from the copy Analyzed takes.
+//
+// gob omits a struct field holding the zero value for its type and flattens a pointer to what it
+// points at, so an optional number that was present and zero came back nil. "minimum": 0,
+// "maximum": 0, "maxLength": 0, "maxItems": 0 and "maxProperties": 0 all disappeared, at any
+// depth and with no error - from OrigSpec, which go-swagger marshals into the specification it
+// embeds in a generated server, and from the definitions ResetDefinitions copies back.
+func TestCloneSpecCarriesZeroValuedBounds(t *testing.T) {
+	t.Parallel()
+
+	document, err := Analyzed(json.RawMessage(zeroValuedBoundsJSON), "")
+	require.NoError(t, err)
+
+	t.Run("the copy agrees with the document it was taken from", func(t *testing.T) {
+		assert.JSONMarshalAsT(t, zeroValuedBoundsJSON, document.OrigSpec())
+	})
+
+	t.Run("resetting the definitions keeps them", func(t *testing.T) {
+		document.ResetDefinitions()
+		assert.JSONMarshalAsT(t, zeroValuedBoundsJSON, document.Spec())
+	})
+
+	t.Run("every bound in the fixture is a zero", func(t *testing.T) {
+		// guards the fixture itself: a bound respelled to a non-zero value would make the
+		// test pass without exercising anything
+		var raw map[string]any
+		require.NoError(t, json.Unmarshal(zeroValuedBoundsJSON, &raw))
+
+		bounds := countZeroBounds(raw)
+		require.Positive(t, bounds.total, "the fixture holds no bound at all")
+		assert.EqualTf(t, bounds.total, bounds.zero,
+			"%d of the fixture's %d bounds are not zero", bounds.total-bounds.zero, bounds.total)
+	})
+}
+
+type boundCount struct{ total, zero int }
+
+// countZeroBounds walks raw JSON and counts the optional numbers gob elides.
+func countZeroBounds(node any) boundCount {
+	var count boundCount
+
+	switch value := node.(type) {
+	case map[string]any:
+		for key, child := range value {
+			if isBoundKeyword(key) {
+				number, ok := child.(float64)
+				if ok {
+					count.total++
+					if number == 0 {
+						count.zero++
+					}
+
+					continue
+				}
+			}
+
+			nested := countZeroBounds(child)
+			count.total += nested.total
+			count.zero += nested.zero
+		}
+	case []any:
+		for _, child := range value {
+			nested := countZeroBounds(child)
+			count.total += nested.total
+			count.zero += nested.zero
+		}
+	}
+
+	return count
+}
+
+func isBoundKeyword(key string) bool {
+	switch key {
+	case "minimum", "maximum", "multipleOf",
+		"minLength", "maxLength",
+		"minItems", "maxItems",
+		"minProperties", "maxProperties":
+		return true
+	default:
+		return strings.HasPrefix(key, "min") || strings.HasPrefix(key, "max")
+	}
+}

go.mod

@@ -3,6 +3,7 @@ module github.com/go-openapi/loads
 require (
 	github.com/go-openapi/analysis v0.26.0
 	github.com/go-openapi/spec v0.22.9
+	github.com/go-openapi/swag/jsonutils v0.29.0
 	github.com/go-openapi/swag/loading v0.29.0
 	github.com/go-openapi/swag/yamlutils v0.29.0
 	github.com/go-openapi/testify/enable/yaml/v2 v2.6.1
@@ -16,7 +17,6 @@ require (
 	github.com/go-openapi/jsonreference v1.0.0 // indirect
 	github.com/go-openapi/strfmt v0.27.0 // indirect
 	github.com/go-openapi/swag/conv v0.29.0 // indirect
-	github.com/go-openapi/swag/jsonutils v0.29.0 // indirect
 	github.com/go-openapi/swag/mangling v0.28.0 // indirect
 	github.com/go-openapi/swag/pools v0.29.0 // indirect
 	github.com/go-openapi/swag/stringutils v0.28.0 // indirect

spec.go

@@ -5,21 +5,16 @@ package loads
 
 import (
 	"bytes"
-	"encoding/gob"
 	"encoding/json"
 	"fmt"
 	"maps"
 
 	"github.com/go-openapi/analysis"
 	"github.com/go-openapi/spec"
+	"github.com/go-openapi/swag/jsonutils"
 	"github.com/go-openapi/swag/yamlutils"
 )
 
-func init() {
-	gob.Register(map[string]any{})
-	gob.Register([]any{})
-}
-
 // Document represents a swagger spec document.
 type Document struct {
 	// specAnalyzer
@@ -276,14 +271,16 @@ func (d *Document) SpecFilePath() string {
 	return d.specFilePath
 }
 
+// cloneSpec deep-copies a specification through its JSON representation.
+//
+// It used to go through gob, which omits a field holding the zero value for its type and
+// flattens a pointer to what it points at, so an optional number that was present and zero came
+// back nil: "minimum": 0, "maximum": 0, "maxLength": 0, "maxItems": 0 and "maxProperties": 0 were
+// dropped from the copy, at any depth and with no error. That copy is what OrigSpec returns and
+// what ResetDefinitions writes back over the live definitions.
 func cloneSpec(src *spec.Swagger) (*spec.Swagger, error) {
-	var b bytes.Buffer
-	if err := gob.NewEncoder(&b).Encode(src); err != nil {
-		return nil, err
-	}
-
 	var dst spec.Swagger
-	if err := gob.NewDecoder(&b).Decode(&dst); err != nil {
+	if err := jsonutils.FromDynamicJSON(src, &dst); err != nil {
 		return nil, err
 	}
 

testdata/json/zero-valued-bounds.json

@@ -0,0 +1,82 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "every optional number a swagger document may hold, set to zero",
+    "version": "1.0.0"
+  },
+  "basePath": "/",
+  "paths": {
+    "/things": {
+      "get": {
+        "operationId": "listThings",
+        "parameters": [
+          {
+            "name": "count",
+            "in": "query",
+            "type": "integer",
+            "minimum": 0,
+            "maximum": 0,
+            "multipleOf": 0
+          },
+          {
+            "name": "tags",
+            "in": "query",
+            "type": "array",
+            "items": {
+              "type": "string",
+              "maxLength": 0,
+              "minLength": 0
+            },
+            "maxItems": 0,
+            "minItems": 0
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "ok",
+            "schema": {
+              "$ref": "#/definitions/thing"
+            },
+            "headers": {
+              "X-Rate": {
+                "type": "integer",
+                "minimum": 0,
+                "maximum": 0
+              }
+            }
+          }
+        }
+      }
+    }
+  },
+  "definitions": {
+    "thing": {
+      "type": "object",
+      "maxProperties": 0,
+      "minProperties": 0,
+      "properties": {
+        "size": {
+          "type": "integer",
+          "minimum": 0,
+          "maximum": 0,
+          "multipleOf": 0
+        },
+        "name": {
+          "type": "string",
+          "maxLength": 0,
+          "minLength": 0
+        },
+        "parts": {
+          "type": "array",
+          "maxItems": 0,
+          "minItems": 0,
+          "items": {
+            "type": "number",
+            "minimum": 0,
+            "maximum": 0
+          }
+        }
+      }
+    }
+  }
+}