Repository: go-openapi/runtime
Pull request #312 (2024-03-07 15:08:23)

Refactor newRequest function to remove error return value
## Summary
This PR refactors the newRequest function in the Swagger HTTP client to remove the error return value, as it is unnecessary. The function does not perform any operations that can result in an error, so the error return value has been removed for simplicity and clarity.

## Changes
- Removed the error return value from the newRequest function signature.
- Modified associated tests to reflect changes in the newRequest function.

Before:

```
func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) (*request, error) {
	return &request{
		pathPattern: pathPattern,
		method:      method,
		writer:      writer,
		header:      make(http.Header),
		query:       make(url.Values),
		timeout:     DefaultTimeout,
		getBody:     getRequestBuffer,
	}, nil
}
```

After:

```
func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) *request {
	return &request{
		pathPattern: pathPattern,
		method:      method,
		writer:      writer,
		header:      make(http.Header),
		query:       make(url.Values),
		timeout:     DefaultTimeout,
		getBody:     getRequestBuffer,
	}
}
```

## Rationale
Removing the error return value simplifies the function signature and makes it clear that the function does not generate any errors that need to be handled. This change enhances the readability and maintainability of the code.


Patch:
diff --git a/client/request.go b/client/request.go
index c238953..c4a891d 100644
--- a/client/request.go
+++ b/client/request.go
@@ -36,7 +36,7 @@ import (
 )
 
 // NewRequest creates a new swagger http client request
-func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) (*request, error) {
+func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) *request {
 	return &request{
 		pathPattern: pathPattern,
 		method:      method,
@@ -45,7 +45,7 @@ func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter)
 		query:       make(url.Values),
 		timeout:     DefaultTimeout,
 		getBody:     getRequestBuffer,
-	}, nil
+	}
 }
 
 // Request represents a swagger client request.
diff --git a/client/runtime.go b/client/runtime.go
index fdf9717..5bd4d75 100644
--- a/client/runtime.go
+++ b/client/runtime.go
@@ -32,12 +32,13 @@ import (
 	"sync"
 	"time"
 
+	"github.com/go-openapi/strfmt"
+	"github.com/opentracing/opentracing-go"
+
 	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/runtime/logger"
 	"github.com/go-openapi/runtime/middleware"
 	"github.com/go-openapi/runtime/yamlpc"
-	"github.com/go-openapi/strfmt"
-	"github.com/opentracing/opentracing-go"
 )
 
 const (
@@ -379,14 +380,11 @@ func (r *Runtime) EnableConnectionReuse() {
 func (r *Runtime) createHttpRequest(operation *runtime.ClientOperation) (*request, *http.Request, error) { //nolint:revive,stylecheck
 	params, _, auth := operation.Params, operation.Reader, operation.AuthInfo
 
-	request, err := newRequest(operation.Method, operation.PathPattern, params)
-	if err != nil {
-		return nil, nil, err
-	}
+	request := newRequest(operation.Method, operation.PathPattern, params)
 
 	var accept []string
 	accept = append(accept, operation.ProducesMediaTypes...)
-	if err = request.SetHeaderParam(runtime.HeaderAccept, accept...); err != nil {
+	if err := request.SetHeaderParam(runtime.HeaderAccept, accept...); err != nil {
 		return nil, nil, err
 	}
 


Test patch:
diff --git a/client/auth_info_test.go b/client/auth_info_test.go
index 1e3cc40..8299a56 100644
--- a/client/auth_info_test.go
+++ b/client/auth_info_test.go
@@ -18,17 +18,17 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/go-openapi/runtime"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/go-openapi/runtime"
 )
 
 func TestBasicAuth(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/", nil)
 
 	writer := BasicAuth("someone", "with a password")
-	err = writer.AuthenticateRequest(r, nil)
+	err := writer.AuthenticateRequest(r, nil)
 	require.NoError(t, err)
 
 	req := new(http.Request)
@@ -41,44 +41,40 @@ func TestBasicAuth(t *testing.T) {
 }
 
 func TestAPIKeyAuth_Query(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/", nil)
 
 	writer := APIKeyAuth("api_key", "query", "the-shared-key")
-	err = writer.AuthenticateRequest(r, nil)
+	err := writer.AuthenticateRequest(r, nil)
 	require.NoError(t, err)
 
 	assert.Equal(t, "the-shared-key", r.query.Get("api_key"))
 }
 
 func TestAPIKeyAuth_Header(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/", nil)
 
 	writer := APIKeyAuth("x-api-token", "header", "the-shared-key")
-	err = writer.AuthenticateRequest(r, nil)
+	err := writer.AuthenticateRequest(r, nil)
 	require.NoError(t, err)
 
 	assert.Equal(t, "the-shared-key", r.header.Get("x-api-token"))
 }
 
 func TestBearerTokenAuth(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/", nil)
 
 	writer := BearerToken("the-shared-token")
-	err = writer.AuthenticateRequest(r, nil)
+	err := writer.AuthenticateRequest(r, nil)
 	require.NoError(t, err)
 
 	assert.Equal(t, "Bearer the-shared-token", r.header.Get(runtime.HeaderAuthorization))
 }
 
 func TestCompose(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/", nil)
 
 	writer := Compose(APIKeyAuth("x-api-key", "header", "the-api-key"), APIKeyAuth("x-secret-key", "header", "the-secret-key"))
-	err = writer.AuthenticateRequest(r, nil)
+	err := writer.AuthenticateRequest(r, nil)
 	require.NoError(t, err)
 
 	assert.Equal(t, "the-api-key", r.header.Get("x-api-key"))
diff --git a/client/request_test.go b/client/request_test.go
index e9234b3..2a90aa7 100644
--- a/client/request_test.go
+++ b/client/request_test.go
@@ -30,10 +30,11 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/strfmt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/go-openapi/runtime"
 )
 
 var testProducers = map[string]runtime.Producer{
@@ -43,8 +44,7 @@ var testProducers = map[string]runtime.Producer{
 }
 
 func TestBuildRequest_SetHeaders(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", nil)
 
 	// single value
 	_ = r.SetHeaderParam("X-Rate-Limit", "500")
@@ -58,16 +58,14 @@ func TestBuildRequest_SetHeaders(t *testing.T) {
 }
 
 func TestBuildRequest_SetPath(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
 
 	_ = r.SetPathParam("id", "1345")
 	assert.Equal(t, "1345", r.pathParams["id"])
 }
 
 func TestBuildRequest_SetQuery(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", nil)
 
 	// single value
 	_ = r.SetQueryParam("hello", "there")
@@ -80,8 +78,7 @@ func TestBuildRequest_SetQuery(t *testing.T) {
 
 func TestBuildRequest_SetForm(t *testing.T) {
 	// non-multipart
-	r, err := newRequest(http.MethodPost, "/flats", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats", nil)
 	_ = r.SetFormParam("hello", "world")
 	assert.Equal(t, "world", r.formFields.Get("hello"))
 	_ = r.SetFormParam("goodbye", "cruel", "world")
@@ -90,11 +87,10 @@ func TestBuildRequest_SetForm(t *testing.T) {
 
 func TestBuildRequest_SetFile(t *testing.T) {
 	// needs to convert form to multipart
-	r, err := newRequest(http.MethodPost, "/flats/{id}/image", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/image", nil)
 
 	// error if it isn't there
-	err = r.SetFileParam("not there", os.NewFile(0, "./i-dont-exist"))
+	err := r.SetFileParam("not there", os.NewFile(0, "./i-dont-exist"))
 	require.Error(t, err)
 
 	// error if it isn't a file
@@ -125,8 +121,7 @@ func mustGetFile(path string) *os.File {
 }
 
 func TestBuildRequest_SetBody(t *testing.T) {
-	r, err := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
 
 	bd := []struct{ Name, Hobby string }{{"Tom", "Organ trail"}, {"John", "Bird watching"}}
 
@@ -142,8 +137,7 @@ func TestBuildRequest_BuildHTTP_NoPayload(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
 	require.NoError(t, err)
@@ -162,8 +156,7 @@ func TestBuildRequest_BuildHTTP_Payload(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
@@ -197,8 +190,7 @@ func TestBuildRequest_BuildHTTP_SetsInAuth(t *testing.T) {
 		return nil
 	})
 
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
 
 	req, err := r.buildHTTP(runtime.JSONMime, "", testProducers, nil, auth)
@@ -228,8 +220,7 @@ func TestBuildRequest_BuildHTTP_XMLPayload(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.XMLMime)
 
 	req, err := r.BuildHTTP(runtime.XMLMime, "", testProducers, nil)
@@ -256,8 +247,7 @@ func TestBuildRequest_BuildHTTP_TextPayload(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.TextMime)
 
 	req, err := r.BuildHTTP(runtime.TextMime, "", testProducers, nil)
@@ -281,8 +271,7 @@ func TestBuildRequest_BuildHTTP_Form(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
@@ -305,8 +294,7 @@ func TestBuildRequest_BuildHTTP_Form_URLEncoded(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.URLencodedFormMime)
 
 	req, err := r.BuildHTTP(runtime.URLencodedFormMime, "", testProducers, nil)
@@ -330,8 +318,7 @@ func TestBuildRequest_BuildHTTP_Form_Content_Length(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
@@ -356,8 +343,7 @@ func TestBuildRequest_BuildHTTP_FormMultipart(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)
 
 	req, err := r.BuildHTTP(runtime.MultipartFormMime, "", testProducers, nil)
@@ -390,8 +376,7 @@ func TestBuildRequest_BuildHTTP_FormMultiples(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)
 
 	req, err := r.BuildHTTP(runtime.MultipartFormMime, "", testProducers, nil)
@@ -438,8 +423,7 @@ func TestBuildRequest_BuildHTTP_Files(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
 	require.NoError(t, err)
@@ -492,8 +476,7 @@ func TestBuildRequest_BuildHTTP_Files_URLEncoded(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.URLencodedFormMime)
 	req, err := r.BuildHTTP(runtime.URLencodedFormMime, "", testProducers, nil)
 	require.NoError(t, err)
@@ -554,8 +537,7 @@ func TestBuildRequest_BuildHTTP_File_ContentType(t *testing.T) {
 
 		return nil
 	})
-	r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
 	_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
 	req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
 	require.NoError(t, err)
@@ -594,8 +576,7 @@ func TestBuildRequest_BuildHTTP_BasePath(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
 	require.NoError(t, err)
@@ -613,8 +594,7 @@ func TestBuildRequest_BuildHTTP_EscapedPath(t *testing.T) {
 		_ = req.SetHeaderParam("X-Rate-Limit", "200")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
 	require.NoError(t, err)
@@ -634,8 +614,7 @@ func TestBuildRequest_BuildHTTP_BasePathWithQueryParameters(t *testing.T) {
 		_ = req.SetPathParam("id", "1234")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?foo=bar", testProducers, nil)
 	require.NoError(t, err)
@@ -653,8 +632,7 @@ func TestBuildRequest_BuildHTTP_PathPatternWithQueryParameters(t *testing.T) {
 		_ = req.SetPathParam("id", "1234")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/?foo=bar", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/?foo=bar", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
 	require.NoError(t, err)
@@ -671,8 +649,7 @@ func TestBuildRequest_BuildHTTP_StaticParametersPathPatternPrevails(t *testing.T
 		_ = req.SetPathParam("id", "1234")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?hello=kitty", testProducers, nil)
 	require.NoError(t, err)
@@ -689,8 +666,7 @@ func TestBuildRequest_BuildHTTP_StaticParametersConflictClientPrevails(t *testin
 		_ = req.SetPathParam("id", "1234")
 		return nil
 	})
-	r, err := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
-	require.NoError(t, err)
+	r := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
 
 	req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?hello=kitty", testProducers, nil)
 	require.NoError(t, err)