Merge pull request #29 from aaearon/feat/go-best-practices

feat: add golangci-lint config and apply Go best practices

Author: aaearon

.golangci.yml

@@ -0,0 +1,111 @@
+# golangci-lint v1 configuration for grant-cli
+# golangci-lint v1.64.8 | Go 1.25+
+# https://golangci-lint.run/usage/configuration/
+
+run:
+  timeout: 3m
+  tests: true
+
+linters:
+  disable-all: true
+  enable:
+    # --- Default linters ---
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    # --- Bug detection ---
+    - bodyclose     # unclosed HTTP response bodies
+    - errorlint     # error wrapping correctness (Go 1.13+)
+    - noctx         # HTTP requests without context.Context
+    # --- Security ---
+    - gosec         # security issue detection
+    # --- Style & naming ---
+    - errname       # sentinel error naming (ErrFoo)
+    - gocritic      # diagnostic/style/performance checks
+    - misspell      # common English typos
+    - revive        # configurable Go linter (golint replacement)
+    # --- Complexity ---
+    - gocognit      # cognitive complexity
+    # --- Performance ---
+    - perfsprint    # fmt.Sprintf replaceable with concatenation
+    - unconvert     # unnecessary type conversions
+    # --- Testing ---
+    - usetesting    # modern test helpers (t.Context, etc.)
+
+linters-settings:
+  gocognit:
+    min-complexity: 40
+
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - style
+      - performance
+    disabled-checks:
+      - hugeParam      # small structs in a CLI are fine
+      - rangeValCopy   # same: small value copies are fine
+
+  gosec:
+    excludes:
+      # G101 "hardcoded credentials" false-positives on test JWT tokens
+      - G101
+
+  revive:
+    severity: warning
+    enable-all-rules: false
+    rules:
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: empty-block
+      - name: error-naming
+      - name: error-return
+      - name: error-strings
+      - name: errorf
+      # exported: disabled because stuttering names (cache.CacheDir,
+      # config.ConfigDir, sca.SCAAccessService) are established API.
+      - name: exported
+        disabled: true
+      - name: increment-decrement
+      - name: indent-error-flow
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: superfluous-else
+      - name: time-naming
+      - name: unexported-return
+      - name: unreachable-code
+      - name: var-declaration
+      - name: var-naming
+      # unused-parameter: disabled because Cobra RunE signatures require
+      # func(cmd *cobra.Command, args []string) and test mocks implement
+      # interfaces with necessarily unused parameters.
+      - name: unused-parameter
+        disabled: true
+
+  misspell:
+    locale: US
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+
+  exclude-rules:
+    # Test files: relax security checks (G306 WriteFile perms in temp dirs)
+    - path: _test\.go
+      linters:
+        - gosec
+
+    # Test files: relax complexity checks (table-driven tests are long)
+    - path: _test\.go
+      linters:
+        - gocognit
+
+    # Test files: bodyclose on mock HTTP responses
+    - path: _test\.go
+      linters:
+        - bodyclose

.goreleaser.yaml

@@ -2,11 +2,14 @@ version: 2
 
 builds:
   - binary: grant
+    flags:
+      - -trimpath
+    mod_timestamp: "{{ .CommitTimestamp }}"
     ldflags:
       - -s -w
       - -X github.com/aaearon/grant-cli/cmd.version={{.Version}}
       - -X github.com/aaearon/grant-cli/cmd.commit={{.ShortCommit}}
-      - -X github.com/aaearon/grant-cli/cmd.buildDate={{.Date}}
+      - -X github.com/aaearon/grant-cli/cmd.buildDate={{.CommitDate}}
     goos:
       - linux
       - darwin

CLAUDE.md

@@ -94,15 +94,25 @@ Custom `SCAAccessService` follows SDK conventions:
 - Skill definition: `.claude/skills/grant-login/SKILL.md`
 - Requires `.env` at project root with `GRANT_PASSWORD` and `TOTP_SECRET`
 
+## Lint
+- Config: `.golangci.yml` (golangci-lint v1 format)
+- 19 linters enabled: defaults (errcheck, gosimple, govet, ineffassign, staticcheck, unused) + bodyclose, errorlint, noctx, gosec (G101 excluded), errname, gocritic, misspell, revive, gocognit (threshold 40), perfsprint, unconvert, usetesting
+- Test files excluded from gosec, gocognit, bodyclose
+- `revive/unused-parameter` and `revive/exported` disabled (Cobra signatures, established API names)
+- Use `errors.New` for static error strings (perfsprint enforced); `fmt.Errorf` only with `%` verbs
+- Use `t.Context()` instead of `context.Background()` in tests (usetesting enforced)
+
 ## Build
 ```bash
-make build              # Build binary with ldflags
+make build              # Build binary with -trimpath and ldflags
 make test               # Run unit tests
 make test-integration   # Run integration tests (builds binary)
 make test-all           # Run all tests
-make lint               # Run linter
+make lint               # Run linter (golangci-lint)
 make clean              # Clean build artifacts
 ```
+- `-trimpath` used in both `Makefile` and `.goreleaser.yaml` for reproducible builds
+- `.goreleaser.yaml` uses `CommitDate` (not build date) and `mod_timestamp` for reproducibility
 
 ## Git
 - Feature branches, conventional commits

Makefile

@@ -10,7 +10,7 @@ LDFLAGS := -s -w \
 .PHONY: build test test-race test-integration test-all test-coverage lint clean
 
 build:
-	go build -ldflags "$(LDFLAGS)" -o $(BINARY_NAME) .
+	go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY_NAME) .
 
 test:
 	go test ./... -v

cmd/configure.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -83,7 +84,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st
 	}
 
 	if strings.TrimSpace(username) == "" {
-		return fmt.Errorf("username is required")
+		return errors.New("username is required")
 	}
 
 	// Create SDK profile
@@ -144,7 +145,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st
 // validateTenantURL validates that the tenant URL is a valid HTTPS URL
 func validateTenantURL(tenantURL string) error {
 	if strings.TrimSpace(tenantURL) == "" {
-		return fmt.Errorf("invalid tenant URL: cannot be empty")
+		return errors.New("invalid tenant URL: cannot be empty")
 	}
 
 	u, err := url.Parse(tenantURL)
@@ -153,11 +154,11 @@ func validateTenantURL(tenantURL string) error {
 	}
 
 	if u.Scheme != "https" {
-		return fmt.Errorf("invalid tenant URL: must use HTTPS scheme")
+		return errors.New("invalid tenant URL: must use HTTPS scheme")
 	}
 
 	if u.Host == "" {
-		return fmt.Errorf("invalid tenant URL: must have a host")
+		return errors.New("invalid tenant URL: must have a host")
 	}
 
 	return nil

cmd/env.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/aaearon/grant-cli/internal/config"
@@ -92,7 +93,7 @@ func runEnvWithDeps(
 	}
 
 	if res.result.AccessCredentials == nil {
-		return fmt.Errorf("no credentials returned; grant env is only supported for AWS elevations")
+		return errors.New("no credentials returned; grant env is only supported for AWS elevations")
 	}
 
 	awsCreds, err := models.ParseAWSCredentials(*res.result.AccessCredentials)