chore: additional linting rules and fixes

.golangci.yml

@@ -37,9 +37,45 @@ formatters:
 linters:
   # Disable everything first to have full control
   disable-all: true
+  exclusions:
+    rules:
+      - path: _test\.go
+        linters:
+          - nilnil       # mocks often return (nil, nil) for success
+          - err113       # mocks and test helpers use dynamic errors
+          - testpackage  # tests use same package (white-box); enable when adopting _test package
   settings:
     lll:
-      line-length: 164  # enforce max line length; lower to 120–140 and fix lines to tighten
+      line-length: 164  # consider 120–140 for new code; many existing lines exceed 140
+    # Struct tag naming: API uses snake_case for json/form/validate/river
+    tagliatelle:
+      case:
+        rules:
+          json: snake
+          form: snake
+          validate: snake
+          river: snake
+    # Ignore common Go idioms (handlers w/r, id, db, test tt, etc.)
+    varnamelen:
+      ignore-names:
+        - id
+        - w
+        - r
+        - db
+        - tt
+        - s
+        - m
+        - mp
+        - rw
+        - wh
+        - i
+        - ss
+        - p
+    # Optional: enable once long functions are refactored (excluded in tests via exclude-rules)
+    # funlen: { lines: 80, statements: 50, ignore-comments: true }
+    # Nesting: only flag deeply nested if (min-complexity 5)
+    nestif:
+      min-complexity: 5
   enable:
     ## 1. Bugs & Logic
     - bodyclose     # Checks HTTP response bodies are closed
@@ -54,42 +90,92 @@ linters:
     - sqlclosecheck # checks if rows/statements are closed
     - durationcheck # detects multiplying two durations (logic error)
     - copyloopvar   # detects loop variable capture issues (modern replacement for exportloopref)
+    - asasalint     # flags passing []any as any in variadic func(...any) (can hide type bugs)
+    - containedctx  # ensures structs that store context.Context use it (no unused ctx fields)
+    - errchkjson    # checks that json.Marshal/Unmarshal (and similar) errors are handled
+    - fatcontext    # discourages storing many values in context (use structs/params instead)
+    - nilnesserr    # checks for nilness issues in error handling paths
+    - nilnil        # flags return nil, nil (often a bug; prefer explicit sentinel or error)
+    - nosprintfhostport # use net.JoinHostPort instead of fmt.Sprintf for host:port
     - spancheck     # OpenTelemetry spans: End(), RecordError(), SetStatus() usage
+    - reassign      # Flags reassignment of package-level variables
+    - wastedassign  # Assignments overwritten before use
+    - unqueryvet    # SQL: detects SELECT *, format strings, etc.
 
     ## 2. Modern Error Handling
     - errorlint     # Enforces errors.Is/As and fmt.Errorf("%w") (CRITICAL for modern Go)
     - wrapcheck     # Enforces wrapping errors from external packages for context
+    - errname       # Sentinel errors: Err prefix; error types: Error suffix
+    - err113        # Stricter error handling (sentinel errors, error types, no dynamic errors)
 
     ## 3. Performance
     - perfsprint    # Checks for optimized usage of fmt.Sprintf
     - prealloc      # Encourages pre-allocating slices
     - makezero      # Prevents slice initialization with length + append (common perf bug)
 
-    ## 4. Security
+    ## 4. Security & API safety
     - gosec         # Security scanner (SQLi, hardcoded creds, etc.)
+    - gosmopolitan  # i18n/l10n anti-patterns (timezone, locale, etc.)
+    - musttag       # Enforce json/form/validate tags on (un)marshaled structs (prevents accidental API breaks)
 
     ## 4b. Exhaustiveness & stdlib
     - exhaustive    # Exhaustive switch on enum-like types (catch missing cases)
     - usestdlibvars # Prefer http.StatusOK, time.Kitchen etc. over magic numbers
+    - exptostd      # Replace golang.org/x/exp with stdlib when possible
+
+    ## 4c. Interfaces & types
+    - iface           # Interface misuse and "interface pollution"
+    - inamedparam     # Interfaces should use named method parameters
+    - interfacebloat  # Limits number of methods in an interface
+    - intrange        # Suggests integer range in for loops where applicable
+    - iotamixing      # Iota mixed with non-iota in same const block
+    - recvcheck       # Receiver type consistency (value vs pointer)
+    - unconvert       # Removes unnecessary type conversions
 
     ## 5. Style & Formatting (The "Clean" factor)
+    - decorder                   # Declaration order and count of types, constants, variables, functions
+    - embeddedstructfieldcheck   # Embedded types at top of struct, blank line before normal fields
+    - funcorder     # Order of functions/methods/constructors
+    - varnamelen    # Variable name length should match scope (short names for short scope)
     - lll           # Max line length (enforces readable line width)
     - whitespace    # Trailing newlines, unnecessary blank lines, spaces (consistency with if/for etc.)
+    - wsl_v5        # Empty-line / statement grouping (granular checks)
+    - nlreturn      # Newline before return and branch statements (complements wsl_v5)
+    - asciicheck    # Identifiers must use only ASCII characters
+    - bidichk       # Dangerous Unicode bidirectional characters
+    - canonicalheader # net/http.Header canonical key format
+    - dogsled       # Too many blank identifiers in assignments
+    - dupword       # Duplicate words in source (e.g. "the the")
     - revive        # Faster, configurable replacement for golint
     - misspell      # Fixes typos
     - godot         # Comments should end in a period
+    - godoclint     # Checks godoc style and conventions
+    - godox         # Finds FIXME, TODO, BUG, etc. in comments
     - unparam       # Reports unused function parameters
     - unused        # Reports unused constants, variables, functions and types
     - sloglint      # Structured logging (log/slog) best practices — you use slog
+    - loggercheck   # Key-value pairs for slog, zap, logr, klog, kitlog
     - promlinter    # Prometheus metrics naming (promlint)
+    - importas      # Enforces consistent import aliases (configure alias map in settings)
+    - tagliatelle   # Struct tag naming (e.g. json, yaml) case
+    - tagalign      # Align and sort struct tags
+    - grouper       # Grouping of expressions (e.g. var blocks)
+    - nakedret      # Restricts or disallows naked returns
+    - goprintffuncname # Printf-like functions must have name ending in f
 
     ## 6. Code Quality/Complexity
+    - nestif        # Deeply nested if statements (reduces nesting complexity)
     - gocritic      # Opinionated checks for bugs, performance, and style
     - modernize     # Suggests newer Go language features
+    - mnd           # Magic number detection (prefer named constants)
     - nolintlint    # Ensures //nolint comments are actually useful and formatted
 
     ## 7. Testing
-    - testifylint   # Checks for misuse of testify/require and testify/assert
+    - testifylint       # Checks for misuse of testify/require and testify/assert
+    - usetesting        # Prefer newer testing APIs (e.g. context.Background vs context.TODO in tests)
+    - tparallel         # Flags inappropriate use of t.Parallel()
+    - testpackage       # Encourages _test package for black-box tests
+    - testableexamples  # Example functions should have expected output
 
 issues:
   # Maximum issues count per one linter. Set to 0 to disable.
@@ -109,6 +195,7 @@ issues:
         - funlen        # tests can be long
         - exhaustive    # test switch on types often intentionally partial
         - lll           # test URLs and mock signatures are often long
+        - err113        # mocks and test helpers use dynamic errors
     # Integration tests (long URLs, request setup)
     - path: tests/
       linters:

cmd/api/app.go

@@ -1,4 +1,3 @@
-// Package main provides the application lifecycle: bootstrap, run, and shutdown.
 package main
 
 import (
@@ -23,14 +22,22 @@ import (
 	"github.com/formbricks/hub/internal/workers"
 )
 
+const (
+	metricsReadHeaderTimeout = 10 * time.Second
+	serverReadTimeout        = 15 * time.Second
+	serverWriteTimeout       = 15 * time.Second
+	serverIdleTimeout        = 60 * time.Second
+	auxShutdownTimeout       = 5 * time.Second
+)
+
 // App holds all server dependencies and coordinates startup and shutdown.
 type App struct {
 	cfg           *config.Config
 	db            *pgxpool.Pool
 	server        *http.Server
 	river         *river.Client[pgx.Tx]
 	message       *service.MessagePublisherManager
-	runCtx        context.Context
+	runCtx        context.Context //nolint:containedctx // used for coordinated shutdown
 	cancel        context.CancelFunc
 	metricsServer *http.Server
 	meterProvider observability.MeterProviderShutdown
@@ -42,21 +49,26 @@ type App struct {
 func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) {
 	ctx, cancel := context.WithCancel(context.Background())
 
-	var metrics observability.HubMetrics
-	var meterProvider observability.MeterProviderShutdown
-	var metricsServer *http.Server
+	var (
+		metrics       observability.HubMetrics
+		meterProvider observability.MeterProviderShutdown
+		metricsServer *http.Server
+	)
+
 	if cfg.PrometheusEnabled {
 		mp, metricsHandler, m, err := observability.NewMeterProvider(ctx, observability.MeterProviderConfig{})
 		if err != nil {
 			cancel()
+
 			return nil, fmt.Errorf("create MeterProvider: %w", err)
 		}
+
 		meterProvider = mp
 		metrics = m
 		metricsServer = &http.Server{
 			Addr:              ":" + cfg.PrometheusExporterPort,
 			Handler:           metricsHandler,
-			ReadHeaderTimeout: 10 * time.Second,
+			ReadHeaderTimeout: metricsReadHeaderTimeout,
 		}
 	}
 
@@ -77,6 +89,7 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) {
 	if err != nil {
 		cancel()
 		messageManager.Shutdown()
+
 		return nil, fmt.Errorf("create River client: %w", err)
 	}
 
@@ -136,12 +149,13 @@ func newHTTPServer(cfg *config.Config, health *handlers.HealthHandler, feedback
 	if metrics != nil {
 		handler = middleware.Metrics(metrics)(handler)
 	}
+
 	return &http.Server{
 		Addr:         ":" + cfg.Port,
 		Handler:      handler,
-		ReadTimeout:  15 * time.Second,
-		WriteTimeout: 15 * time.Second,
-		IdleTimeout:  60 * time.Second,
+		ReadTimeout:  serverReadTimeout,
+		WriteTimeout: serverWriteTimeout,
+		IdleTimeout:  serverIdleTimeout,
 	}
 }
 
@@ -154,7 +168,8 @@ func (a *App) Run(ctx context.Context) error {
 	if a.metricsServer != nil {
 		go func() {
 			slog.Info("Starting metrics server", "port", a.cfg.PrometheusExporterPort)
-			if err := a.metricsServer.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+
+			if err := a.metricsServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 				select {
 				case runErr <- fmt.Errorf("metrics server: %w", err):
 				default:
@@ -174,7 +189,8 @@ func (a *App) Run(ctx context.Context) error {
 
 	go func() {
 		slog.Info("Starting server", "port", a.cfg.Port)
-		if err := a.server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+
+		if err := a.server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 			select {
 			case runErr <- fmt.Errorf("server: %w", err):
 			default:
@@ -185,6 +201,7 @@ func (a *App) Run(ctx context.Context) error {
 	select {
 	case err := <-runErr:
 		a.cancel()
+
 		return err
 	case <-ctx.Done():
 		return nil
@@ -194,26 +211,34 @@ func (a *App) Run(ctx context.Context) error {
 // Shutdown stops the server, optional metrics server, MeterProvider, message publisher, and River in order. Call after Run returns.
 func (a *App) Shutdown(ctx context.Context) error {
 	defer a.message.Shutdown()
+
 	if a.metricsServer != nil {
-		shutdownCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+		shutdownCtx, cancel := context.WithTimeout(ctx, auxShutdownTimeout)
 		if err := a.metricsServer.Shutdown(shutdownCtx); err != nil {
 			slog.Warn("Metrics server shutdown", "error", err)
 		}
+
 		cancel()
 	}
+
 	if a.meterProvider != nil {
-		shutdownCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+		shutdownCtx, cancel := context.WithTimeout(ctx, auxShutdownTimeout)
 		if err := a.meterProvider.Shutdown(shutdownCtx); err != nil {
 			slog.Warn("MeterProvider shutdown", "error", err)
 		}
+
 		cancel()
 	}
+
 	if err := a.server.Shutdown(ctx); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		_ = a.river.Stop(ctx)
+
 		return fmt.Errorf("server shutdown: %w", err)
 	}
+
 	if err := a.river.Stop(ctx); err != nil {
 		return fmt.Errorf("river stop: %w", err)
 	}
+
 	return nil
 }

cmd/api/main.go

@@ -24,24 +24,30 @@ func main() {
 
 func run() int {
 	setupLogging(getLogLevelEnv())
+
 	cfg, err := config.Load()
 	if err != nil {
 		slog.Error("Failed to load configuration", "error", err)
+
 		return exitFailure
 	}
+
 	setupLogging(cfg.LogLevel)
 
 	ctx := context.Background()
+
 	db, err := database.NewPostgresPool(ctx, cfg.DatabaseURL)
 	if err != nil {
 		slog.Error("Failed to connect to database", "error", err)
+
 		return exitFailure
 	}
 	defer db.Close()
 
 	app, err := NewApp(cfg, db)
 	if err != nil {
 		slog.Error("Failed to create application", "error", err)
+
 		return exitFailure
 	}
 
@@ -50,34 +56,42 @@ func run() int {
 
 	if err := app.Run(sigCtx); err != nil {
 		slog.Error("Component failed, exiting", "error", err)
+
 		shutdownCtx, cancel := context.WithTimeout(context.Background(), cfg.ShutdownTimeout)
 		defer cancel()
+
 		if shutdownErr := app.Shutdown(shutdownCtx); shutdownErr != nil {
 			slog.Warn("Shutdown error", "error", shutdownErr)
 		}
+
 		return exitFailure
 	}
 
 	shutdownCtx, cancel := context.WithTimeout(context.Background(), cfg.ShutdownTimeout)
 	defer cancel()
+
 	if err := app.Shutdown(shutdownCtx); err != nil {
 		slog.Error("Shutdown failed", "error", err)
+
 		return exitFailure
 	}
 
 	slog.Info("Server stopped")
+
 	return exitSuccess
 }
 
 func getLogLevelEnv() string {
 	if v := os.Getenv("LOG_LEVEL"); v != "" {
 		return v
 	}
+
 	return "info"
 }
 
 func setupLogging(level string) {
 	var logLevel slog.Level
+
 	switch strings.ToLower(level) {
 	case "debug":
 		logLevel = slog.LevelDebug
@@ -90,6 +104,7 @@ func setupLogging(level string) {
 	default:
 		logLevel = slog.LevelInfo
 	}
+
 	opts := &slog.HandlerOptions{Level: logLevel}
 	handler := slog.NewTextHandler(os.Stdout, opts)
 	slog.SetDefault(slog.New(handler))