Skip to content

Commit

Permalink
ci: address multiple lint rules (#2869)
Browse files Browse the repository at this point in the history
* ci: explicitly disable tagalign

Tagalign requires awkward manual formatting and doesn't provide much
value for readability.

* ci: enable mirror linter

mirror warns against certain cases of useless conversion between string
and []byte.

* ci: enable perfsprint linter

This linter encourages replacing several functions from the fmt package
with faster alternatives. While fixing issues, I also added a few
exported error types rather than returning a naked errors.New().
  • Loading branch information
nickajacks1 committed Feb 19, 2024
1 parent 529086a commit 4c68e02
Show file tree
Hide file tree
Showing 19 changed files with 79 additions and 73 deletions.
8 changes: 4 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ issues:

linters:
disable:
- spancheck
- spancheck # opentelemetry, irrelevant
- tagalign # requires awkward manual formatting of struct tags
enable:
- asasalint
- asciicheck
Expand Down Expand Up @@ -199,7 +200,7 @@ linters:
- grouper
- inamedparam
- loggercheck
# - mirror # TODO https://github.com/gofiber/fiber/issues/2816
- mirror
- misspell
- nakedret
- nilerr
Expand All @@ -208,7 +209,7 @@ linters:
- nolintlint
- nonamedreturns
- nosprintfhostport
# - perfsprint # TODO https://github.com/gofiber/fiber/issues/2816
- perfsprint
- predeclared
- promlinter
- reassign
Expand All @@ -217,7 +218,6 @@ linters:
- sqlclosecheck
- staticcheck
- stylecheck
# - tagalign # TODO https://github.com/gofiber/fiber/issues/2816
- tagliatelle
- testifylint
# - testpackage # TODO: Enable once https://github.com/gofiber/fiber/issues/2252 is implemented
Expand Down
6 changes: 3 additions & 3 deletions addon/retry/exponential_backoff_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package retry

import (
"fmt"
"errors"
"testing"
"time"

Expand Down Expand Up @@ -44,9 +44,9 @@ func TestExponentialBackoff_Retry(t *testing.T) {
MaxRetryCount: 5,
},
f: func() error {
return fmt.Errorf("failed function")
return errors.New("failed function")
},
expErr: fmt.Errorf("failed function"),
expErr: errors.New("failed function"),
},
}

Expand Down
9 changes: 3 additions & 6 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ func (app *App) ShutdownWithContext(ctx context.Context) error {
app.mutex.Lock()
defer app.mutex.Unlock()
if app.server == nil {
return fmt.Errorf("shutdown: server is not running")
return ErrNotRunning
}
return app.server.ShutdownWithContext(ctx)
}
Expand Down Expand Up @@ -948,7 +948,7 @@ func (app *App) Test(req *http.Request, msTimeout ...int) (*http.Response, error
var returned bool
defer func() {
if !returned {
channel <- fmt.Errorf("runtime.Goexit() called in handler or server panic")
channel <- ErrHandlerExited
}
}()

Expand Down Expand Up @@ -1071,10 +1071,7 @@ func (app *App) ErrorHandler(ctx Ctx, err error) error {
// errors before calling the application's error handler method.
func (app *App) serverErrorHandler(fctx *fasthttp.RequestCtx, err error) {
// Acquire Ctx with fasthttp request from pool
c, ok := app.AcquireCtx().(*DefaultCtx)
if !ok {
panic(fmt.Errorf("failed to type-assert to *DefaultCtx"))
}
c := app.AcquireCtx()
c.Reset(fctx)

defer app.ReleaseCtx(c)
Expand Down
6 changes: 3 additions & 3 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1672,7 +1672,7 @@ func Test_App_ReadBodyStream(t *testing.T) {
require.NoError(t, err, "app.Test(req)")
body, err := io.ReadAll(resp.Body)
require.NoError(t, err, "io.ReadAll(resp.Body)")
require.Equal(t, fmt.Sprintf("true %s", testString), string(body))
require.Equal(t, "true "+testString, string(body))
}

func Test_App_DisablePreParseMultipartForm(t *testing.T) {
Expand All @@ -1688,7 +1688,7 @@ func Test_App_DisablePreParseMultipartForm(t *testing.T) {
return err
}
if !req.IsBodyStream() {
return fmt.Errorf("not a body stream")
return errors.New("not a body stream")
}
file, err := mpf.File["test"][0].Open()
if err != nil {
Expand All @@ -1700,7 +1700,7 @@ func Test_App_DisablePreParseMultipartForm(t *testing.T) {
return fmt.Errorf("failed to read: %w", err)
}
if n != len(testString) {
return fmt.Errorf("bad read length")
return errors.New("bad read length")
}
return c.Send(buffer)
})
Expand Down
2 changes: 1 addition & 1 deletion bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1556,7 +1556,7 @@ func (*structValidator) ValidateStruct(out any) error {
out = reflect.ValueOf(out).Elem().Interface()
sq, ok := out.(simpleQuery)
if !ok {
return fmt.Errorf("failed to type-assert to simpleQuery")
return errors.New("failed to type-assert to simpleQuery")
}

if sq.Name != "john" {
Expand Down
11 changes: 6 additions & 5 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"encoding/json"
"encoding/xml"
"errors"
"fmt"
"io"
"mime/multipart"
Expand Down Expand Up @@ -885,7 +886,7 @@ func AcquireClient() *Client {
}
c, ok := v.(*Client)
if !ok {
panic(fmt.Errorf("failed to type-assert to *Client"))
panic(errors.New("failed to type-assert to *Client"))
}
return c
}
Expand All @@ -911,7 +912,7 @@ func ReleaseClient(c *Client) {
func AcquireAgent() *Agent {
a, ok := agentPool.Get().(*Agent)
if !ok {
panic(fmt.Errorf("failed to type-assert to *Agent"))
panic(errors.New("failed to type-assert to *Agent"))
}
return a
}
Expand All @@ -938,7 +939,7 @@ func AcquireResponse() *Response {
}
r, ok := v.(*Response)
if !ok {
panic(fmt.Errorf("failed to type-assert to *Response"))
panic(errors.New("failed to type-assert to *Response"))
}
return r
}
Expand All @@ -965,7 +966,7 @@ func AcquireArgs() *Args {
}
a, ok := v.(*Args)
if !ok {
panic(fmt.Errorf("failed to type-assert to *Args"))
panic(errors.New("failed to type-assert to *Args"))
}
return a
}
Expand All @@ -990,7 +991,7 @@ func AcquireFormFile() *FormFile {
}
ff, ok := v.(*FormFile)
if !ok {
panic(fmt.Errorf("failed to type-assert to *FormFile"))
panic(errors.New("failed to type-assert to *FormFile"))
}
return ff
}
Expand Down
3 changes: 1 addition & 2 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"encoding/json"
"encoding/xml"
"errors"
"fmt"
"io"
"mime/multipart"
"net"
Expand Down Expand Up @@ -630,7 +629,7 @@ type readErrorConn struct {
}

func (*readErrorConn) Read(_ []byte) (int, error) {
return 0, fmt.Errorf("error")
return 0, errors.New("error")
}

func (*readErrorConn) Write(p []byte) (int, error) {
Expand Down
11 changes: 4 additions & 7 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"context"
"crypto/tls"
"errors"
"fmt"
"io"
"mime/multipart"
Expand Down Expand Up @@ -615,7 +616,7 @@ func (c *DefaultCtx) Hostname() string {
func (c *DefaultCtx) Port() string {
tcpaddr, ok := c.fasthttp.RemoteAddr().(*net.TCPAddr)
if !ok {
panic(fmt.Errorf("failed to type-assert to *net.TCPAddr"))
panic(errors.New("failed to type-assert to *net.TCPAddr"))
}
return strconv.Itoa(tcpaddr.Port)
}
Expand Down Expand Up @@ -1108,12 +1109,8 @@ func (c *DefaultCtx) Queries() map[string]string {
// age := Query[int](c, "age") // Returns 8
// unknown := Query[string](c, "unknown", "default") // Returns "default" since the query parameter "unknown" is not found
func Query[V QueryType](c Ctx, key string, defaultValue ...V) V {
ctx, ok := c.(*DefaultCtx)
if !ok {
panic(fmt.Errorf("failed to type-assert to *DefaultCtx"))
}
var v V
q := ctx.app.getString(ctx.fasthttp.QueryArgs().Peek(key))
q := c.App().getString(c.Context().QueryArgs().Peek(key))

switch any(v).(type) {
case int:
Expand Down Expand Up @@ -1151,7 +1148,7 @@ func Query[V QueryType](c Ctx, key string, defaultValue ...V) V {
if q == "" && len(defaultValue) > 0 {
return defaultValue[0]
}
return assertValueType[V, []byte](ctx.app.getBytes(q))
return assertValueType[V, []byte](c.App().getBytes(q))
default:
if len(defaultValue) > 0 {
return defaultValue[0]
Expand Down
4 changes: 2 additions & 2 deletions ctx_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package fiber
import (
"context"
"crypto/tls"
"fmt"
"errors"
"io"
"mime/multipart"
"sync"
Expand Down Expand Up @@ -446,7 +446,7 @@ func (app *App) NewCtx(fctx *fasthttp.RequestCtx) Ctx {
func (app *App) AcquireCtx() Ctx {
ctx, ok := app.pool.Get().(Ctx)
if !ok {
panic(fmt.Errorf("failed to type-assert to Ctx"))
panic(errors.New("failed to type-assert to Ctx"))
}
return ctx
}
Expand Down
6 changes: 3 additions & 3 deletions ctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ func Test_Ctx_FormValue(t *testing.T) {
require.NoError(t, writer.Close())

req := httptest.NewRequest(MethodPost, "/test", body)
req.Header.Set("Content-Type", fmt.Sprintf("multipart/form-data; boundary=%s", writer.Boundary()))
req.Header.Set("Content-Type", "multipart/form-data; boundary="+writer.Boundary())
req.Header.Set("Content-Length", strconv.Itoa(len(body.Bytes())))

resp, err := app.Test(req)
Expand Down Expand Up @@ -2155,7 +2155,7 @@ func Test_Ctx_MultipartForm(t *testing.T) {
require.NoError(t, writer.Close())

req := httptest.NewRequest(MethodPost, "/test", body)
req.Header.Set(HeaderContentType, fmt.Sprintf("multipart/form-data; boundary=%s", writer.Boundary()))
req.Header.Set(HeaderContentType, "multipart/form-data; boundary="+writer.Boundary())
req.Header.Set(HeaderContentLength, strconv.Itoa(len(body.Bytes())))

resp, err := app.Test(req)
Expand Down Expand Up @@ -4181,7 +4181,7 @@ func Test_Ctx_Render_Go_Template(t *testing.T) {
require.NoError(t, err)
}()

_, err = file.Write([]byte("template"))
_, err = file.WriteString("template")
require.NoError(t, err)

err = file.Close()
Expand Down
6 changes: 5 additions & 1 deletion error.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ import (
// Unexported because users will hopefully never need to see it.
var errUnreachable = errors.New("fiber: unreachable code, please create an issue at github.com/gofiber/fiber")

// Graceful shutdown errors
// General errors
var (
ErrGracefulTimeout = errors.New("shutdown: graceful timeout has been reached, exiting")
// ErrNotRunning indicates that a Shutdown method was called when the server was not running.
ErrNotRunning = errors.New("shutdown: server is not running")
// ErrHandlerExited is returned by App.Test if a handler panics or calls runtime.Goexit().
ErrHandlerExited = errors.New("runtime.Goexit() called in handler or server panic")
)

// Fiber redirection errors
Expand Down
39 changes: 21 additions & 18 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package fiber
import (
"bytes"
"crypto/tls"
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -41,25 +42,27 @@ func getTLSConfig(ln net.Listener) *tls.Config {
pointer := reflect.ValueOf(ln)

// Is it a tls.listener?
if pointer.String() == "<*tls.listener Value>" {
// Copy value from pointer
if val := reflect.Indirect(pointer); val.Type() != nil {
// Get private field from value
if field := val.FieldByName("config"); field.Type() != nil {
// Copy value from pointer field (unsafe)
newval := reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())) //nolint:gosec // Probably the only way to extract the *tls.Config from a net.Listener. TODO: Verify there really is no easier way without using unsafe.
if newval.Type() != nil {
// Get element from pointer
if elem := newval.Elem(); elem.Type() != nil {
// Cast value to *tls.Config
c, ok := elem.Interface().(*tls.Config)
//nolint:revive // We need to check if the type assertion was successful
if !ok {
panic(fmt.Errorf("failed to type-assert to *tls.Config"))
}
return c
}
if pointer.String() != "<*tls.listener Value>" {
return nil
}

// Copy value from pointer
if val := reflect.Indirect(pointer); val.Type() != nil {
// Get private field from value
if field := val.FieldByName("config"); field.Type() != nil {
// Copy value from pointer field (unsafe)
newval := reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())) //nolint:gosec // Probably the only way to extract the *tls.Config from a net.Listener. TODO: Verify there really is no easier way without using unsafe.
if newval.Type() == nil {
return nil
}
// Get element from pointer
if elem := newval.Elem(); elem.Type() != nil {
// Cast value to *tls.Config
c, ok := elem.Interface().(*tls.Config)
if !ok {
panic(errors.New("failed to type-assert to *tls.Config"))
}
return c
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions middleware/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func Test_Cache_Expired(t *testing.T) {
app.Use(New(Config{Expiration: 2 * time.Second}))

app.Get("/", func(c fiber.Ctx) error {
return c.SendString(fmt.Sprintf("%d", time.Now().UnixNano()))
return c.SendString(strconv.FormatInt(time.Now().UnixNano(), 10))
})

resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
Expand Down Expand Up @@ -88,7 +88,7 @@ func Test_Cache(t *testing.T) {
app.Use(New())

app.Get("/", func(c fiber.Ctx) error {
now := fmt.Sprintf("%d", time.Now().UnixNano())
now := strconv.FormatInt(time.Now().UnixNano(), 10)
return c.SendString(now)
})

Expand Down Expand Up @@ -307,7 +307,7 @@ func Test_Cache_Invalid_Expiration(t *testing.T) {
app.Use(cache)

app.Get("/", func(c fiber.Ctx) error {
now := fmt.Sprintf("%d", time.Now().UnixNano())
now := strconv.FormatInt(time.Now().UnixNano(), 10)
return c.SendString(now)
})

Expand Down Expand Up @@ -513,8 +513,7 @@ func Test_CustomExpiration(t *testing.T) {

app.Get("/", func(c fiber.Ctx) error {
c.Response().Header.Add("Cache-Time", "1")
now := fmt.Sprintf("%d", time.Now().UnixNano())
return c.SendString(now)
return c.SendString(strconv.FormatInt(time.Now().UnixNano(), 10))
})

resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
Expand Down Expand Up @@ -620,7 +619,7 @@ func Test_Cache_WithHead(t *testing.T) {
app.Use(New())

handler := func(c fiber.Ctx) error {
now := fmt.Sprintf("%d", time.Now().UnixNano())
now := strconv.FormatInt(time.Now().UnixNano(), 10)
return c.SendString(now)
}
app.Route("/").Get(handler).Head(handler)
Expand Down

0 comments on commit 4c68e02

Please sign in to comment.