Linters (#397)

* fix some lint warnings from main package * add new lint config * add golangci-lint action on MR * fix build on linux * fix linting warnings on Windows * fixed thelper linter * fix golantci-lint version tag * add predeclared linter * add tests on remote package
creativeprojects · Aug 18, 2024 · ccccfa2 · ccccfa2
1 parent 0ad1b68
commit ccccfa2
Show file tree

Hide file tree

Showing 93 changed files with 492 additions and 294 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -39,7 +39,12 @@ jobs:
       - name: Set up Node.js LTS for running JSON schema tests (using ajv)
         uses: actions/setup-node@v4
         with:
-          node-version: 18
+          node-version: "18"
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.60
 
       - name: Test
         run: make test-ci

diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,27 @@
+linters:
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - contextcheck
+    - errname
+    - gocheckcompilerdirectives
+    - gosec
+    # - maintidx
+    - nilnil
+    - noctx
+    - nolintlint
+    - predeclared
+    - reassign
+    - sloglint
+    - spancheck
+    - unconvert
+    - unparam
+    - usestdlibvars
+
+linters-settings:
+  gosec:
+    excludes:
+      - G204 # Audit the use of command execution
+      - G404 # Insecure random number source (rand)
diff --git a/calendar/event_parse.go b/calendar/event_parse.go
@@ -29,13 +29,13 @@ var (
 		expr        *regexp.Regexp
 		parseValues []parseFunc
 	}{
-		{regexpFullPattern, []parseFunc{parseWeekday(1), parseYear(2), parseMonth(3), parseDay(4), parseHour(5), parseMinute(6), parseSecond(7)}},
+		{regexpFullPattern, []parseFunc{parseWeekday(), parseYear(2), parseMonth(3), parseDay(4), parseHour(5), parseMinute(6), parseSecond(7)}},
 		{regexpDatePattern, []parseFunc{parseYear(1), parseMonth(2), parseDay(3), setMidnight()}},
 		{regexpTimePattern, []parseFunc{parseHour(1), parseMinute(2), parseSecond(3)}},
 		{regexpDateTimePattern, []parseFunc{parseYear(1), parseMonth(2), parseDay(3), parseHour(4), parseMinute(5), parseSecond(6)}},
-		{regexpWeekdayPattern, []parseFunc{parseWeekday(1), setMidnight()}},
-		{regexpWeekdayDatePattern, []parseFunc{parseWeekday(1), parseYear(2), parseMonth(3), parseDay(4), setMidnight()}},
-		{regexpWeekdayTimePattern, []parseFunc{parseWeekday(1), parseHour(2), parseMinute(3), parseSecond(4)}},
+		{regexpWeekdayPattern, []parseFunc{parseWeekday(), setMidnight()}},
+		{regexpWeekdayDatePattern, []parseFunc{parseWeekday(), parseYear(2), parseMonth(3), parseDay(4), setMidnight()}},
+		{regexpWeekdayTimePattern, []parseFunc{parseWeekday(), parseHour(2), parseMinute(3), parseSecond(4)}},
 	}
 )
 
@@ -98,7 +98,10 @@ func parseSecond(index int) parseFunc {
 	return func(e *Event, match []string) error {
 		// second can be empty => it means zero
 		if match[index] == "" {
-			e.Second.AddValue(0)
+			err := e.Second.AddValue(0)
+			if err != nil {
+				return fmt.Errorf("cannot parse second: %w", err)
+			}
 			return nil
 		}
 		err := e.Second.Parse(strings.Trim(match[index], ":"))
@@ -111,16 +114,25 @@ func parseSecond(index int) parseFunc {
 
 func setMidnight() parseFunc {
 	return func(e *Event, match []string) error {
-		e.Hour.AddValue(0)
-		e.Minute.AddValue(0)
-		e.Second.AddValue(0)
+		err := e.Hour.AddValue(0)
+		if err != nil {
+			return err
+		}
+		err = e.Minute.AddValue(0)
+		if err != nil {
+			return err
+		}
+		err = e.Second.AddValue(0)
+		if err != nil {
+			return err
+		}
 		return nil
 	}
 }
 
-func parseWeekday(index int) parseFunc {
+func parseWeekday() parseFunc {
 	return func(e *Event, match []string) error {
-		weekdays := strings.ToLower(match[index])
+		weekdays := strings.ToLower(match[1]) // weekday is always the first match
 		for dayIndex, day := range longWeekDay {
 			weekdays = strings.ReplaceAll(weekdays, day, fmt.Sprintf("%02d", dayIndex))
 		}

diff --git a/calendar/value.go b/calendar/value.go
@@ -42,40 +42,40 @@ type Value struct {
 }
 
 // NewValue creates a new value
-func NewValue(min, max int) *Value {
+func NewValue(minValue, maxValue int) *Value {
 	return &Value{
-		minRange: min,
-		maxRange: max,
+		minRange: minValue,
+		maxRange: maxValue,
 	}
 }
 
 // NewValueFromType creates a new value from a predefined type
 func NewValueFromType(t TypeValue) *Value {
-	min, max := 0, 0
+	minValue, maxValue := 0, 0
 	switch t {
 	case TypeWeekDay:
-		min = minDay
-		max = maxDay - 1
+		minValue = minDay
+		maxValue = maxDay - 1
 	case TypeYear:
-		min = 2000
-		max = 2200
+		minValue = 2000
+		maxValue = 2200
 	case TypeMonth:
-		min = 1
-		max = 12
+		minValue = 1
+		maxValue = 12
 	case TypeDay:
-		min = 1
-		max = 31
+		minValue = 1
+		maxValue = 31
 	case TypeHour:
-		max = 23
+		maxValue = 23
 	case TypeMinute:
-		max = 59
+		maxValue = 59
 	case TypeSecond:
-		max = 59
+		maxValue = 59
 	}
 	return &Value{
 		definedType: t,
-		minRange:    min,
-		maxRange:    max,
+		minRange:    minValue,
+		maxRange:    maxValue,
 	}
 }
 
@@ -157,8 +157,8 @@ func (v *Value) AddValue(value int) error {
 }
 
 // MustAddRange adds a range of values from min to max and panics if an error occurs
-func (v *Value) MustAddRange(min int, max int) {
-	err := v.AddRange(min, max)
+func (v *Value) MustAddRange(minValue int, maxValue int) {
+	err := v.AddRange(minValue, maxValue)
 	if err != nil {
 		panic(err)
 	}

diff --git a/calendar/value_test.go b/calendar/value_test.go
@@ -5,12 +5,13 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestEmptyValue(t *testing.T) {
-	min := 10
-	max := 20
-	value := NewValue(min, max)
+	minValue := 10
+	maxValue := 20
+	value := NewValue(minValue, maxValue)
 	assert.False(t, value.HasValue())
 	assert.False(t, value.HasSingleValue())
 	assert.False(t, value.HasRange())
@@ -22,11 +23,12 @@ func TestEmptyValue(t *testing.T) {
 }
 
 func TestSingleValue(t *testing.T) {
-	min := 0
-	max := 10
+	minValue := 0
+	maxValue := 10
 	entry := 5
-	value := NewValue(min, max)
-	value.AddValue(entry)
+	value := NewValue(minValue, maxValue)
+	err := value.AddValue(entry)
+	require.NoError(t, err)
 	assert.True(t, value.HasValue())
 	assert.True(t, value.HasSingleValue())
 	assert.False(t, value.HasRange())
@@ -38,30 +40,32 @@ func TestSingleValue(t *testing.T) {
 }
 
 func TestSimpleRangeValue(t *testing.T) {
-	min := 1
-	max := 9
-	entries := []int{min, max}
-	value := NewValue(min, max)
+	minValue := 1
+	maxValue := 9
+	entries := []int{minValue, maxValue}
+	value := NewValue(minValue, maxValue)
 	for _, entry := range entries {
-		value.AddValue(entry)
+		err := value.AddValue(entry)
+		require.NoError(t, err)
 	}
 	assert.True(t, value.HasValue())
 	assert.False(t, value.HasSingleValue())
 	assert.True(t, value.HasRange())
 	assert.False(t, value.HasContiguousRange())
 	assert.False(t, value.HasLongContiguousRange())
 	assert.ElementsMatch(t, entries, value.GetRangeValues())
-	assert.ElementsMatch(t, []Range{{min, min}, {max, max}}, value.GetRanges())
-	assert.Equal(t, fmt.Sprintf("%02d,%02d", min, max), value.String())
+	assert.ElementsMatch(t, []Range{{minValue, minValue}, {maxValue, maxValue}}, value.GetRanges())
+	assert.Equal(t, fmt.Sprintf("%02d,%02d", minValue, maxValue), value.String())
 }
 
 func TestContiguousRangeValue(t *testing.T) {
-	min := 10
-	max := 20
+	minValue := 10
+	maxValue := 20
 	entries := []int{11, 12, 14}
-	value := NewValue(min, max)
+	value := NewValue(minValue, maxValue)
 	for _, entry := range entries {
-		value.AddValue(entry)
+		err := value.AddValue(entry)
+		require.NoError(t, err)
 	}
 	assert.True(t, value.HasValue())
 	assert.False(t, value.HasSingleValue())
@@ -74,12 +78,13 @@ func TestContiguousRangeValue(t *testing.T) {
 }
 
 func TestComplexContiguousRanges(t *testing.T) {
-	min := 10
-	max := 20
+	minValue := 10
+	maxValue := 20
 	entries := []int{10, 11, 14, 15, 16, 19, 20}
-	value := NewValue(min, max)
+	value := NewValue(minValue, maxValue)
 	for _, entry := range entries {
-		value.AddValue(entry)
+		err := value.AddValue(entry)
+		require.NoError(t, err)
 	}
 	assert.True(t, value.HasValue())
 	assert.False(t, value.HasSingleValue())
@@ -92,12 +97,15 @@ func TestComplexContiguousRanges(t *testing.T) {
 }
 
 func TestAddRanges(t *testing.T) {
-	min := 10
-	max := 20
+	minValue := 10
+	maxValue := 20
 	entries := []int{11, 12, 15}
-	value := NewValue(min, max)
-	value.AddRange(12, 11) // wrong order on purpose
-	value.AddRange(15, 15)
+	value := NewValue(minValue, maxValue)
+	err := value.AddRange(12, 11) // wrong order on purpose
+	require.NoError(t, err)
+	err = value.AddRange(15, 15)
+	require.NoError(t, err)
+
 	assert.True(t, value.HasValue())
 	assert.False(t, value.HasSingleValue())
 	assert.True(t, value.HasRange())
@@ -109,24 +117,24 @@ func TestAddRanges(t *testing.T) {
 }
 
 func TestAddValueOutOfRange(t *testing.T) {
-	min := 10
-	max := 20
-	entries := []int{min - 1, max + 1}
+	minValue := 10
+	maxValue := 20
+	entries := []int{minValue - 1, maxValue + 1}
 	for _, entry := range entries {
-		value := NewValue(min, max)
+		value := NewValue(minValue, maxValue)
 		assert.Panics(t, func() {
 			value.MustAddValue(entry)
 		})
 	}
 }
 
 func TestAddRangeValuesOutOfRange(t *testing.T) {
-	min := 10
-	max := 20
+	minValue := 10
+	maxValue := 20
 
-	value := NewValue(min, max)
+	value := NewValue(minValue, maxValue)
 	assert.Panics(t, func() {
-		value.MustAddRange(min-1, max-1)
+		value.MustAddRange(minValue-1, maxValue-1)
 	})
 
 }

diff --git a/commands.go b/commands.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"crypto/rand"
 	"encoding/base64"
 	"errors"
@@ -10,6 +11,7 @@ import (
 	"slices"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/creativeprojects/clog"
 	"github.com/creativeprojects/resticprofile/config"
@@ -167,7 +169,7 @@ func completeCommand(output io.Writer, ctx commandContext) error {
 
 	// Parse requester as first argument. Format "[kind]:v[version]", e.g. "bash:v1"
 	if len(args) > 0 {
-		matcher := regexp.MustCompile("^(bash|zsh):v(\\d+)$")
+		matcher := regexp.MustCompile(`^(bash|zsh):v(\d+)$`)
 		if matches := matcher.FindStringSubmatch(args[0]); matches != nil {
 			requester = matches[1]
 			if v, err := strconv.Atoi(matches[2]); err == nil {
@@ -570,7 +572,7 @@ func testElevationCommand(_ io.Writer, ctx commandContext) error {
 		return nil
 	}
 
-	return elevated(ctx.flags)
+	return elevated()
 }
 
 func retryElevated(err error, flags commandLineFlags) error {
@@ -580,7 +582,7 @@ func retryElevated(err error, flags commandLineFlags) error {
 	// maybe can find a better way than searching for the word "denied"?
 	if platform.IsWindows() && !flags.isChild && strings.Contains(err.Error(), "denied") {
 		clog.Info("restarting resticprofile in elevated mode...")
-		err := elevated(flags)
+		err := elevated()
 		if err != nil {
 			return err
 		}
@@ -589,7 +591,7 @@ func retryElevated(err error, flags commandLineFlags) error {
 	return err
 }
 
-func elevated(flags commandLineFlags) error {
+func elevated() error {
 	if !platform.IsWindows() {
 		return errors.New("only available on Windows platform")
 	}
@@ -601,7 +603,9 @@ func elevated(flags commandLineFlags) error {
 	}
 	err = win.RunElevated(remote.GetPort())
 	if err != nil {
-		remote.StopServer()
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		remote.StopServer(ctx)
 		return err
 	}
 

diff --git a/commands_display.go b/commands_display.go
@@ -117,7 +117,7 @@ func displayOwnCommandHelp(output io.Writer, commandName string, ctx commandCont
 	out("\t%s %s\n\n", getCommonUsageHelpLine(command.name, command.needConfiguration && !command.noProfile), commandFlags)
 
 	var flags = make([]string, 0, len(command.flags))
-	for f, _ := range command.flags {
+	for f := range command.flags {
 		flags = append(flags, f)
 	}
 	if len(flags) > 0 {