chore(relint): relint code base (#461)

* strengthen golangci rules: closer to full alignment with other
  go-openapi repos
* relint all
* readability: refactor complex methods & functions

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

Author: fredbi

.golangci.yml

@@ -2,13 +2,13 @@ version: "2"
 linters:
   default: all
   disable:
-    - cyclop
+    #- cyclop
     - depguard
     - err113 # disabled temporarily: there are just too many issues to address
-    - errchkjson
-    - errorlint
+    #- errchkjson
+    #- errorlint
     - exhaustruct
-    - forcetypeassert
+    #- forcetypeassert
     - funlen
     - gochecknoglobals
     - gochecknoinits
@@ -18,10 +18,10 @@ linters:
     - gomoddirectives # moved to mono-repo, multi-modules, so replace directives are needed
     - gosmopolitan
     - inamedparam
-    - ireturn
-    - lll
+    - ireturn # this repo adopted a pattern where there are quite many returned interfaces. To be challenged with v2
+    #- lll
     - musttag
-    - nestif
+    #- nestif
     - nilerr # nilerr crashes on this repo
     - nlreturn
     - noinlineerr
@@ -31,7 +31,7 @@ linters:
     - testpackage
     - thelper
     - tparallel
-    - unparam
+    #- unparam
     - varnamelen
     - whitespace
     - wrapcheck
@@ -43,8 +43,17 @@ linters:
     goconst:
       min-len: 2
       min-occurrences: 3
+    cyclop:
+      max-complexity: 25
     gocyclo:
-      min-complexity: 45
+      min-complexity: 25
+    gocognit:
+      min-complexity: 35
+    exhaustive:
+      default-signifies-exhaustive: true
+      default-case-required: true
+    lll:
+      line-length: 180
   exclusions:
     generated: lax
     presets:
@@ -61,13 +70,17 @@ formatters:
   enable:
     - gofmt
     - goimports
+  settings:
+    # local prefixes regroup imports from these packages
+    goimports:
+      local-prefixes:
+        - github.com/go-openapi
   exclusions:
     generated: lax
     paths:
       - .worktrees
       - third_party$
       - builtin$
-      - examples$
 issues:
   # Maximum issues count per one linter.
   # Set to 0 to disable.

client-middleware/opentracing/opentracing.go

@@ -7,11 +7,12 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/go-openapi/strfmt"
 	"github.com/opentracing/opentracing-go"
 	"github.com/opentracing/opentracing-go/ext"
 	"github.com/opentracing/opentracing-go/log"
 
+	"github.com/go-openapi/strfmt"
+
 	"github.com/go-openapi/runtime"
 )
 

client/content_negotiation_test.go

@@ -238,9 +238,9 @@ func writerSetBody(payload any) runtime.ClientRequestWriter {
 	})
 }
 
-func writerSetHeader(name, value string) runtime.ClientRequestWriter {
+func writerSetContentType(value string) runtime.ClientRequestWriter {
 	return runtime.ClientRequestWriterFunc(func(r runtime.ClientRequest, _ strfmt.Registry) error {
-		return r.SetHeaderParam(name, value)
+		return r.SetHeaderParam("Content-Type", value)
 	})
 }
 
@@ -250,9 +250,9 @@ func writerSetForm(name string, values ...string) runtime.ClientRequestWriter {
 	})
 }
 
-func writerSetFile(name string, files ...runtime.NamedReadCloser) runtime.ClientRequestWriter {
+func writerSetFileToUpload(files ...runtime.NamedReadCloser) runtime.ClientRequestWriter {
 	return runtime.ClientRequestWriterFunc(func(r runtime.ClientRequest, _ strfmt.Registry) error {
-		return r.SetFileParam(name, files...)
+		return r.SetFileParam("upload", files...)
 	})
 }
 
@@ -303,8 +303,8 @@ type staticFileWithCT struct{ *staticFile }
 
 func (f *staticFileWithCT) ContentType() string { return f.ct }
 
-func newFile(name, data string) runtime.NamedReadCloser {
-	return &staticFile{name: name, r: strings.NewReader(data)}
+func newFile(data string) runtime.NamedReadCloser {
+	return &staticFile{name: "doc.txt", r: strings.NewReader(data)}
 }
 
 func newFileWithCT(name, data, ct string) runtime.NamedReadCloser {
@@ -374,7 +374,7 @@ func payloadStructCases() iter.Seq[buildHTTPCase] {
 			name:      "struct + SetHeader Content-Type is ignored — picker wins",
 			mediaType: runtime.JSONMime,
 			writer: writerCompose(
-				writerSetHeader("Content-Type", "application/x-ignored"),
+				writerSetContentType("application/x-ignored"),
 				writerSetBody(task{Content: "y"}),
 			),
 			wantContentType: runtime.JSONMime,
@@ -509,7 +509,7 @@ func payloadReaderCases() iter.Seq[buildHTTPCase] {
 			name:      "io.Reader + SetHeader Content-Type wins over picker",
 			mediaType: runtime.JSONMime,
 			writer: writerCompose(
-				writerSetHeader("Content-Type", vendorMime),
+				writerSetContentType(vendorMime),
 				writerSetBody(bytes.NewReader([]byte("v"))),
 			),
 			wantContentType: vendorMime,
@@ -520,7 +520,7 @@ func payloadReaderCases() iter.Seq[buildHTTPCase] {
 			name:      "io.Reader + SetHeader wins over payload ContentType()",
 			mediaType: runtime.JSONMime,
 			writer: writerCompose(
-				writerSetHeader("Content-Type", "application/x-explicit"),
+				writerSetContentType("application/x-explicit"),
 				writerSetBody(&readerWithCT{
 					Reader: strings.NewReader("body"),
 					ct:     ndjsonMime,
@@ -535,7 +535,7 @@ func payloadReaderCases() iter.Seq[buildHTTPCase] {
 			mediaType: runtime.JSONMime,
 			consumes:  []string{runtime.JSONMime, runtime.DefaultMime},
 			writer: writerCompose(
-				writerSetHeader("Content-Type", "application/x-explicit"),
+				writerSetContentType("application/x-explicit"),
 				writerSetBody(bytes.NewReader([]byte("v"))),
 			),
 			wantContentType: "application/x-explicit",
@@ -546,7 +546,7 @@ func payloadReaderCases() iter.Seq[buildHTTPCase] {
 			name:      "io.ReadCloser + SetHeader Content-Type wins",
 			mediaType: runtime.TextMime,
 			writer: writerCompose(
-				writerSetHeader("Content-Type", vendorMime),
+				writerSetContentType(vendorMime),
 				writerSetBody(io.NopCloser(strings.NewReader("data"))),
 			),
 			wantContentType: vendorMime,
@@ -615,7 +615,7 @@ func fileFieldCases() iter.Seq[buildHTTPCase] {
 		{
 			name:                  "file field + multipart mime",
 			mediaType:             runtime.MultipartFormMime,
-			writer:                writerSetFile("upload", newFile("doc.txt", "filebody")),
+			writer:                writerSetFileToUpload(newFile("filebody")),
 			wantContentTypePrefix: runtime.MultipartFormMime + "; boundary=",
 			wantBody:              bodyContainsAll(`name="upload"`, `filename="doc.txt"`, "filebody"),
 		},
@@ -624,14 +624,14 @@ func fileFieldCases() iter.Seq[buildHTTPCase] {
 			// the file content travels as a regular form value.
 			name:            "file field + urlencoded mime — file inlined as form value",
 			mediaType:       runtime.URLencodedFormMime,
-			writer:          writerSetFile("upload", newFile("doc.txt", "abc")),
+			writer:          writerSetFileToUpload(newFile("abc")),
 			wantContentType: runtime.URLencodedFormMime,
 			wantBody:        bodyContainsAll("upload=abc"),
 		},
 		{
 			name:                  "file with declared ContentType()",
 			mediaType:             runtime.MultipartFormMime,
-			writer:                writerSetFile("upload", newFileWithCT("doc.txt", "x", "application/json")),
+			writer:                writerSetFileToUpload(newFileWithCT("doc.txt", "x", "application/json")),
 			wantContentTypePrefix: runtime.MultipartFormMime + "; boundary=",
 			wantBody:              bodyContainsAll("application/json"),
 		},
@@ -646,7 +646,7 @@ func formAndFileFieldCases() iter.Seq[buildHTTPCase] {
 			mediaType: runtime.MultipartFormMime,
 			writer: writerCompose(
 				writerSetForm("name", "fido"),
-				writerSetFile("upload", newFile("doc.txt", "filebody")),
+				writerSetFileToUpload(newFile("filebody")),
 			),
 			wantContentTypePrefix: runtime.MultipartFormMime + "; boundary=",
 			wantBody:              bodyContainsAll(`name="name"`, "fido", `filename="doc.txt"`, "filebody"),
@@ -656,7 +656,7 @@ func formAndFileFieldCases() iter.Seq[buildHTTPCase] {
 			mediaType: runtime.URLencodedFormMime,
 			writer: writerCompose(
 				writerSetForm("name", "fido"),
-				writerSetFile("upload", newFile("doc.txt", "abc")),
+				writerSetFileToUpload(newFile("abc")),
 			),
 			wantContentType: runtime.URLencodedFormMime,
 			wantBody:        bodyContainsAll("name=fido", "upload=abc"),
@@ -748,7 +748,7 @@ func submitWiringCases() iter.Seq[submitCase] {
 			name:     "consumes [json] + SetHeader Content-Type — escape hatch wins",
 			consumes: []string{runtime.JSONMime},
 			writer: writerCompose(
-				writerSetHeader("Content-Type", vendorMime),
+				writerSetContentType(vendorMime),
 				writerSetBody(bytes.NewReader([]byte("data"))),
 			),
 			wantContentType: vendorMime,

client/httptrace_tls.go

@@ -71,7 +71,7 @@ func introspectTLSConfig(client *http.Client) *tls.Config {
 func (s *traceSession) emitTLSDiagnostic(state tls.ConnectionState, err error) {
 	s.emitf("# TLS DIAGNOSTIC")
 
-	//nolint:exhaustive // tlsAxisGeneric is handled by the default branch.
+	// tlsAxisGeneric is handled by the default branch.
 	switch axis := classifyTLSError(err); axis {
 	case tlsAxisProtocolVersion:
 		s.diagnoseProtocolVersion(state, err)
@@ -215,7 +215,6 @@ func (s *traceSession) diagnoseCertInvalid(certInvalid x509.CertificateInvalidEr
 	cert := certInvalid.Cert
 	s.emitf("#   reason: %s", certInvalidReasonName(certInvalid.Reason))
 
-	//nolint:exhaustive // Less-common reasons render via the default branch (issuer + NotAfter dump).
 	switch certInvalid.Reason {
 	case x509.Expired:
 		s.emitf("#   leaf:    subject=%s", cert.Subject)
@@ -231,6 +230,7 @@ func (s *traceSession) diagnoseCertInvalid(certInvalid x509.CertificateInvalidEr
 		s.emitf("#   suggested: set TLSClientOptions.ServerName to match")
 		s.emitf("#              one of the cert SANs, or fix the cert.")
 	default:
+		// Less-common reasons render via the default branch (issuer + NotAfter dump).
 		s.emitf("#   leaf:    subject=%s, issuer=%s", cert.Subject, cert.Issuer)
 		s.emitf("#            NotBefore=%s", cert.NotBefore.UTC().Format(time.RFC3339))
 		s.emitf("#            NotAfter=%s", cert.NotAfter.UTC().Format(time.RFC3339))
@@ -326,7 +326,7 @@ func cipherSuiteNames(ids []uint16) []string {
 // human-readable label. The stdlib does not expose a String()
 // method for these, so we keep a small table.
 //
-//nolint:exhaustive // Anything outside the listed cases falls through to the numeric default.
+// Anything outside the listed cases falls through to the numeric default.
 func certInvalidReasonName(r x509.InvalidReason) string {
 	switch r {
 	case x509.NotAuthorizedToSign:

client/internal/request/request.go

@@ -6,6 +6,7 @@ package request
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -82,7 +83,7 @@ var _ runtime.ClientRequest = new(Request) // ensure compliance to the interface
 //     [Request.SetHeaderParam] during WriteToRequest, and we treat that as an intentional escape hatch
 //  2. use payload's [runtime.ContentTyper] declaration (in this case, the produced payload knows its content type)
 //  3. use `application/octet-stream` if it is available in the registered producers
-//  4. otherwise ser the picker's mediaType
+//  4. otherwise set the picker's mediaType
 //
 // For multi-part requests, the content type of each part is auto-detected using the following sequence:
 //
@@ -316,7 +317,9 @@ func (r *Request) SetConsumes(consumers []string) {
 //
 // On error the cancel is invoked internally and a no-op cancel is returned,
 // so callers can defer cancel unconditionally.
-func (r *Request) BuildHTTPContext(parentCtx context.Context, mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (*http.Request, context.CancelFunc, error) {
+func (r *Request) BuildHTTPContext(parentCtx context.Context, mediaType, basePath string,
+	producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter,
+) (*http.Request, context.CancelFunc, error) {
 	if err := r.writer.WriteToRequest(r, registry); err != nil {
 		return nil, noop, err
 	}
@@ -362,11 +365,13 @@ func (r *Request) usesStreamingBody(mediaType string) bool {
 	if (len(r.formFields) > 0 || len(r.fileFields) > 0) && r.isMultipart(mediaType) {
 		return true
 	}
+
 	if r.payload != nil {
 		if _, ok := r.payload.(io.Reader); ok {
 			return true
 		}
 	}
+
 	return false
 }
 
@@ -409,7 +414,9 @@ func (r *Request) isMultipart(mediaType string) bool {
 //
 // Auth is trivial in this flow because the buffer is already populated when the auth helper
 // asks for the body via r.GetBody().
-func (r *Request) buildBufferedRequest(ctx context.Context, mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (*http.Request, error) {
+func (r *Request) buildBufferedRequest(ctx context.Context, mediaType, basePath string,
+	producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter,
+) (*http.Request, error) {
 	var body io.Reader
 	var err error
 
@@ -450,7 +457,9 @@ func (r *Request) buildBufferedRequest(ctx context.Context, mediaType, basePath
 // (it would otherwise park forever on pw.Write with no reader).
 //
 // For stream payloads it closes the user-provided io.ReadCloser.
-func (r *Request) buildStreamingRequest(ctx context.Context, mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (req *http.Request, retErr error) {
+func (r *Request) buildStreamingRequest(ctx context.Context, mediaType, basePath string,
+	producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter,
+) (req *http.Request, retErr error) {
 	var body io.Reader
 	if len(r.formFields) > 0 || len(r.fileFields) > 0 {
 		body = r.writeMultipartBody(ctx, mediaType)
@@ -603,7 +612,7 @@ func (r *Request) applyAuthWithBodyCopy(auth runtime.ClientAuthInfoWriter, body
 	// underlying pipe/stream. Caller treats body as ignorable when
 	// err != nil per Go convention; the defer reads it via closure.
 	if copyErr != nil {
-		return body, fmt.Errorf("error retrieving the response body: %v", copyErr)
+		return body, fmt.Errorf("error copying the request body: %w", copyErr)
 	}
 
 	if authErr != nil {
@@ -731,7 +740,7 @@ func (r *Request) streamMultipartParts(ctx context.Context, mp *multipart.Writer
 				const contentTypeBufferSize = 512
 				buf := make([]byte, contentTypeBufferSize)
 				size, err := fi.Read(buf)
-				if err != nil && err != io.EOF {
+				if err != nil && !errors.Is(err, io.EOF) {
 					logClose(err, pw)
 					return
 				}
@@ -789,7 +798,13 @@ func (r *Request) writeStreamPayload(mediaType string, producers map[string]runt
 	if rdr, ok := r.payload.(io.ReadCloser); ok {
 		return rdr
 	}
-	return r.payload.(io.Reader)
+
+	rdr, ok := r.payload.(io.Reader)
+	if !ok {
+		panic("internal error: payload expected to be an io.Reader") // guaranteed by earlier checks
+	}
+
+	return rdr
 }
 
 // writeNonStreamPayload runs the producer registered for mediaType
@@ -903,7 +918,8 @@ func logClose(err error, pw *io.PipeWriter) {
 	}
 }
 
-func mangleContentType(_, boundary string) string {
+func mangleContentType(mediaType, boundary string) string {
+	_ = mediaType // reserved for future enhancement: honor caller-provided media type
 	// Proposal for enhancement: honor caller's boundary if specified
 	return "multipart/form-data; boundary=" + boundary
 }

client/internal/request/request_test.go

@@ -304,7 +304,7 @@ func TestBuildRequest_BuildHTTP_XMLPayload(t *testing.T) {
 }
 
 func TestBuildRequest_BuildHTTP_TextPayload(t *testing.T) {
-	const bd = "Tom: Organ trail; John: Bird watching"
+	const bd = "Tom: Oregon trail; John: Bird watching"
 
 	reqWrtr := runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error {
 		_ = req.SetBodyParam(bd)

client/opentelemetry.go

@@ -8,14 +8,15 @@ import (
 	"net/http"
 	"strings"
 
-	"github.com/go-openapi/runtime"
-	"github.com/go-openapi/strfmt"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/propagation"
 	semconv "go.opentelemetry.io/otel/semconv/v1.37.0"
 	"go.opentelemetry.io/otel/trace"
+
+	"github.com/go-openapi/runtime"
+	"github.com/go-openapi/strfmt"
 )
 
 const (