From 5449b0410161225a09cf8f02b1d41b3bdc232314 Mon Sep 17 00:00:00 2001 From: leonklingele Date: Sun, 17 Mar 2024 13:46:20 +0100 Subject: [PATCH] feat: make golangci-lint config stricter (#2874) --- .golangci.yml | 274 ++++++++++++++++----- listen_test.go | 2 +- middleware/cors/cors.go | 5 +- middleware/idempotency/idempotency_test.go | 7 +- middleware/session/store.go | 2 +- path.go | 7 +- redirect.go | 7 +- 7 files changed, 225 insertions(+), 79 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9dfb719117..965b5cab06 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,45 +1,98 @@ -# Created based on v1.51.0 -# NOTE: Keep this in sync with the version in .github/workflows/linter.yml +# v1.2.0. Created based on golangci-lint v1.56.2 run: - modules-download-mode: readonly + timeout: 5m skip-dirs-use-default: false + modules-download-mode: readonly + allow-serial-runners: true skip-dirs: - - internal + - internal # TODO: Do not ignore interal output: sort-results: true + uniq-by-line: false linters-settings: - testifylint: - enable-all: true + depguard: + rules: + all: + list-mode: lax + deny: + - pkg: "flag" + desc: '`flag` package is only allowed in main.go' + - pkg: "log" + desc: 'logging is provided by `pkg/log`' + - pkg: "io/ioutil" + desc: '`io/ioutil` package is deprecated, use the `io` and `os` package instead' + # TODO: Prevent using these without a reason + # - pkg: "reflect" + # desc: '`reflect` package is dangerous to use' + # - pkg: "unsafe" + # desc: '`unsafe` package is dangerous to use' + errcheck: - check-type-assertions: true - check-blank: true - disable-default-exclusions: true - exclude-functions: - - '(*bytes.Buffer).Write' # always returns nil error - - '(*github.com/valyala/bytebufferpool.ByteBuffer).Write' # always returns nil error - - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteByte' # always returns nil error - - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteString' # always returns nil error + check-type-assertions: true + check-blank: true + disable-default-exclusions: true + exclude-functions: + - '(*bytes.Buffer).Write' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).Write' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteByte' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteString' # always returns nil error errchkjson: report-no-exported: true exhaustive: + check-generated: true default-signifies-exhaustive: true forbidigo: forbid: - - ^(fmt\.Print(|f|ln)|print|println)$ - - 'http\.Default(Client|Transport)' + - ^print(ln)?$ + - ^fmt\.Print(f|ln)?$ + - ^http\.Default(Client|ServeMux|Transport)$ # TODO: Eventually enable these patterns - # - 'time\.Sleep' - # - 'panic' + # - ^panic$ + # - ^time\.Sleep$ + analyze-types: true + + gci: + sections: + - standard + - prefix(github.com/gofiber/fiber) + - default + - blank + - dot + # - alias + custom-order: true + + goconst: + numbers: true gocritic: + # TODO: Uncomment the following lines + # enabled-tags: + # - diagnostic + # - style + # - performance + # - experimental + # - opinionated disabled-checks: - - ifElseChain + - ifElseChain # TODO: Do not disable + # - hugeParam + # - rangeExprCopy + # - rangeValCopy + settings: + captLocal: + paramsOnly: false + elseif: + skipBalanced: false + underef: + skipRecvDeref: false + # NOTE: Set this option to false if other projects rely on this project's code + # unnamedResult: + # checkExported: false gofumpt: module-path: github.com/gofiber/fiber @@ -47,31 +100,27 @@ linters-settings: gosec: excludes: - - G104 # Provided by errcheck + - G104 # TODO: Enable this again. Mostly provided by errcheck config: global: + # show-ignored: true # TODO: Enable this audit: true - - depguard: - rules: - main: - deny: - - pkg: flag - desc: '`flag` package is only allowed in main.go' - - pkg: io/ioutil - desc: '`io/ioutil` package is deprecated, use the `io` and `os` package instead' govet: - check-shadowing: true enable-all: true disable: - - shadow - fieldalignment - - loopclosure + - shadow grouper: + # const-require-grouping: true # TODO: Enable this import-require-single-import: true import-require-grouping: true + # var-require-grouping: true # TODO: Conflicts with gofumpt + + loggercheck: + require-string-key: true + no-printf-like: true misspell: locale: US @@ -83,12 +132,20 @@ linters-settings: nonamedreturns: report-error-in-defer: true + perfsprint: + err-error: true + predeclared: q: true promlinter: strict: true + # TODO: Enable this + # reassign: + # patterns: + # - '.*' + revive: enable-all-rules: true rules: @@ -103,18 +160,25 @@ linters-settings: - name: cognitive-complexity disabled: true - name: comment-spacings - disabled: true # TODO https://github.com/gofiber/fiber/issues/2816 + arguments: + - nolint + disabled: true # TODO: Do not disable - name: cyclomatic disabled: true - - name: early-return - severity: warning - disabled: true + # TODO: Enable this check. Currently disabled due to upstream bug. + # - name: enforce-repeated-arg-type-style + # arguments: + # - short + - name: enforce-slice-style + arguments: + - make + disabled: true # TODO: Do not disable - name: exported disabled: true - name: file-header disabled: true - name: function-result-limit - disabled: true + arguments: [3] - name: function-length disabled: true - name: line-length-limit @@ -124,14 +188,13 @@ linters-settings: - name: modifies-parameter disabled: true - name: nested-structs - disabled: true + disabled: true # TODO: Do not disable - name: package-comments disabled: true - - name: unchecked-type-assertion - disabled: true # TODO https://github.com/gofiber/fiber/issues/2816 - # Provided by errcheck - - name: unhandled-error + - name: optimize-operands-order disabled: true + - name: unchecked-type-assertion + disabled: true # TODO: Do not disable stylecheck: checks: @@ -141,6 +204,9 @@ linters-settings: - -ST1021 - -ST1022 + tagalign: + strict: true + tagliatelle: case: rules: @@ -149,8 +215,41 @@ linters-settings: tenv: all: true - #unparam: - # check-exported: true + testifylint: + enable-all: true + # TODO: Do not disable any options + disable: + - go-require + + testpackage: + skip-regexp: "^$" + + unparam: + # NOTE: Set this option to false if other projects rely on this project's code + check-exported: false + + unused: + # TODO: Uncomment these two lines + # parameters-are-used: false + # local-variables-are-used: false + # NOTE: Set these options to true if other projects rely on this project's code + field-writes-are-uses: true + # exported-is-used: true # TODO: Fix issues with this option (upstream) + exported-fields-are-used: true + + usestdlibvars: + http-method: true + http-status-code: true + time-weekday: false # TODO: Set to true + time-month: false # TODO: Set to true + time-layout: false # TODO: Set to true + crypto-hash: true + default-rpc-path: true + os-dev-null: true + sql-isolation-level: true + tls-signature-scheme: true + constant-kind: true + syslog-priority: true wrapcheck: ignorePackageGlobs: @@ -158,18 +257,24 @@ linters-settings: - github.com/valyala/fasthttp issues: - exclude-use-default: false - - exclude-rules: - # Exclude some linters from running on tests files. - - path: _test\.go - linters: - - bodyclose + exclude-use-default: false + exclude-case-sensitive: true + max-issues-per-linter: 0 + max-same-issues: 0 + exclude-rules: + - linters: + - goerr113 + text: 'do not define dynamic errors, use wrapped static errors instead*' + - path: log/.*\.go + linters: + - depguard + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - bodyclose + # fix: true linters: - disable: - - spancheck # opentelemetry, irrelevant - - tagalign # requires awkward manual formatting of struct tags enable: - asasalint - asciicheck @@ -177,8 +282,12 @@ linters: - bodyclose - containedctx - contextcheck + # - cyclop + - decorder - depguard - dogsled + # - dupl + # - dupword # TODO: Enable - durationcheck - errcheck - errchkjson @@ -186,45 +295,84 @@ linters: - errorlint - execinquery - exhaustive + # - exhaustivestruct + # - exhaustruct - exportloopref - forbidigo - forcetypeassert - # - gochecksumtype # TODO https://github.com/gofiber/fiber/issues/2816 - # - goconst # TODO https://github.com/gofiber/fiber/issues/2816 + # - funlen + # - gci # TODO: Enable + - ginkgolinter + # - gocheckcompilerdirectives # TODO: Enable + # - gochecknoglobals # TODO: Enable + # - gochecknoinits # TODO: Enable + - gochecksumtype + # - gocognit + # - goconst # TODO: Enable - gocritic + # - gocyclo + # - godot + # - godox + - goerr113 - gofmt - gofumpt - - goimports + # - goheader + # - goimports + # - golint + # - gomnd # TODO: Enable - gomoddirectives + # - gomodguard - goprintffuncname - gosec - gosimple - # - gosmopolitan # TODO https://github.com/gofiber/fiber/issues/2816 + # - gosmopolitan # TODO: Enable - govet - grouper - - inamedparam + # - ifshort # TODO: Enable + # - importas + # - inamedparam + - ineffassign + # - interfacebloat + # - interfacer + # - ireturn + # - lll - loggercheck + # - maintidx + - makezero + # - maligned - mirror - misspell + - musttag - nakedret + # - nestif - nilerr - nilnil + # - nlreturn - noctx - nolintlint - nonamedreturns - nosprintfhostport + # - paralleltest # TODO: Enable - perfsprint + # - prealloc - predeclared - promlinter + - protogetter - reassign - revive - rowserrcheck + # - scopelint # TODO: Enable + - sloglint + - spancheck - sqlclosecheck - staticcheck - stylecheck + # - tagalign # TODO: Enable - tagliatelle + - tenv + - testableexamples - testifylint - # - testpackage # TODO: Enable once https://github.com/gofiber/fiber/issues/2252 is implemented + # - testpackage # TODO: Enable - thelper - tparallel - typecheck @@ -232,7 +380,9 @@ linters: - unparam - unused - usestdlibvars - # - wastedassign # TODO https://github.com/gofiber/fiber/issues/2816 + # - varnamelen + # - wastedassign # TODO: Enable - whitespace - wrapcheck - - tenv + # - wsl + - zerologlint diff --git a/listen_test.go b/listen_test.go index 762fe8291d..b8d249f431 100644 --- a/listen_test.go +++ b/listen_test.go @@ -8,7 +8,7 @@ import ( "errors" "fmt" "io" - "log" + "log" //nolint:depguard // TODO: Required to capture output, use internal log package instead "net" "os" "strings" diff --git a/middleware/cors/cors.go b/middleware/cors/cors.go index 0869c4d90d..ee99b189d1 100644 --- a/middleware/cors/cors.go +++ b/middleware/cors/cors.go @@ -126,11 +126,10 @@ func New(config ...Config) fiber.Handler { trimmedOrigin := strings.TrimSpace(origin) isValid, normalizedOrigin := normalizeOrigin(trimmedOrigin) - if isValid { - allowOrigins[i] = normalizedOrigin - } else { + if !isValid { log.Panicf("[CORS] Invalid origin format in configuration: %s", trimmedOrigin) //nolint:revive // we want to exit the program } + allowOrigins[i] = normalizedOrigin } } else { // If AllowOrigins is set to a wildcard or not set, diff --git a/middleware/idempotency/idempotency_test.go b/middleware/idempotency/idempotency_test.go index f8a4030a1e..91394ca26a 100644 --- a/middleware/idempotency/idempotency_test.go +++ b/middleware/idempotency/idempotency_test.go @@ -42,11 +42,8 @@ func Test_Idempotency(t *testing.T) { if !isIdempotent { return errors.New("request with unsafe HTTP method should be idempotent if X-Idempotency-Key request header is set") } - } else { - // No request header - if isIdempotent { - return errors.New("request with unsafe HTTP method should not be idempotent if X-Idempotency-Key request header is not set") - } + } else if isIdempotent { + return errors.New("request with unsafe HTTP method should not be idempotent if X-Idempotency-Key request header is not set") } } diff --git a/middleware/session/store.go b/middleware/session/store.go index b176e5f596..c1f36ec983 100644 --- a/middleware/session/store.go +++ b/middleware/session/store.go @@ -75,7 +75,7 @@ func (s *Store) Get(c fiber.Ctx) (*Session, error) { if raw != nil && err == nil { mux.Lock() defer mux.Unlock() - sess.byteBuffer.Write(raw) + _, _ = sess.byteBuffer.Write(raw) // Ignore error, this will never fail encCache := gob.NewDecoder(sess.byteBuffer) err := encCache.Decode(&sess.data.Data) if err != nil { diff --git a/path.go b/path.go index a51b31562e..72c440c0f3 100644 --- a/path.go +++ b/path.go @@ -591,11 +591,12 @@ func findGreedyParamLen(s string, searchCount int, segment *routeSegment) int { // check all from right to left segments for i := segment.PartCount; i > 0 && searchCount > 0; i-- { searchCount-- - if constPosition := strings.LastIndex(s, segment.ComparePart); constPosition != -1 { - s = s[:constPosition] - } else { + + constPosition := strings.LastIndex(s, segment.ComparePart) + if constPosition == -1 { break } + s = s[:constPosition] } return len(s) diff --git a/redirect.go b/redirect.go index 98053db80a..71e71cca58 100644 --- a/redirect.go +++ b/redirect.go @@ -278,13 +278,12 @@ func (r *Redirect) setFlash() { var commaPos int for { commaPos = findNextNonEscapedCharsetPosition(cookieValue, []byte(CookieDataSeparator)) - if commaPos != -1 { - r.c.redirectionMessages = append(r.c.redirectionMessages, strings.Trim(cookieValue[:commaPos], " ")) - cookieValue = cookieValue[commaPos+1:] - } else { + if commaPos == -1 { r.c.redirectionMessages = append(r.c.redirectionMessages, strings.Trim(cookieValue, " ")) break } + r.c.redirectionMessages = append(r.c.redirectionMessages, strings.Trim(cookieValue[:commaPos], " ")) + cookieValue = cookieValue[commaPos+1:] } r.c.ClearCookie(FlashCookieName)