chore: full relinting

* enabled linting check in CI (github action) Signed-off-by: Frederic BIDON <[email protected]>
spf13 · Oct 2, 2023 · c0dd564 · c0dd564
1 parent fa62798
commit c0dd564
Show file tree

Hide file tree

Showing 31 changed files with 440 additions and 268 deletions.
diff --git a/.github/workflows/01-golang-lint.yaml b/.github/workflows/01-golang-lint.yaml
@@ -0,0 +1,24 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+      - main
+  pull_request:
+permissions:
+  contents: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v4
+        with:
+          go-version: stable
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,39 @@
+linters-settings:
+  govet:
+    check-shadowing: true
+  maligned:
+    suggest-new: true
+  dupl:
+    threshold: 200
+  goconst:
+    min-len: 3
+    min-occurrences: 2
+  #forbidigo:
+  #  forbid:
+  #    - ^print.*$
+  #    - 'fmt\.Print.*'
+  gocognit:
+    min-complexity: 61 # This is a rather high value. We should gradually lower it to 30-40.
+
+linters:
+  enable:
+    - gofmt
+    - goimports
+    - bodyclose
+    - dupl
+    - gocognit
+    - gocritic
+    - goimports
+    - gosec
+    - nakedret
+     #- nolintlint
+    - revive
+    - stylecheck
+    - unconvert
+    - unparam
+  disable:
+    - forbidigo
+    - maligned
+    - lll
+    - gochecknoinits
+    - gochecknoglobals
diff --git a/bool_slice_test.go b/bool_slice_test.go
@@ -42,10 +42,11 @@ func TestBS(t *testing.T) {
 
 	vals := []string{"1", "F", "TRUE", "0"}
 	arg := fmt.Sprintf("--bs=%s", strings.Join(vals, ","))
-	err := f.Parse([]string{arg})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{arg})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range bs {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {
@@ -55,10 +56,12 @@ func TestBS(t *testing.T) {
 			t.Fatalf("expected is[%d] to be %s but got: %t", i, vals[i], v)
 		}
 	}
-	getBS, err := f.GetBoolSlice("bs")
-	if err != nil {
-		t.Fatalf("got error: %v", err)
+
+	getBS, erb := f.GetBoolSlice("bs")
+	if erb != nil {
+		t.Fatalf("got error: %v", erb)
 	}
+
 	for i, v := range getBS {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {
@@ -76,10 +79,11 @@ func TestBSDefault(t *testing.T) {
 
 	vals := []string{"false", "T"}
 
-	err := f.Parse([]string{})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range bs {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {
@@ -90,10 +94,11 @@ func TestBSDefault(t *testing.T) {
 		}
 	}
 
-	getBS, err := f.GetBoolSlice("bs")
-	if err != nil {
-		t.Fatal("got an error from GetBoolSlice():", err)
+	getBS, erb := f.GetBoolSlice("bs")
+	if erb != nil {
+		t.Fatal("got an error from GetBoolSlice():", erb)
 	}
+
 	for i, v := range getBS {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {
@@ -111,10 +116,11 @@ func TestBSWithDefault(t *testing.T) {
 
 	vals := []string{"FALSE", "1"}
 	arg := fmt.Sprintf("--bs=%s", strings.Join(vals, ","))
-	err := f.Parse([]string{arg})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{arg})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range bs {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {
@@ -125,10 +131,11 @@ func TestBSWithDefault(t *testing.T) {
 		}
 	}
 
-	getBS, err := f.GetBoolSlice("bs")
-	if err != nil {
-		t.Fatal("got an error from GetBoolSlice():", err)
+	getBS, erb := f.GetBoolSlice("bs")
+	if erb != nil {
+		t.Fatal("got an error from GetBoolSlice():", erb)
 	}
+
 	for i, v := range getBS {
 		b, err := strconv.ParseBool(vals[i])
 		if err != nil {

diff --git a/bool_test.go b/bool_test.go
@@ -26,7 +26,7 @@ func (v *triStateValue) IsBoolFlag() bool {
 }
 
 func (v *triStateValue) Get() interface{} {
-	return triStateValue(*v)
+	return *v
 }
 
 func (v *triStateValue) Set(s string) error {

diff --git a/bytes_test.go b/bytes_test.go
@@ -52,14 +52,15 @@ func TestBytesHex(t *testing.T) {
 		for _, arg := range args {
 			err := f.Parse([]string{arg})
 
-			if err != nil && tc.success == true {
+			switch {
+			case err != nil && tc.success:
 				t.Errorf("expected success, got %q", err)
 				continue
-			} else if err == nil && tc.success == false {
+			case err == nil && !tc.success:
 				// bytesHex, err := f.GetBytesHex("bytes")
 				t.Errorf("expected failure while processing %q", tc.input)
 				continue
-			} else if tc.success {
+			case tc.success:
 				bytesHex, err := f.GetBytesHex("bytes")
 				if err != nil {
 					t.Errorf("Got error trying to fetch the 'bytes' flag: %v", err)
@@ -113,14 +114,15 @@ func TestBytesBase64(t *testing.T) {
 		for _, arg := range args {
 			err := f.Parse([]string{arg})
 
-			if err != nil && tc.success == true {
+			switch {
+			case err != nil && tc.success:
 				t.Errorf("expected success, got %q", err)
 				continue
-			} else if err == nil && tc.success == false {
+			case err == nil && !tc.success:
 				// bytesBase64, err := f.GetBytesBase64("bytes")
 				t.Errorf("expected failure while processing %q", tc.input)
 				continue
-			} else if tc.success {
+			case tc.success:
 				bytesBase64, err := f.GetBytesBase64("bytes")
 				if err != nil {
 					t.Errorf("Got error trying to fetch the 'bytes' flag: %v", err)

diff --git a/count.go b/count.go
@@ -13,7 +13,7 @@ func newCountValue(val int, p *int) *countValue {
 func (i *countValue) Set(s string) error {
 	// "+1" means that no specific value was passed, so increment
 	if s == "+1" {
-		*i = countValue(*i + 1)
+		*i++
 		return nil
 	}
 	v, err := strconv.ParseInt(s, 0, 0)

diff --git a/count_test.go b/count_test.go
@@ -37,13 +37,14 @@ func TestCount(t *testing.T) {
 		tc := &testCases[i]
 
 		err := f.Parse(tc.input)
-		if err != nil && tc.success == true {
-			t.Errorf("expected success, got %q", err)
+		switch {
+		case err != nil && tc.success:
+			t.Errorf("expected success with %q, got %q", tc.input, err)
 			continue
-		} else if err == nil && tc.success == false {
-			t.Errorf("expected failure, got success")
+		case err == nil && !tc.success:
+			t.Errorf("expected failure with %q, got success", tc.input)
 			continue
-		} else if tc.success {
+		case tc.success:
 			c, err := f.GetCount("verbose")
 			if err != nil {
 				t.Errorf("Got error trying to fetch the counter flag")

diff --git a/duration_slice_test.go b/duration_slice_test.go
@@ -46,10 +46,11 @@ func TestDS(t *testing.T) {
 
 	vals := []string{"1ns", "2ms", "3m", "4h"}
 	arg := fmt.Sprintf("--ds=%s", strings.Join(vals, ","))
-	err := f.Parse([]string{arg})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{arg})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range ds {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -59,10 +60,12 @@ func TestDS(t *testing.T) {
 			t.Fatalf("expected ds[%d] to be %s but got: %d", i, vals[i], v)
 		}
 	}
-	getDS, err := f.GetDurationSlice("ds")
-	if err != nil {
-		t.Fatalf("got error: %v", err)
+
+	getDS, erd := f.GetDurationSlice("ds")
+	if erd != nil {
+		t.Fatalf("got error: %v", erd)
 	}
+
 	for i, v := range getDS {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -80,10 +83,11 @@ func TestDSDefault(t *testing.T) {
 
 	vals := []string{"0s", "1ns"}
 
-	err := f.Parse([]string{})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range ds {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -94,10 +98,11 @@ func TestDSDefault(t *testing.T) {
 		}
 	}
 
-	getDS, err := f.GetDurationSlice("ds")
-	if err != nil {
-		t.Fatal("got an error from GetDurationSlice():", err)
+	getDS, erd := f.GetDurationSlice("ds")
+	if erd != nil {
+		t.Fatal("got an error from GetDurationSlice():", erd)
 	}
+
 	for i, v := range getDS {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -115,10 +120,11 @@ func TestDSWithDefault(t *testing.T) {
 
 	vals := []string{"1ns", "2ns"}
 	arg := fmt.Sprintf("--ds=%s", strings.Join(vals, ","))
-	err := f.Parse([]string{arg})
-	if err != nil {
-		t.Fatal("expected no error; got", err)
+	erp := f.Parse([]string{arg})
+	if erp != nil {
+		t.Fatal("expected no error; got", erp)
 	}
+
 	for i, v := range ds {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -129,10 +135,11 @@ func TestDSWithDefault(t *testing.T) {
 		}
 	}
 
-	getDS, err := f.GetDurationSlice("ds")
-	if err != nil {
-		t.Fatal("got an error from GetDurationSlice():", err)
+	getDS, erd := f.GetDurationSlice("ds")
+	if erd != nil {
+		t.Fatal("got an error from GetDurationSlice():", erd)
 	}
+
 	for i, v := range getDS {
 		d, err := time.ParseDuration(vals[i])
 		if err != nil {
@@ -180,6 +187,7 @@ func TestDSCalledTwice(t *testing.T) {
 	if err != nil {
 		t.Fatal("expected no error; got", err)
 	}
+
 	for i, v := range ds {
 		if expected[i] != v {
 			t.Fatalf("expected ds[%d] to be %d but got: %d", i, expected[i], v)