Merge pull request #92 from github/lint-action

Set up and configure actions-based linting, and clean up some more warnings

.github/workflows/lint.yml

@@ -0,0 +1,55 @@
+name: Lint
+on:
+  push:
+    paths:
+      - "**.go"
+      - go.mod
+      - go.sum
+  pull_request:
+    paths:
+      - "**.go"
+      - go.mod
+      - go.sum
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Verify dependencies
+        run: |
+          go mod verify
+          go mod download
+
+          LINT_VERSION=1.43.0
+          curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \
+            tar xz --strip-components 1 --wildcards \*/golangci-lint
+          mkdir -p bin && mv golangci-lint bin/
+
+      - name: Run checks
+        run: |
+          STATUS=0
+          assert-nothing-changed() {
+            local diff
+            "$@" >/dev/null || return 1
+            if ! diff="$(git diff -U1 --color --exit-code)"; then
+              printf '\e[31mError: running `\e[1m%s\e[22m` results in modifications that you must check into version control:\e[0m\n%s\n\n' "$*" "$diff" >&2
+              git checkout -- .
+              STATUS=1
+            fi
+          }
+
+          assert-nothing-changed go fmt ./...
+          assert-nothing-changed go mod tidy
+
+          bin/golangci-lint run --out-format=github-actions --timeout=3m || STATUS=$?
+
+          exit $STATUS

.github/workflows/test.yml

@@ -8,12 +8,12 @@ jobs:
       fail-fast: false
     runs-on: ${{ matrix.os }}
     steps:
-      - name: Setup go
+      - name: Set up Go
         uses: actions/setup-go@v2
         with:
           go-version: '1.17'
 
-      - name: Checkout code
+      - name: Check out code
         uses: actions/checkout@v2
 
       - name: Get full repo history

git-sizer.go

@@ -190,7 +190,7 @@ func mainImplementation(stdout, stderr io.Writer, args []string) error {
 
 	err = flags.Parse(args)
 	if err != nil {
-		if err == pflag.ErrHelp {
+		if errors.Is(err, pflag.ErrHelp) {
 			return nil
 		}
 		return err

git_sizer_test.go

@@ -27,10 +27,12 @@ import (
 func sizerExe(t *testing.T) string {
 	t.Helper()
 
-	v := "bin/git-sizer"
+	var v string
 	switch runtime.GOOS {
 	case "windows":
 		v = `bin\git-sizer.exe`
+	default:
+		v = "bin/git-sizer"
 	}
 
 	v, err := exec.LookPath(v)
@@ -238,19 +240,19 @@ func TestRefSelections(t *testing.T) {
 			name: "branches-refgroup",
 			args: []string{"--include=@mygroup"},
 			config: []git.ConfigEntry{
-				{"refgroup.mygroup.include", "refs/heads"},
+				{Key: "refgroup.mygroup.include", Value: "refs/heads"},
 			},
 		},
 		{ // 18
 			name: "combination-refgroup",
 			args: []string{"--include=@mygroup"},
 			config: []git.ConfigEntry{
-				{"refgroup.mygroup.include", "refs/heads"},
-				{"refgroup.mygroup.include", "refs/tags"},
-				{"refgroup.mygroup.exclude", "refs/heads/foo"},
-				{"refgroup.mygroup.includeRegexp", ".*foo.*"},
-				{"refgroup.mygroup.exclude", "refs/foo"},
-				{"refgroup.mygroup.excludeRegexp", "refs/tags/release-.*"},
+				{Key: "refgroup.mygroup.include", Value: "refs/heads"},
+				{Key: "refgroup.mygroup.include", Value: "refs/tags"},
+				{Key: "refgroup.mygroup.exclude", Value: "refs/heads/foo"},
+				{Key: "refgroup.mygroup.includeRegexp", Value: ".*foo.*"},
+				{Key: "refgroup.mygroup.exclude", Value: "refs/foo"},
+				{Key: "refgroup.mygroup.excludeRegexp", Value: "refs/tags/release-.*"},
 			},
 		},
 	} {
@@ -387,14 +389,14 @@ References (included references marked with '+'):
 			config: []git.ConfigEntry{
 				// Note that refgroup "misc" is defined implicitly.
 
-				{"refgroup.misc.foo.includeRegexp", ".*foo.*"},
+				{Key: "refgroup.misc.foo.includeRegexp", Value: ".*foo.*"},
 
-				{"refgroup.misc.foo.oatend.includeRegexp", ".*o"},
+				{Key: "refgroup.misc.foo.oatend.includeRegexp", Value: ".*o"},
 
-				{"refgroup.misc.foo.bogus.include", "bogus"},
+				{Key: "refgroup.misc.foo.bogus.include", Value: "bogus"},
 
-				{"refgroup.tags.releases.name", "Releases"},
-				{"refgroup.tags.releases.includeRegexp", "refs/tags/release-.*"},
+				{Key: "refgroup.tags.releases.name", Value: "Releases"},
+				{Key: "refgroup.tags.releases.includeRegexp", Value: "refs/tags/release-.*"},
 			},
 			stdout: `
 | * References                 |           |                                |
@@ -420,10 +422,10 @@ References (included references marked with '+'):
 			name: "include-refgroups",
 			args: []string{"--include=@branches", "--include=@tags.releases", "--include=@oatend"},
 			config: []git.ConfigEntry{
-				{"refgroup.oatend.includeRegexp", ".*o"},
+				{Key: "refgroup.oatend.includeRegexp", Value: ".*o"},
 
-				{"refgroup.tags.releases.name", "Releases"},
-				{"refgroup.tags.releases.includeRegexp", "refs/tags/release-.*"},
+				{Key: "refgroup.tags.releases.name", Value: "Releases"},
+				{Key: "refgroup.tags.releases.includeRegexp", Value: "refs/tags/release-.*"},
 			},
 			stdout: `
 | * References                 |           |                                |

go.mod

@@ -1,6 +1,6 @@
 module github.com/github/git-sizer
 
-go 1.16
+go 1.17
 
 require (
 	github.com/cli/safeexec v1.0.0
@@ -10,3 +10,8 @@ require (
 	go.uber.org/goleak v1.1.12
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
 )
+
+require (
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
+)

internal/pipe/pipeline.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"sync/atomic"
 )
 
@@ -22,7 +21,7 @@ type Env struct {
 // request that the iteration be ended early (possibly without reading
 // all of its input). This "error" is considered a successful return,
 // and is not reported to the caller.
-//nolint:revive
+//nolint:errname
 var FinishEarly = errors.New("finish stage early")
 
 // Pipeline represents a Unix-like pipe that can include multiple
@@ -134,7 +133,7 @@ func (p *Pipeline) Start(ctx context.Context) error {
 	if p.stdin != nil {
 		// We don't want the first stage to actually close this, and
 		// it's not even an `io.ReadCloser`, so fake it:
-		nextStdin = ioutil.NopCloser(p.stdin)
+		nextStdin = io.NopCloser(p.stdin)
 	}
 
 	for i, s := range p.stages {
@@ -204,7 +203,7 @@ func (p *Pipeline) Wait() error {
 			finishedEarly = false
 			continue
 
-		case err == FinishEarly:
+		case errors.Is(err, FinishEarly):
 			// We ignore `FinishEarly` errors because that is how a
 			// stage informs us that it intentionally finished early.
 			// Moreover, if we see a `FinishEarly` error, ignore any

internal/pipe/pipeline_test.go

@@ -109,7 +109,7 @@ func TestPipelineReadFromSlowly(t *testing.T) {
 	go func() {
 		time.Sleep(200 * time.Millisecond)
 		var err error
-		buf, err = ioutil.ReadAll(r)
+		buf, err = io.ReadAll(r)
 		readErr <- err
 	}()
 
@@ -361,7 +361,7 @@ func TestFunction(t *testing.T) {
 		pipe.Function(
 			"farewell",
 			func(_ context.Context, _ pipe.Env, stdin io.Reader, stdout io.Writer) error {
-				buf, err := ioutil.ReadAll(stdin)
+				buf, err := io.ReadAll(stdin)
 				if err != nil {
 					return err
 				}
@@ -389,7 +389,7 @@ func TestPipelineWithFunction(t *testing.T) {
 		pipe.Function(
 			"farewell",
 			func(_ context.Context, _ pipe.Env, stdin io.Reader, stdout io.Writer) error {
-				buf, err := ioutil.ReadAll(stdin)
+				buf, err := io.ReadAll(stdin)
 				if err != nil {
 					return err
 				}
@@ -419,7 +419,7 @@ func (s ErrorStartingStage) Name() string {
 func (s ErrorStartingStage) Start(
 	ctx context.Context, env pipe.Env, stdin io.ReadCloser,
 ) (io.ReadCloser, error) {
-	return ioutil.NopCloser(&bytes.Buffer{}), s.err
+	return io.NopCloser(&bytes.Buffer{}), s.err
 }
 
 func (s ErrorStartingStage) Wait() error {
@@ -525,7 +525,7 @@ func TestScannerAlwaysFlushes(t *testing.T) {
 			"compute-length",
 			func(_ context.Context, _ pipe.Env, stdin io.Reader, _ io.Writer) error {
 				var err error
-				length, err = io.Copy(ioutil.Discard, stdin)
+				length, err = io.Copy(io.Discard, stdin)
 				return err
 			},
 		),
@@ -567,7 +567,7 @@ func TestScannerFinishEarly(t *testing.T) {
 			"compute-length",
 			func(_ context.Context, _ pipe.Env, stdin io.Reader, _ io.Writer) error {
 				var err error
-				length, err = io.Copy(ioutil.Discard, stdin)
+				length, err = io.Copy(io.Discard, stdin)
 				return err
 			},
 		),

internal/testutils/repoutils.go

@@ -196,7 +196,7 @@ func (repo *TestRepo) CreateObject(
 		t.FailNow()
 	}
 
-	output, err := ioutil.ReadAll(out)
+	output, err := io.ReadAll(out)
 	err2 = cmd.Wait()
 	require.NoError(t, err)
 	require.NoError(t, err2)
@@ -216,7 +216,7 @@ func (repo *TestRepo) AddFile(t *testing.T, relativePath, contents string) {
 	if dirPath != "." {
 		require.NoError(
 			t,
-			os.MkdirAll(filepath.Join(repo.Path, dirPath), 0777),
+			os.MkdirAll(filepath.Join(repo.Path, dirPath), 0o777),
 			"creating subdir",
 		)
 	}

sizes/graph.go

@@ -571,7 +571,7 @@ func (r *treeRecord) initialize(g *Graph, oid git.OID, tree *git.Tree) error {
 		name := entry.Name
 
 		switch {
-		case entry.Filemode&0170000 == 0040000:
+		case entry.Filemode&0o170000 == 0o40000:
 			// Tree
 			listener := func(size TreeSize) {
 				// This listener is called when the tree pointed to by
@@ -595,12 +595,12 @@ func (r *treeRecord) initialize(g *Graph, oid git.OID, tree *git.Tree) error {
 			}
 			r.entryCount.Increment(1)
 
-		case entry.Filemode&0170000 == 0160000:
+		case entry.Filemode&0o170000 == 0o160000:
 			// Commit (i.e., submodule)
 			r.size.addSubmodule(name)
 			r.entryCount.Increment(1)
 
-		case entry.Filemode&0170000 == 0120000:
+		case entry.Filemode&0o170000 == 0o120000:
 			// Symlink
 			g.pathResolver.RecordTreeEntry(oid, name, entry.OID)