Relinting (2)

* addressed linting issues, especially those related to non-wrapping
  errors: refactored all calls to fmt.Errorf()
* fixes #114: unfortunate copy paste typo in mixin.go

* test: worked around go issue on windows: golang/go#71544

Signed-off-by: Frederic BIDON <[email protected]>

Author: fredbi

analyzer.go

@@ -681,7 +681,7 @@ func (s *Spec) paramsAsMap(parameters []spec.Parameter, res map[string]spec.Para
 
 		obj, _, err := pr.Ref.GetPointer().Get(s.spec)
 		if err != nil {
-			if callmeOnError(param, fmt.Errorf("invalid reference: %q", pr.Ref.String())) {
+			if callmeOnError(param, ErrInvalidRef(pr.Ref.String())) {
 				continue
 			}
 
@@ -690,7 +690,7 @@ func (s *Spec) paramsAsMap(parameters []spec.Parameter, res map[string]spec.Para
 
 		objAsParam, ok := obj.(spec.Parameter)
 		if !ok {
-			if callmeOnError(param, fmt.Errorf("resolved reference is not a parameter: %q", pr.Ref.String())) {
+			if callmeOnError(param, ErrInvalidParameterRef(pr.Ref.String())) {
 				continue
 			}
 

analyzer_test.go

@@ -19,6 +19,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/go-openapi/analysis/internal/antest"
@@ -426,8 +427,8 @@ func TestAnalyzer_ParamsAsMapWithCallback(t *testing.T) {
 		return true // Continue
 	})
 
-	assert.Contains(t, e, `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
-	assert.Contains(t, e, `invalid reference: "#/definitions/sample_info/properties/sids"`)
+	assert.Contains(t, strings.Join(e, ","), `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
+	assert.Contains(t, strings.Join(e, ","), `invalid reference: "#/definitions/sample_info/properties/sids"`)
 
 	// bail out callback
 	m = make(map[string]spec.Parameter)
@@ -533,8 +534,8 @@ func TestAnalyzer_SafeParamsFor(t *testing.T) {
 		require.Fail(t, "There should be no safe parameter in this testcase")
 	}
 
-	assert.Contains(t, e, `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
-	assert.Contains(t, e, `invalid reference: "#/definitions/sample_info/properties/sids"`)
+	assert.Contains(t, strings.Join(e, ","), `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
+	assert.Contains(t, strings.Join(e, ","), `invalid reference: "#/definitions/sample_info/properties/sids"`)
 }
 
 func TestAnalyzer_ParamsFor(t *testing.T) {
@@ -581,8 +582,8 @@ func TestAnalyzer_SafeParametersFor(t *testing.T) {
 		require.Fail(t, "There should be no safe parameter in this testcase")
 	}
 
-	assert.Contains(t, e, `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
-	assert.Contains(t, e, `invalid reference: "#/definitions/sample_info/properties/sids"`)
+	assert.Contains(t, strings.Join(e, ","), `resolved reference is not a parameter: "#/definitions/sample_info/properties/sid"`)
+	assert.Contains(t, strings.Join(e, ","), `invalid reference: "#/definitions/sample_info/properties/sids"`)
 }
 
 func TestAnalyzer_ParametersFor(t *testing.T) {

errors.go

@@ -0,0 +1,53 @@
+package analysis
+
+import (
+	"errors"
+	"fmt"
+)
+
+type analysisError string
+
+const (
+	ErrAnalysis analysisError = "analysis error"
+	ErrNoSchema analysisError = "no schema to analyze"
+)
+
+func (e analysisError) Error() string {
+	return string(e)
+}
+
+func ErrAtKey(key string, err error) error {
+	return errors.Join(
+		fmt.Errorf("key %s: %w", key, err),
+		ErrAnalysis,
+	)
+}
+
+func ErrInvalidRef(key string) error {
+	return fmt.Errorf("invalid reference: %q: %w", key, ErrAnalysis)
+}
+
+func ErrInvalidParameterRef(key string) error {
+	return fmt.Errorf("resolved reference is not a parameter: %q: %w", key, ErrAnalysis)
+}
+
+func ErrResolveSchema(err error) error {
+	return errors.Join(
+		fmt.Errorf("could not resolve schema: %w", err),
+		ErrAnalysis,
+	)
+}
+
+func ErrRewriteRef(key string, target any, err error) error {
+	return errors.Join(
+		fmt.Errorf("failed to rewrite ref for key %q at %v: %w", key, target, err),
+		ErrAnalysis,
+	)
+}
+
+func ErrInlineDefinition(key string, err error) error {
+	return errors.Join(
+		fmt.Errorf("error while creating definition %q from inline schema: %w", key, err),
+		ErrAnalysis,
+	)
+}

flatten.go

@@ -15,7 +15,6 @@
 package analysis
 
 import (
-	"fmt"
 	"log"
 	"path"
 	"sort"
@@ -251,7 +250,7 @@ func nameInlinedSchemas(opts *FlattenOpts) error {
 
 		asch, err := Schema(SchemaOpts{Schema: sch.Schema, Root: opts.Swagger(), BasePath: opts.BasePath})
 		if err != nil {
-			return fmt.Errorf("schema analysis [%s]: %w", key, err)
+			return ErrAtKey(key, err)
 		}
 
 		if asch.isAnalyzedAsComplex() { // move complex schemas to definitions
@@ -320,7 +319,7 @@ func importNewRef(entry sortref.RefRevIdx, refStr string, opts *FlattenOpts) err
 
 	sch, err := spec.ResolveRefWithBase(opts.Swagger(), &entry.Ref, opts.ExpandOpts(false))
 	if err != nil {
-		return fmt.Errorf("could not resolve schema: %w", err)
+		return ErrResolveSchema(err)
 	}
 
 	// at this stage only $ref analysis matters
@@ -335,7 +334,7 @@ func importNewRef(entry sortref.RefRevIdx, refStr string, opts *FlattenOpts) err
 	// now rewrite those refs with rebase
 	for key, ref := range partialAnalyzer.references.allRefs {
 		if err := replace.UpdateRef(sch, key, spec.MustCreateRef(normalize.RebaseRef(entry.Ref.String(), ref.String()))); err != nil {
-			return fmt.Errorf("failed to rewrite ref for key %q at %s: %w", key, entry.Ref.String(), err)
+			return ErrRewriteRef(key, entry.Ref.String(), err)
 		}
 	}
 
@@ -429,7 +428,7 @@ func importExternalReferences(opts *FlattenOpts) (bool, error) {
 			ref := spec.MustCreateRef(r.path)
 			sch, err := spec.ResolveRefWithBase(opts.Swagger(), &ref, opts.ExpandOpts(false))
 			if err != nil {
-				return false, fmt.Errorf("could not resolve schema: %w", err)
+				return false, ErrResolveSchema(err)
 			}
 
 			r.schema = sch
@@ -666,7 +665,7 @@ func namePointers(opts *FlattenOpts) error {
 
 		result, err := replace.DeepestRef(opts.Swagger(), opts.ExpandOpts(false), ref)
 		if err != nil {
-			return fmt.Errorf("at %s, %w", k, err)
+			return ErrAtKey(k, err)
 		}
 
 		replacingRef := result.Ref
@@ -697,7 +696,7 @@ func namePointers(opts *FlattenOpts) error {
 		// update current replacement, which may have been updated by previous changes of deeper elements
 		result, erd := replace.DeepestRef(opts.Swagger(), opts.ExpandOpts(false), v.Ref)
 		if erd != nil {
-			return fmt.Errorf("at %s, %w", key, erd)
+			return ErrAtKey(key, erd)
 		}
 
 		if opts.flattenContext != nil {
@@ -743,7 +742,7 @@ func flattenAnonPointer(key string, v SchemaRef, refsToReplace map[string]Schema
 	// qualify the expanded schema
 	asch, ers := Schema(SchemaOpts{Schema: v.Schema, Root: opts.Swagger(), BasePath: opts.BasePath})
 	if ers != nil {
-		return fmt.Errorf("schema analysis [%s]: %w", key, ers)
+		return ErrAtKey(key, ers)
 	}
 	callers := make([]string, 0, allocMediumMap)
 
@@ -753,7 +752,7 @@ func flattenAnonPointer(key string, v SchemaRef, refsToReplace map[string]Schema
 	for k, w := range an.references.allRefs {
 		r, err := replace.DeepestRef(opts.Swagger(), opts.ExpandOpts(false), w)
 		if err != nil {
-			return fmt.Errorf("at %s, %w", key, err)
+			return ErrAtKey(key, err)
 		}
 
 		if opts.flattenContext != nil {

flatten_name.go

@@ -43,7 +43,7 @@ func (isn *InlineSchemaNamer) Name(key string, schema *spec.Schema, aschema *Ana
 		debugLog("rewriting schema to ref: key=%s with new name: %s", key, newName)
 		if err := replace.RewriteSchemaToRef(isn.Spec, key,
 			spec.MustCreateRef(path.Join(definitionsPath, newName))); err != nil {
-			return fmt.Errorf("error while creating definition %q from inline schema: %w", newName, err)
+			return ErrInlineDefinition(newName, err)
 		}
 
 		// rewrite any dependent $ref pointing to this place,
@@ -54,7 +54,7 @@ func (isn *InlineSchemaNamer) Name(key string, schema *spec.Schema, aschema *Ana
 		for k, v := range an.references.allRefs {
 			r, erd := replace.DeepestRef(isn.opts.Swagger(), isn.opts.ExpandOpts(false), v)
 			if erd != nil {
-				return fmt.Errorf("at %s, %w", k, erd)
+				return ErrAtKey(k, erd)
 			}
 
 			if isn.opts.flattenContext != nil {

internal/antest/helpers_test.go

@@ -2,6 +2,7 @@ package antest
 
 import (
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -54,7 +55,7 @@ func prepareBadDoc(t testing.TB, kind string, invalidFormat bool) (string, func(
 
 	switch kind {
 	case "yaml", "yml":
-		f, err := os.CreateTemp("", "*.yaml")
+		f, err := os.CreateTemp(workaroundTempDir(t)(), "*.yaml")
 		require.NoError(t, err)
 		file = f.Name()
 
@@ -72,7 +73,7 @@ info:
 		}
 
 	case "json":
-		f, err := os.CreateTemp("", "*.json")
+		f, err := os.CreateTemp(workaroundTempDir(t)(), "*.json")
 		require.NoError(t, err)
 		file = f.Name()
 
@@ -98,7 +99,18 @@ info:
 		os.WriteFile(file, data, 0600),
 	)
 
-	return file, func() {
-		_ = os.RemoveAll(file)
+	return file, func() {}
+}
+
+func workaroundTempDir(t testing.TB) func() string {
+	// Workaround for go testing bug on Windows: https://github.com/golang/go/issues/71544
+	// On windows, t.TempDir() doesn't properly release file handles yet,
+	// se we just leave it unchecked (no cleanup would take place).
+	if runtime.GOOS == "windows" {
+		return func() string {
+			return ""
+		}
 	}
+
+	return t.TempDir
 }

internal/debug/debug_test.go

@@ -16,23 +16,19 @@ package debug
 
 import (
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestDebug(t *testing.T) {
-	tmpFile, err := os.CreateTemp("", "debug-test")
+	tmpFile, err := os.CreateTemp(workaroundTempDir(t)(), "debug-test")
 	require.NoError(t, err)
 
 	output = tmpFile
-
 	tmpName := tmpFile.Name()
-	defer func() {
-		_ = os.Remove(tmpName)
-	}()
-
 	testLogger := GetLogger("test", true)
 
 	testLogger("A debug: %s", "a string")
@@ -43,14 +39,11 @@ func TestDebug(t *testing.T) {
 
 	assert.Contains(t, string(flushed), "A debug: a string")
 
-	tmpEmptyFile, err := os.CreateTemp("", "debug-test")
+	tmpEmptyFile, err := os.CreateTemp(workaroundTempDir(t)(), "debug-test")
 	require.NoError(t, err)
 	tmpEmpty := tmpEmptyFile.Name()
-	defer func() {
-		_ = os.Remove(tmpEmpty)
-	}()
-
 	testLogger = GetLogger("test", false)
+
 	testLogger("A debug: %s", "a string")
 	tmpFile.Close()
 
@@ -59,3 +52,16 @@ func TestDebug(t *testing.T) {
 
 	assert.Empty(t, flushed)
 }
+
+func workaroundTempDir(t testing.TB) func() string {
+	// Workaround for go testing bug on Windows: https://github.com/golang/go/issues/71544
+	// On windows, t.TempDir() doesn't properly release file handles yet,
+	// se we just leave it unchecked (no cleanup would take place).
+	if runtime.GOOS == "windows" {
+		return func() string {
+			return ""
+		}
+	}
+
+	return t.TempDir
+}

internal/flatten/replace/errors.go

@@ -0,0 +1,61 @@
+package replace
+
+import (
+	"errors"
+	"fmt"
+)
+
+type replaceError string
+
+const (
+	ErrReplace        replaceError = "flatten replace error"
+	ErrUnexpectedType replaceError = "unexpected type used in getPointerFromKey"
+)
+
+func (e replaceError) Error() string {
+	return string(e)
+}
+
+func ErrNoSchemaWithRef(key string, value any) error {
+	return fmt.Errorf("no schema with ref found at %s for %T: %w", key, value, ErrReplace)
+}
+
+func ErrNoSchema(key string) error {
+	return fmt.Errorf("no schema found at %s: %w", key, ErrReplace)
+}
+
+func ErrNotANumber(key string, err error) error {
+	return errors.Join(
+		ErrReplace,
+		fmt.Errorf("%s not a number: %w", key, err),
+	)
+}
+
+func ErrUnhandledParentRewrite(key string, value any) error {
+	return fmt.Errorf("unhandled parent schema rewrite %s: %T: %w", key, value, ErrReplace)
+}
+
+func ErrUnhandledParentType(key string, value any) error {
+	return fmt.Errorf("unhandled type for parent of %s: %T: %w", key, value, ErrReplace)
+}
+
+func ErrNoParent(key string, err error) error {
+	return errors.Join(
+		fmt.Errorf("can't get parent for %s: %w", key, err),
+		ErrReplace,
+	)
+}
+
+func ErrUnhandledContainerType(key string, value any) error {
+	return fmt.Errorf("unhandled container type at %s: %T: %w", key, value, ErrReplace)
+}
+
+func ErrCyclicChain(key string) error {
+	return fmt.Errorf("cannot resolve cyclic chain of pointers under %s: %w", key, ErrReplace)
+}
+
+func ErrInvalidPointerType(key string, value any, err error) error {
+	return fmt.Errorf("invalid type for resolved JSON pointer %s. Expected a schema a, got: %T (%v): %w",
+		key, value, err, ErrReplace,
+	)
+}