Fix all golangci-lint issues across the codebase (#32)

* Fix all golangci-lint issues across the codebase

Resolve 77 lint issues reported by golangci-lint with gocritic, gocognit,
gocyclo, maintidx, dupl, mnd, unparam, ireturn, goconst, and errcheck
enabled. Net reduction of ~175 lines through shared helpers and
deduplication.

* Suppress staticcheck SA1019 for intentional deprecated field usage

The Storage.Path field is deprecated but still read for backwards
compatibility with existing configs that haven't migrated to the URL field.

Author: andrew

cmd/proxy/main.go

@@ -103,6 +103,8 @@ import (
 	"github.com/git-pkgs/proxy/internal/server"
 )
 
+const defaultTopN = 10
+
 var (
 	// Version is set at build time.
 	Version = "dev"
@@ -211,7 +213,7 @@ func runServe() {
 		cfg.Storage.URL = *storageURL
 	}
 	if *storagePath != "" {
-		cfg.Storage.Path = *storagePath
+		cfg.Storage.Path = *storagePath //nolint:staticcheck // backwards compat
 	}
 	if *databaseDriver != "" {
 		cfg.Database.Driver = *databaseDriver
@@ -247,7 +249,6 @@ func runServe() {
 
 	// Handle shutdown signals
 	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
 
 	go func() {
 		sigCh := make(chan os.Signal, 1)
@@ -266,10 +267,12 @@ func runServe() {
 	// Wait for shutdown or error
 	select {
 	case <-ctx.Done():
+		cancel()
 		if err := srv.Shutdown(context.Background()); err != nil {
 			logger.Error("shutdown error", "error", err)
 		}
 	case err := <-errCh:
+		cancel()
 		if err != nil {
 			logger.Error("server error", "error", err)
 			os.Exit(1)
@@ -283,8 +286,8 @@ func runStats() {
 	databasePath := fs.String("database-path", "./cache/proxy.db", "Path to SQLite database file")
 	databaseURL := fs.String("database-url", "", "PostgreSQL connection URL")
 	asJSON := fs.Bool("json", false, "Output as JSON")
-	popular := fs.Int("popular", 10, "Show top N most popular packages")
-	recent := fs.Int("recent", 10, "Show N recently cached packages")
+	popular := fs.Int("popular", defaultTopN, "Show top N most popular packages")
+	recent := fs.Int("recent", defaultTopN, "Show N recently cached packages")
 
 	fs.Usage = func() {
 		fmt.Fprintf(os.Stderr, "git-pkgs proxy - Show cache statistics\n\n")
@@ -330,32 +333,37 @@ func runStats() {
 		fmt.Fprintf(os.Stderr, "error opening database: %v\n", err)
 		os.Exit(1)
 	}
+
+	if err := printStats(db, *popular, *recent, *asJSON); err != nil {
+		fmt.Fprintf(os.Stderr, "%v\n", err)
+		os.Exit(1)
+	}
+}
+
+func printStats(db *database.DB, popular, recent int, asJSON bool) error {
 	defer func() { _ = db.Close() }()
 
-	// Get stats
 	stats, err := db.GetCacheStats()
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error getting stats: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("error getting stats: %w", err)
 	}
 
-	popularPkgs, err := db.GetMostPopularPackages(*popular)
+	popularPkgs, err := db.GetMostPopularPackages(popular)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error getting popular packages: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("error getting popular packages: %w", err)
 	}
 
-	recentPkgs, err := db.GetRecentlyCachedPackages(*recent)
+	recentPkgs, err := db.GetRecentlyCachedPackages(recent)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error getting recent packages: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("error getting recent packages: %w", err)
 	}
 
-	if *asJSON {
+	if asJSON {
 		outputJSON(stats, popularPkgs, recentPkgs)
 	} else {
 		outputText(stats, popularPkgs, recentPkgs)
 	}
+	return nil
 }
 
 type jsonOutput struct {

internal/config/config.go

@@ -109,8 +109,9 @@ type StorageConfig struct {
 	URL string `json:"url" yaml:"url"`
 
 	// Path is the directory where cached artifacts are stored.
-	// Deprecated: Use URL with file:// scheme instead.
 	// If URL is empty, this is used as file://{Path}.
+	//
+	// Deprecated: Use URL with file:// scheme instead.
 	Path string `json:"path" yaml:"path"`
 
 	// MaxSize is the maximum cache size (e.g., "10GB", "500MB").

internal/config/config_test.go

@@ -6,6 +6,12 @@ import (
 	"testing"
 )
 
+const (
+	testDriverPostgres = "postgres"
+	testInvalid        = "invalid"
+	testLevelDebug     = "debug"
+)
+
 func TestDefault(t *testing.T) {
 	cfg := Default()
 
@@ -63,27 +69,27 @@ func TestValidate(t *testing.T) {
 		},
 		{
 			name:    "postgres without url",
-			modify:  func(c *Config) { c.Database.Driver = "postgres"; c.Database.URL = "" },
+			modify:  func(c *Config) { c.Database.Driver = testDriverPostgres; c.Database.URL = "" },
 			wantErr: true,
 		},
 		{
 			name:    "postgres with url",
-			modify:  func(c *Config) { c.Database.Driver = "postgres"; c.Database.URL = "postgres://localhost/test" },
+			modify:  func(c *Config) { c.Database.Driver = testDriverPostgres; c.Database.URL = "postgres://localhost/test" },
 			wantErr: false,
 		},
 		{
 			name:    "invalid log level",
-			modify:  func(c *Config) { c.Log.Level = "invalid" },
+			modify:  func(c *Config) { c.Log.Level = testInvalid },
 			wantErr: true,
 		},
 		{
 			name:    "invalid log format",
-			modify:  func(c *Config) { c.Log.Format = "invalid" },
+			modify:  func(c *Config) { c.Log.Format = testInvalid },
 			wantErr: true,
 		},
 		{
 			name:    "invalid max size",
-			modify:  func(c *Config) { c.Storage.MaxSize = "invalid" },
+			modify:  func(c *Config) { c.Storage.MaxSize = testInvalid },
 			wantErr: true,
 		},
 		{
@@ -176,8 +182,8 @@ log:
 	if cfg.Storage.MaxSize != "5GB" {
 		t.Errorf("Storage.MaxSize = %q, want %q", cfg.Storage.MaxSize, "5GB")
 	}
-	if cfg.Log.Level != "debug" {
-		t.Errorf("Log.Level = %q, want %q", cfg.Log.Level, "debug")
+	if cfg.Log.Level != testLevelDebug {
+		t.Errorf("Log.Level = %q, want %q", cfg.Log.Level, testLevelDebug)
 	}
 	if cfg.Log.Format != "json" {
 		t.Errorf("Log.Format = %q, want %q", cfg.Log.Format, "json")
@@ -215,7 +221,7 @@ func TestLoadFromEnv(t *testing.T) {
 	t.Setenv("PROXY_LISTEN", ":9000")
 	t.Setenv("PROXY_BASE_URL", "https://env.example.com")
 	t.Setenv("PROXY_STORAGE_PATH", "/env/cache")
-	t.Setenv("PROXY_LOG_LEVEL", "debug")
+	t.Setenv("PROXY_LOG_LEVEL", testLevelDebug)
 
 	cfg.LoadFromEnv()
 
@@ -228,8 +234,8 @@ func TestLoadFromEnv(t *testing.T) {
 	if cfg.Storage.Path != "/env/cache" {
 		t.Errorf("Storage.Path = %q, want %q", cfg.Storage.Path, "/env/cache")
 	}
-	if cfg.Log.Level != "debug" {
-		t.Errorf("Log.Level = %q, want %q", cfg.Log.Level, "debug")
+	if cfg.Log.Level != testLevelDebug {
+		t.Errorf("Log.Level = %q, want %q", cfg.Log.Level, testLevelDebug)
 	}
 }
 

internal/cooldown/cooldown.go

@@ -7,6 +7,8 @@ import (
 	"time"
 )
 
+const hoursPerDay = 24
+
 // Config holds cooldown settings for version filtering.
 // Cooldown hides package versions published too recently, giving the community
 // time to spot malicious releases before they're pulled into projects.
@@ -112,7 +114,7 @@ func ParseDuration(s string) (time.Duration, error) {
 		if err != nil {
 			return 0, fmt.Errorf("invalid duration %q: %w", s, err)
 		}
-		return time.Duration(days * float64(24*time.Hour)), nil
+		return time.Duration(days * float64(hoursPerDay*time.Hour)), nil
 	}
 
 	d, err := time.ParseDuration(s)

internal/database/database.go

@@ -12,6 +12,8 @@ import (
 
 const SchemaVersion = 1
 
+const dirPermissions = 0755
+
 type Dialect string
 
 const (
@@ -56,7 +58,7 @@ func Create(path string) (*DB, error) {
 
 func Open(path string) (*DB, error) {
 	if dir := filepath.Dir(path); dir != "." && dir != "/" {
-		if err := os.MkdirAll(dir, 0755); err != nil {
+		if err := os.MkdirAll(dir, dirPermissions); err != nil {
 			return nil, fmt.Errorf("creating database directory: %w", err)
 		}
 	}