Files
1856433721 chore(lint): clear the lint baseline and fix six empty assertions (#217)
* chore(lint): clear the lint baseline and lint the whole repository

The v1.5.0 tag run failed Lint with 42 findings that had nothing to do
with the release. The chain:

  1. .golangci.yml set issues.new: true, which makes golangci-lint's diff
     processor run `git diff HEAD~`.
  2. On PR and main-push runs golangci-lint-action fetches a patch from
     the GitHub compare API and passes --new-from-patch --new=false, which
     overrides that config. So the setting was dead on every normal run.
  3. On the tag push the compare call returned 404, so the action invoked
     golangci-lint bare and the config took effect for the first time.
  4. actions/checkout defaults to fetch-depth: 1, so HEAD~ does not exist
     and the diff processor failed. golangci-lint treats that as a warning
     and falls back to reporting everything: 42 pre-existing findings.

issues.new also made local linting lie. `make lint` and the pre-commit
hook both run golangci-lint bare, so in a clean checkout they reported
0 issues while 42 stood.

Fix the 42, then drop both filters so every event type behaves the same
and no git history is needed:

  - testifylint (29): assert.Nil(t, err) -> require/assert.NoError,
    assert.Equal(t, "", x) -> assert.Empty, assert.True(t, strings
    .Contains(..)) -> assert.Contains. Error assertions now abort the
    test rather than continue, which is what require-error asks for.
  - noctx (2): gravatar.go and handler.go already built a ctx with a
    timeout and then threw it away via NewRequest + req.WithContext;
    they now call NewRequestWithContext. No behaviour change.
  - staticcheck (6): strings.ReplaceAll x2, fmt.Fprintf, getMD5Hash
    returns its error last (unexported, two call sites), and the
    unexported gravatarBaseUrl/gravatarUrl renamed to ...URL.
  - ST1003 on the exported Gravatar.GravatarUrl field, ST1005 on the two
    capitalised SMTP error strings, gosmopolitan on the deliberate IDN
    test input and noctx on the one-shot generator are suppressed with
    reasons. The two error strings are passed through verbatim by
    ParseSMTPError as LookupError.Message, so they are user-facing text
    consistent with the Err* constants in error.go, not internal Go
    error strings.

Upstream frames new-from-* as a migration aid for a large existing
codebase, and the FAQ notes that --new (this repo's issues.new) is less
safe than --new-from-rev because generated files derail it -- this repo
has a generator. Of nine comparable Go projects sampled, none set
issues.new and six of seven using golangci-lint-action do not set
only-new-issues.

Verified: a bare `golangci-lint run` and a depth-1 clone both report
0 issues; go build, go vet and 67 offline tests pass under -race. The
network-dependent tests are left to CI, as they make real SMTP
connections.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* test: make six vacuous error assertions actually assert

testifylint's require-error check surfaced these while clearing the lint
baseline. All six passed a value that reads as the expected error into
testify's msgAndArgs parameter, which is only used to decorate a failure
message. The expectation was never applied: the assertions checked no
more than err != nil.

Demonstrated on the 404 case -- with the old form, an expected
"status_code: 403" passes against an actual 404.

  mx_test.go:26          assert.Error(t, err, ErrNoSuchHost)
  smtp_test.go:141       assert.Error(t, err, ErrNoSuchHost)
  smtp_test.go:159       assert.Error(t, err, syscall.ECONNREFUSED)
  handler_test.go:186    assert.Error(t, err, "...status_code: 404")
  handler_test.go:196    assert.Error(t, err, "...status_code: 500")
  handler_test.go:219    assert.Error(t, err, "invalid character ...")

Each now uses an assertion that matches what is being expected: a full
message (EqualError), a substring (ErrorContains) or an error value
(ErrorIs).

mx_test.go needed more than a mechanical swap. CheckMX returns the
resolver's *net.DNSError unchanged -- "lookup githubexists.com on
...:53: no such host" -- so it never contained ErrNoSuchHost ("Mail
server does not exist"); it is Verify that maps it. Asserting the
message text would also bake in the resolver address, which varies by
environment, so the test now asserts the DNS semantics via ErrorAs and
IsNotFound.

Verified: each of the six fails when its expected value is mutated
(IsNotFound -> False, ErrNoSuchHost -> ErrTimeout, ECONNREFUSED ->
ETIMEDOUT, 404 -> 403, 500 -> 503, and one character of the JSON error).
70 offline tests pass under -race. The six touched tests reach the
network only via DNS, or via a SOCKS dial to 127.0.0.1; none opens an
SMTP connection.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* chore(lint): enable usestdlibvars and intrange

Both catch a legacy literal where the standard library offers a named
form, and both fix mechanically with no behaviour change. Seven findings:

  gravatar.go, handler.go        "GET" -> http.MethodGet
  gravatar.go                    200 -> http.StatusOK
  cmd/apiserver/main.go (x2)     500 -> http.StatusInternalServerError
  smtp.go, schedule_test.go      for i := 0; i < n; i++ -> for range n

Enabling the linters rather than only editing the call sites is the
point: with the baseline at zero there is nothing to migrate around, so
the rule is enforced for new code instead of being reapplied by hand.

Turning on every usestdlibvars opt-in as well (time-weekday, time-month,
time-layout, crypto-hash, default-rpc-path, sql-isolation-level,
tls-signature-scheme, constant-kind) surfaces nothing further, so the
defaults are left in place.

Two adjacent linters were surveyed and deliberately not enabled:

  - nilnil (1): smtp.go's `return nil, nil` is the contract CheckSMTP
    documents for a disabled SMTP check. Changing it is a public API
    decision, not a cleanup.
  - godot (3): requiring declaration comments to end in a period is a
    style preference that would bind every future contribution.

Verified: golangci-lint run reports 0, go build and go vet are clean,
70 offline tests pass under -race, and GenerateRandomEmail still yields
32 alphanumeric bytes after the loop rewrite.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
2026-09-11 10:09:20 +08:00

249 lines
8.0 KiB
YAML

version: "2"
run:
# Timeout for total work, e.g. 30s, 5m, 5m30s.
# If the value is lower or equal to 0, the timeout is disabled.
# Default: 0 (disabled)
timeout: 5m
# Number of operating system threads (`GOMAXPROCS`) that can execute golangci-lint simultaneously.
# Default: 0 (automatically set to match Linux container CPU quota and
# fall back to the number of logical CPUs in the machine)
concurrency: 4
# Exit code when at least one issue was found.
# Default: 1
issues-exit-code: 1
# Include test files or not.
# Default: true
tests: true
linters:
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- contextcheck
- durationcheck
- errchkjson
- errorlint
- exhaustive
- gocheckcompilerdirectives
- gochecksumtype
- gosec
- gosmopolitan
- intrange
- loggercheck
- makezero
- musttag
- nilerr
- nilnesserr
- noctx
- protogetter
- reassign
- recvcheck
- rowserrcheck
- spancheck
- sqlclosecheck
- testifylint
- unparam
- usestdlibvars
- zerologlint
disable:
- prealloc
settings:
dogsled:
# Checks assignments with too many blank identifiers.
# Default: 2
max-blank-identifiers: 2
dupl:
# Tokens count to trigger issue.
# Default: 150
threshold: 100
errcheck:
# Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
# Such cases aren't reported by default.
# Default: false
check-type-assertions: false
# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`.
# Such cases aren't reported by default.
# Default: false
check-blank: false
funlen:
# Checks the number of lines in a function.
# If lower than 0, disable the check.
# Default: 60
lines: 60
# Checks the number of statements in a function.
# If lower than 0, disable the check.
# Default: 40
statements: 40
gocognit:
# Minimal code complexity to report.
# Default: 30 (but we recommend 10-20)
min-complexity: 20
goconst:
# Minimal length of string constant.
# Default: 3
min-len: 3
# Minimum occurrences of constant string count to trigger issue.
# Default: 3
min-occurrences: 3
gocritic:
# Enable multiple checks by tags in addition to default checks.
# Run `GL_DEBUG=gocritic golangci-lint run --enable=gocritic` to see all tags and checks.
# See https://github.com/go-critic/go-critic#usage -> section "Tags".
# Default: []
enabled-tags:
- performance
settings:
captLocal:
# Whether to restrict checker to params only.
# Default: true
paramsOnly: true
rangeValCopy:
# Size in bytes that makes the warning trigger.
# Default: 128
sizeThreshold: 32
gocyclo:
# Minimal code complexity to report.
# Default: 30 (but we recommend 10-20)
min-complexity: 10
gosec:
# To specify a set of rules to explicitly exclude.
# Available rules: https://github.com/securego/gosec#available-rules
# Default: []
excludes: []
# - G107 # Url provided to HTTP request as taint input
# - G404 # Insecure random number source (rand)
# - G501 # Import blocklist: crypto/md5
govet:
enable:
# Check for non-64-bits-aligned arguments to sync/atomic functions.
- atomicalign
disable:
# Check for possible unintended shadowing of variables.
- shadow
enable-all: false
disable-all: false
# Settings per analyzer.
settings:
# Analyzer name, run `go tool vet help` to see all analyzers.
printf:
# Comma-separated list of print function names to check (in addition to default, see `go tool vet help printf`).
# Default: []
funcs:
- (github.com/golangci/golangci-lint/v2/pkg/logutils.Log).Infof
- (github.com/golangci/golangci-lint/v2/pkg/logutils.Log).Warnf
- (github.com/golangci/golangci-lint/v2/pkg/logutils.Log).Errorf
- (github.com/golangci/golangci-lint/v2/pkg/logutils.Log).Fatalf
lll:
# Max line length, lines longer will be reported.
# '\t' is counted as 1 character by default, and can be changed with the tab-width option.
# Default: 120.
line-length: 120
# Tab width in spaces.
# Default: 1
tab-width: 1
misspell:
# Correct spellings using locale preferences for US or UK.
# Setting locale to US will correct the British spelling of 'colour' to 'color'.
# Default is to use a neutral variety of English.
locale: US
# Typos to ignore.
# Should be in lower case.
ignore-rules:
- aftership
- automizely
nakedret:
# Make an issue if func has more lines of code than this setting, and it has naked returns.
# Default: 30
max-func-lines: 30
prealloc:
# Report pre-allocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
# Default: true
simple: true
# Report pre-allocation suggestions on range loops.
# Default: true
range-loops: true
# Report pre-allocation suggestions on for loops.
# Default: false
for-loops: false
staticcheck:
# SAxxxx checks in https://staticcheck.dev/docs/configuration/options/#checks
# Example (to disable some checks): [ "all", "-SA1000", "-SA1001"]
# Run `GL_DEBUG=staticcheck golangci-lint run --enable=staticcheck` to see all available checks and enabled by config checks.
# Default: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
checks:
- all
# - -SA1029
whitespace:
# Enforces newlines (or comments) after every multi-line if statement.
# Default: false
multi-if: false
# Enforces newlines (or comments) after every multi-line function signature.
# Default: false
multi-func: false
# Defines a set of rules to ignore issues.
# It does not skip the analysis, and so does not ignore "typecheck" errors.
exclusions:
# Mode of the generated files analysis.
#
# - `strict`: sources are excluded by strictly following the Go generated file convention.
# Source files that have lines matching only the following regular expression will be excluded: `^// Code generated .* DO NOT EDIT\.$`
# This line must appear before the first non-comment, non-blank text in the file.
# https://go.dev/s/generatedcode
# - `lax`: sources are excluded if they contain lines like `autogenerated file`, `code generated`, `do not edit`, etc.
# - `disable`: disable the generated files exclusion.
#
# Default: strict
generated: lax
# Predefined exclusion rules.
# Default: []
presets:
- comments
- std-error-handling
- common-false-positives
- legacy
# Excluding configuration per-path, per-linter, per-text and per-source.
rules:
# Exclude some linters from running on tests files.
- path: _test\.go
linters:
- dupl
- errcheck
- gocyclo
- gosec
# Exclude `lll` issues for long lines with `go:generate`.
- linters:
- lll
source: '^//go:generate '
paths:
- third_party$
- builtin$
- examples$
issues:
# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
max-issues-per-linter: 0
# Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
max-same-issues: 0
formatters:
# Enable specific formatter.
# Default: [] (uses standard Go formatting)
enable:
- gofmt
- goimports
settings:
gofmt:
# Simplify code: gofmt with `-s` option.
# Default: true
simplify: true
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$