Skip to content

Re-enable default golangci-lint v2 linters (govet, staticcheck, unused) #367

@schmidtw

Description

@schmidtw

Summary

The golangci-lint v2 migration (PR #365, PR #366) temporarily disabled the default linters (govet, staticcheck, unused) to maintain parity with the v1 configuration. However, these linters provide valuable code quality checks and should be re-enabled after addressing the issues they report.

Background

In golangci-lint v2, several linters are enabled by default:

  • govet - Vet examines Go source code and reports suspicious constructs
  • staticcheck - Set of rules for code quality and style
  • unused - Checks for unused constants, variables, functions and types

These were disabled in PR #366 to prevent breaking the CI, but they should be re-enabled to improve code quality.

Issues to Fix

Before re-enabling these linters, the following issues must be resolved:

gosec (7 issues)

G104: Unhandled errors - 7 occurrences

These are similar to errcheck violations. We should either:

  1. Add gosec.excludes: [G104] to permanently ignore these
  2. OR handle these errors properly

Locations:

  • basculehash/bcrypt_test.go:32 - suite.badHash() result not checked
  • basculehttp/challenge.go:259 - c.Parameters.SetRealm(realm) error not checked
  • basculehttp/challenge.go:261 - c.Parameters.SetCharset("UTF-8") error not checked
  • basculehttp/middleware.go:206 - response.Write(errBody) error not checked
  • basculehttp/middleware.go:242 - response.Write(content) error not checked
  • basculehttp/middleware_test.go:35 - response.Write() error not checked
  • cmd/hash/cli.go:50 - digest.WriteTo(kong.Stdout) error not checked

staticcheck (12 issues)

ST1005: Error strings should not be capitalized - 10 occurrences

Error messages should start with lowercase letters unless they begin with proper nouns or acronyms.

Locations:

  • basculehttp/basculecaps/approver.go:51 - "Unable to compile..." → "unable to compile..."
  • basculehttp/basculecaps/approver.go:54 - "The prefix..." → "the prefix..."
  • basculehttp/challenge.go:33 - "Invalid challenge..." → "invalid challenge..."
  • basculehttp/challenge.go:38 - "Invalid challenge..." → "invalid challenge..."
  • basculehttp/challenge.go:45 - "Reserved challenge..." → "reserved challenge..."
  • basculehttp/challenge.go:208 - "Odd number..." → "odd number..."
  • basculehttp/middleware.go:18 - "An Authenticator..." → "an Authenticator..."
  • cmd/hash/cli.go:30 - "Cost cannot..." → "cost cannot..."
  • cmd/hash/cli.go:33 - "Cost cannot..." → "cost cannot..."
  • cmd/hash/cli.go:36 - "Plaintext length..." → "plaintext length..."

ST1023: Should omit type from declaration - 2 occurrences

Type can be inferred from right-hand side.

Locations:

  • token_test.go:295 - var cf Custom = Custom(...)var cf = Custom(...)
  • token_test.go:314 - var cf Custom = Custom(...)var cf = Custom(...)

Recommendation

Option 1: Fix All Issues (Recommended)

  1. Fix all 12 staticcheck issues (straightforward)
  2. Decide on gosec G104:
    • Either handle the errors properly
    • Or exclude G104 via config if we agree error handling isn't critical in these locations
  3. Re-enable govet, staticcheck, unused in .golangci.yaml

Option 2: Gradual Approach

  1. Fix staticcheck issues first (easy wins)
  2. Create a separate issue for gosec G104 decisions
  3. Re-enable linters incrementally

Total Issues: 19

  • gosec (G104): 7 issues - Decision needed on error handling
  • staticcheck (ST1005): 10 issues - Fix capitalization
  • staticcheck (ST1023): 2 issues - Remove redundant type declarations

Once these are resolved, update .golangci.yaml to remove govet, staticcheck, and unused from the disable list.

Metadata

Metadata

Assignees

No one assigned

    Labels

    normalizeRepo normalization work

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions