build: Add golangci-lint checks

Implement make validate to run golangci-lint checks and call it from
the Test GitHub workflow.

An initial set of rules have been enabled, and the violations fixed.
Over time a larger number of rules will be enabled.

Signed-off-by: Paulo Gomes <[email protected]>

Author: pjbgf

.github/workflows/test.yml

@@ -21,5 +21,10 @@ jobs:
       uses: actions/setup-go@v5
       with:
         go-version: ${{ matrix.go-version }}
+
+    - name: Validate
+      if: matrix.platform == 'ubuntu-latest'
+      run: make validate
+
     - name: Test
       run: make test

.gitignore

@@ -1,4 +1,3 @@
 /coverage.txt
 /vendor
-Gopkg.lock
-Gopkg.toml
+/build/

.golangci.yaml

@@ -0,0 +1,57 @@
+linters:
+  disable-all: true
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - containedctx
+    - contextcheck
+    - decorder
+    - dogsled
+    - dupl
+    - dupword
+    - durationcheck
+    - errcheck
+    - errchkjson
+    - errname
+    - errorlint
+    - exhaustive
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecknoinits
+    - gochecksumtype
+    - goheader
+    - goimports
+    - gomodguard
+    - goprintffuncname
+    - gosimple
+    - gosmopolitan
+    - govet
+    - grouper
+    - importas
+    - ineffassign
+    - loggercheck
+    - makezero
+    - mirror
+    - misspell
+    - noctx
+    - nosprintfhostport
+    - promlinter
+    - reassign
+    - revive
+    - rowserrcheck
+    - sloglint
+    - spancheck
+    - sqlclosecheck
+    - tagalign
+    - tagliatelle
+    - tenv
+    - testableexamples
+    - typecheck
+    - unconvert
+    - unused
+    - usestdlibvars
+    - wastedassign
+    - whitespace
+    - zerologlint

Makefile

@@ -3,6 +3,15 @@ GOCMD = go
 GOTEST = $(GOCMD) test 
 WASIRUN_WRAPPER := $(CURDIR)/scripts/wasirun-wrapper
 
+GOLANGCI_VERSION ?= v1.63.4
+TOOLS_BIN := $(shell mkdir -p build/tools && realpath build/tools)
+
+GOLANGCI = $(TOOLS_BIN)/golangci-lint-$(GOLANGCI_VERSION)
+$(GOLANGCI):
+	rm -f $(TOOLS_BIN)/golangci-lint*
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/$(GOLANGCI_VERSION)/install.sh | sh -s -- -b $(TOOLS_BIN) $(GOLANGCI_VERSION)
+	mv $(TOOLS_BIN)/golangci-lint $(TOOLS_BIN)/golangci-lint-$(GOLANGCI_VERSION)
+
 .PHONY: test
 test:
 	$(GOTEST) -race ./...
@@ -16,3 +25,24 @@ wasitest: export GOARCH=wasm
 wasitest: export GOOS=wasip1
 wasitest:
 	$(GOTEST) -exec $(WASIRUN_WRAPPER) ./...
+
+validate: validate-lint validate-dirty ## Run validation checks.
+
+validate-lint: $(GOLANGCI)
+	$(GOLANGCI) run
+
+define go-install-tool
+@[ -f $(1) ] || { \
+set -e ;\
+echo "Downloading $(2)" ;\
+GOBIN=$(TOOLS_BIN) go install $(2) ;\
+}
+endef
+
+validate-dirty:
+ifneq ($(shell git status --porcelain --untracked-files=no),)
+	@echo worktree is dirty
+	@git --no-pager status
+	@git --no-pager diff
+	@exit 1
+endif

fs_test.go

@@ -3,7 +3,7 @@ package billy_test
 import (
 	"testing"
 
-	. "github.com/go-git/go-billy/v6"
+	. "github.com/go-git/go-billy/v6" //nolint
 	"github.com/go-git/go-billy/v6/internal/test"
 	"github.com/stretchr/testify/assert"
 )

helper/chroot/chroot.go

@@ -11,7 +11,7 @@ import (
 )
 
 // ChrootHelper is a helper to implement billy.Chroot.
-type ChrootHelper struct {
+type ChrootHelper struct { //nolint
 	underlying billy.Filesystem
 	base       string
 }

helper/chroot/chroot_test.go

@@ -8,14 +8,15 @@ import (
 	"github.com/go-git/go-billy/v6"
 	"github.com/go-git/go-billy/v6/internal/test"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCreate(t *testing.T) {
 	m := &test.BasicMock{}
 
 	fs := New(m, "/foo")
 	f, err := fs.Create("bar/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, f.Name(), filepath.Join("bar", "qux"))
 
 	assert.Len(t, m.CreateArgs, 1)
@@ -35,7 +36,7 @@ func TestLeadingPeriodsPathNotCrossedBoundary(t *testing.T) {
 
 	fs := New(m, "/foo")
 	f, err := fs.Create("..foo")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, f.Name(), "..foo")
 }
 
@@ -44,7 +45,7 @@ func TestOpen(t *testing.T) {
 
 	fs := New(m, "/foo")
 	f, err := fs.Open("bar/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, f.Name(), filepath.Join("bar", "qux"))
 
 	assert.Len(t, m.OpenArgs, 1)
@@ -56,7 +57,7 @@ func TestChroot(t *testing.T) {
 
 	fs, _ := New(m, "/foo").Chroot("baz")
 	f, err := fs.Open("bar/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, f.Name(), filepath.Join("bar", "qux"))
 
 	assert.Len(t, m.OpenArgs, 1)
@@ -84,7 +85,7 @@ func TestOpenFile(t *testing.T) {
 
 	fs := New(m, "/foo")
 	f, err := fs.OpenFile("bar/qux", 42, 0777)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, f.Name(), filepath.Join("bar", "qux"))
 
 	assert.Len(t, m.OpenFileArgs, 1)
@@ -104,7 +105,7 @@ func TestStat(t *testing.T) {
 
 	fs := New(m, "/foo")
 	_, err := fs.Stat("bar/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.StatArgs, 1)
 	assert.Equal(t, m.StatArgs[0], "/foo/bar/qux")
@@ -123,7 +124,7 @@ func TestRename(t *testing.T) {
 
 	fs := New(m, "/foo")
 	err := fs.Rename("bar/qux", "qux/bar")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.RenameArgs, 1)
 	assert.Equal(t, m.RenameArgs[0], [2]string{"/foo/bar/qux", "/foo/qux/bar"})
@@ -145,7 +146,7 @@ func TestRemove(t *testing.T) {
 
 	fs := New(m, "/foo")
 	err := fs.Remove("bar/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.RemoveArgs, 1)
 	assert.Equal(t, m.RemoveArgs[0], "/foo/bar/qux")
@@ -164,7 +165,7 @@ func TestTempFile(t *testing.T) {
 
 	fs := New(m, "/foo")
 	_, err := fs.TempFile("bar", "qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.TempFileArgs, 1)
 	assert.Equal(t, m.TempFileArgs[0], [2]string{"/foo/bar", "qux"})
@@ -191,7 +192,7 @@ func TestReadDir(t *testing.T) {
 
 	fs := New(m, "/foo")
 	_, err := fs.ReadDir("bar")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.ReadDirArgs, 1)
 	assert.Equal(t, m.ReadDirArgs[0], "/foo/bar")
@@ -218,7 +219,7 @@ func TestMkDirAll(t *testing.T) {
 
 	fs := New(m, "/foo")
 	err := fs.MkdirAll("bar", 0777)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.MkdirAllArgs, 1)
 	assert.Equal(t, m.MkdirAllArgs[0], [2]interface{}{"/foo/bar", os.FileMode(0777)})
@@ -245,7 +246,7 @@ func TestLstat(t *testing.T) {
 
 	fs := New(m, "/foo")
 	_, err := fs.Lstat("qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.LstatArgs, 1)
 	assert.Equal(t, m.LstatArgs[0], "/foo/qux")
@@ -272,7 +273,7 @@ func TestSymlink(t *testing.T) {
 
 	fs := New(m, "/foo")
 	err := fs.Symlink("../baz", "qux/bar")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.SymlinkArgs, 1)
 	assert.Equal(t, m.SymlinkArgs[0], [2]string{filepath.FromSlash("../baz"), "/foo/qux/bar"})
@@ -283,7 +284,7 @@ func TestSymlinkWithAbsoluteTarget(t *testing.T) {
 
 	fs := New(m, "/foo")
 	err := fs.Symlink("/bar", "qux/baz")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, m.SymlinkArgs, 1)
 	assert.Equal(t, m.SymlinkArgs[0], [2]string{filepath.FromSlash("/foo/bar"), "/foo/qux/baz"})
@@ -310,7 +311,7 @@ func TestReadlink(t *testing.T) {
 
 	fs := New(m, "/foo")
 	link, err := fs.Readlink("/qux")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, link, filepath.FromSlash("/qux"))
 
 	assert.Len(t, m.ReadlinkArgs, 1)
@@ -322,7 +323,7 @@ func TestReadlinkWithRelative(t *testing.T) {
 
 	fs := New(m, "/foo")
 	link, err := fs.Readlink("qux/bar")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, link, filepath.FromSlash("/qux/bar"))
 
 	assert.Len(t, m.ReadlinkArgs, 1)

helper/iofs/iofs_test.go

@@ -17,10 +17,6 @@ type errorList interface {
 	Unwrap() []error
 }
 
-type wrappedError interface {
-	Unwrap() error
-}
-
 // TestWithFSTest leverages the packaged Go fstest package, which seems comprehensive.
 func TestWithFSTest(t *testing.T) {
 	t.Parallel()
@@ -32,17 +28,17 @@ func TestWithFSTest(t *testing.T) {
 		"bar.txt":                       "goodbye, world",
 		filepath.Join("dir", "baz.txt"): "こんにちわ, world",
 	}
-	created_files := make([]string, 0, len(files))
+	createdFiles := make([]string, 0, len(files))
 	for filename, contents := range files {
 		makeFile(memfs, t, filename, contents)
-		created_files = append(created_files, filename)
+		createdFiles = append(createdFiles, filename)
 	}
 
 	if runtime.GOOS == "windows" {
 		t.Skip("fstest.TestFS is not yet windows path aware")
 	}
 
-	err := fstest.TestFS(iofs, created_files...)
+	err := fstest.TestFS(iofs, createdFiles...)
 	if err != nil {
 		checkFsTestError(t, err, files)
 	}
@@ -101,7 +97,6 @@ func checkFsTestError(t *testing.T, err error, files map[string]string) {
 	// has nicely-Joined wrapped errors.  Try that first.
 	if errs, ok := err.(errorList); ok {
 		for _, e := range errs.Unwrap() {
-
 			if strings.Contains(e.Error(), "ModTime") {
 				// Memfs returns the current time for Stat().ModTime(), which triggers
 				// a diff complaint in fstest.  We can ignore this, or store modtimes
@@ -129,10 +124,10 @@ func checkFsTestError(t *testing.T, err error, files map[string]string) {
 		// We filter on "empty line" or "ModTime" or "$filename: mismatch" to ignore these.
 		lines := strings.Split(err.Error(), "\n")
 		filtered := make([]string, 0, len(lines))
-		filename_mismatches := make(map[string]struct{}, len(files)*2)
+		mismatches := make(map[string]struct{}, len(files)*2)
 		for name := range files {
 			for dirname := name; dirname != "."; dirname = filepath.Dir(dirname) {
-				filename_mismatches[dirname+": mismatch:"] = struct{}{}
+				mismatches[dirname+": mismatch:"] = struct{}{}
 			}
 		}
 		if strings.TrimSpace(lines[0]) == "TestFS found errors:" {
@@ -144,7 +139,7 @@ func checkFsTestError(t *testing.T, err error, files map[string]string) {
 				continue
 			}
 
-			if _, ok := filename_mismatches[trimmed]; ok {
+			if _, ok := mismatches[trimmed]; ok {
 				continue
 			}
 			filtered = append(filtered, line)