feat: add code linter (#44)

Author: sgaunet

.github/workflows/linter.yml

@@ -0,0 +1,33 @@
+name: linter
+
+on:
+  pull_request:
+  push:
+
+permissions:
+  contents: write
+
+jobs:
+  linter:
+    runs-on: ubuntu-latest
+    steps:
+      -
+        name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: '>=1.24'
+      - name: Install devbox
+        uses: jetify-com/devbox-install-action@v0.12.0
+        with:
+          enable-cache: true
+          devbox-version: 0.14.0
+
+      - name: Install prerequisites
+        shell: /usr/bin/bash {0}
+        run: |
+          devbox install
+          devbox run linter

.golangci.yml

@@ -1,160 +1,18 @@
-linters-settings:
-  dupl:
-    threshold: 100
-  funlen:
-    lines: -1 # the number of lines (code + empty lines) is not a right metric and leads to code without empty line or one-liner.
-    statements: 50
-  goconst:
-    min-len: 2
-    min-occurrences: 3
-  gocritic:
-    enabled-tags:
-      - diagnostic
-      - experimental
-      - opinionated
-      - performance
-      - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - ifElseChain
-      - octalLiteral
-      - whyNoLint
-  gocyclo:
-    min-complexity: 15
-  gofmt:
-    rewrite-rules:
-      - pattern: 'interface{}'
-        replacement: 'any'
-  goimports:
-    local-prefixes: github.com/golangci/golangci-lint
-  gomnd:
-    # don't include the "operation" and "assign"
-    checks:
-      - argument
-      - case
-      - condition
-      - return
-    ignored-numbers:
-      - '0'
-      - '1'
-      - '2'
-      - '3'
-    ignored-functions:
-      - strings.SplitN
-
-  govet:
-    check-shadowing: true
-    settings:
-      printf:
-        funcs:
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
-  errorlint:
-    asserts: false
-  lll:
-    line-length: 140
-  misspell:
-    locale: US
-  nolintlint:
-    allow-unused: false # report any unused nolint directives
-    require-explanation: false # don't require an explanation for nolint directives
-    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
-  revive:
-    rules:
-      - name: unexported-return
-        disabled: true
-      - name: unused-parameter
+---
+version: "2"
+# Configure which files to skip during linting
+run:
+  tests: false
 
 linters:
-  disable-all: true
-  enable:
-    - bodyclose
-    # - depguard
-    - dogsled
-    - dupl
-    - errcheck
-    - errorlint
-    - exportloopref
-    - funlen
-    - gocheckcompilerdirectives
-    - gochecknoinits
-    - goconst
-    - gocritic
-    - gocyclo
-    - gofmt
-    - goimports
-    - gomnd
-    - goprintffuncname
-    - gosec
-    - gosimple
-    - govet
-    - ineffassign
-    - lll
-    - misspell
-    - nakedret
-    - noctx
-    - nolintlint
-    - revive
-    - staticcheck
-    - stylecheck
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - whitespace
-
-  # don't enable:
-  # - asciicheck
-  # - scopelint
-  # - gochecknoglobals
-  # - gocognit
-  # - godot
-  # - godox
-  # - goerr113
-  # - interfacer
-  # - maligned
-  # - nestif
-  # - prealloc
-  # - testpackage
-  # - wsl
-
-issues:
-  # Excluding configuration per-path, per-linter, per-text and per-source
-  exclude-rules:
-    - path: _test\.go
-      linters:
-        - gomnd
+  default: all
 
-    - path: pkg/golinters/errcheck.go
-      text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead"
-    - path: pkg/commands/run.go
-      text: "SA1019: lsc.Errcheck.Exclude is deprecated: use ExcludeFunctions instead"
-    - path: pkg/commands/run.go
-      text: "SA1019: e.cfg.Run.Deadline is deprecated: Deadline exists for historical compatibility and should not be used."
-
-    - path: pkg/golinters/gofumpt.go
-      text: "SA1019: settings.LangVersion is deprecated: use the global `run.go` instead."
-    - path: pkg/golinters/staticcheck_common.go
-      text: "SA1019: settings.GoVersion is deprecated: use the global `run.go` instead."
-    - path: pkg/lint/lintersdb/manager.go
-      text: "SA1019: (.+).(GoVersion|LangVersion) is deprecated: use the global `run.go` instead."
-    - path: pkg/golinters/unused.go
-      text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)"
-    - path: test/(fix|linters)_test.go
-      text: "string `gocritic.go` has 3 occurrences, make it a constant"
-
-    # Due to a change inside go-critic v0.10.0, some reports have been removed,
-    # but as we run analysis with the previous version of golangci-lint this leads to a paradoxical situation.
-    # This exclusion will be removed when the next version of golangci-lint (v1.56.0) will be released.
-    - path: pkg/golinters/nolintlint/nolintlint.go
-      text: "hugeParam: (i|b) is heavy \\(\\d+ bytes\\); consider passing it by pointer"
-
-run:
-  timeout: 5m
-  skip-dirs:
-    - test/testdata_etc # test files
-    - internal/cache # extracted from Go code
-    - internal/renameio # extracted from Go code
-    - internal/robustio # extracted from Go code
+  disable:
+    - wsl
+    - nlreturn
+    - depguard
+    - gochecknoinits
+    - gochecknoglobals
+    - forbidigo
+    - varnamelen
+    - exhaustruct

Taskfile.yml

@@ -13,6 +13,11 @@ tasks:
     cmds:
       - task -a
 
+  linter:
+    desc: "Run linter"
+    cmds:
+      - golangci-lint run --timeout 5m
+
   binary:
     desc: "Build binary"
     cmds:

gitlab-backup2s3.go

@@ -1,3 +1,4 @@
+// Package main is the entry point for gitlab-backup2s3.
 package main
 
 import (

pkg/app/app.go

@@ -1,3 +1,4 @@
+// Package app contains the main application logic for gitlab-backup2s3.
 package app
 
 import (
@@ -9,13 +10,13 @@ import (
 	"github.com/sgaunet/gitlab-backup2s3/pkg/logger"
 )
 
-// App is the main application
+// App is the main application.
 type App struct {
 	logger    logger.Logger
 	backupCmd []string
 }
 
-// NewApp creates a new App
+// NewApp creates a new App.
 func NewApp() *App {
 	return &App{
 		logger: logger.NoLogger(),
@@ -25,7 +26,7 @@ func NewApp() *App {
 	}
 }
 
-// SetLogger sets the logger
+// SetLogger sets the logger.
 func (a *App) SetLogger(log logger.Logger) {
 	if log == nil {
 		a.logger = logger.NoLogger()
@@ -34,13 +35,25 @@ func (a *App) SetLogger(log logger.Logger) {
 	a.logger = log
 }
 
-// SetBackupCmd sets the backup command
-// Use this method for testing purposes
+// SetBackupCmd sets the backup command.
+// Use this method for testing purposes.
 func (a *App) SetBackupCmd(backupCmd []string) {
 	a.backupCmd = backupCmd
 }
 
-// execCommand executes a command
+// Run executes the application.
+func (a *App) Run() error {
+	a.logger.Info("Execute gitlab-backup")
+	err := a.execCommand(a.backupCmd)
+	if err != nil {
+		a.logger.Error("Error executing gitlab-backup", slog.String("error", err.Error()))
+		return err
+	}
+	return nil
+}
+
+// execCommand executes a command.
+// It wraps all errors from external packages.
 func (a *App) execCommand(cmdToExec []string) error {
 	cmd := exec.Command(cmdToExec[0], cmdToExec[1:]...) // #nosec G204
 	stderr, err := cmd.StderrPipe()
@@ -71,16 +84,8 @@ func (a *App) execCommand(cmdToExec []string) error {
 		}
 	}()
 	err = cmd.Wait()
-	return err
-}
-
-// Run executes the application
-func (a *App) Run() error {
-	a.logger.Info("Execute gitlab-backup")
-	err := a.execCommand(a.backupCmd)
 	if err != nil {
-		a.logger.Error("Error executing gitlab-backup", slog.String("error", err.Error()))
-		return err
+		return fmt.Errorf("error waiting for command: %w", err)
 	}
 	return nil
 }

pkg/app/app_test.go

@@ -35,7 +35,7 @@ func TestSetLoggerNil(t *testing.T) {
 	if a == nil {
 		t.Fatalf("Expected non-nil app, got nil")
 	}
-	a.SetLogger(nil)
+	a.SetLogger(nil) // now accepts *logger.Logger as nil
 	a.SetBackupCmd([]string{"echo", "hello"})
 	err := a.Run()
 	if err != nil {

pkg/logger/logger.go

@@ -1,3 +1,4 @@
+// Package logger provides logging utilities for gitlab-backup2s3.
 package logger
 
 import (
@@ -6,6 +7,7 @@ import (
 	"os"
 )
 
+// Logger is the interface for logging in gitlab-backup2s3.
 type Logger interface {
 	Debug(msg string, args ...any)
 	Info(msg string, args ...any)
@@ -16,8 +18,8 @@ type Logger interface {
 // NewLogger creates a new logger
 // logLevel is the level of logging
 // Possible values of logLevel are: "debug", "info", "warn", "error"
-// Default value is "info"
-func NewLogger(logLevel string) Logger {
+// Default value is "info".
+func NewLogger(logLevel string) *slog.Logger {
 	var level slog.Level
 	switch logLevel {
 	case "debug":
@@ -39,8 +41,8 @@ func NewLogger(logLevel string) Logger {
 	return logger
 }
 
-// NoLogger creates a logger that does not log anything
-func NoLogger() Logger {
+// NoLogger creates a logger that does not log anything.
+func NoLogger() *slog.Logger {
 	noLogger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{
 		Level:     slog.LevelDebug,
 		AddSource: false,