Enable some more golangci-lint checks, fix findings (#1164)

* Enable some more golangci-lint checks, fix findings

There's still quite a bit of errchecks to chase down, but this is an
improvement at least.

* Unexport NewErrBadName and remove IsErrBadName

* put IsErrBadName back and deprecate it

.github/workflows/style.yaml

@@ -9,49 +9,22 @@ jobs:
   autoformat:
     name: Auto-format and Check
     runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false # Keep running if one leg fails.
-      matrix:
-        tool:
-          - goimports
-          - gofmt
-
-        include:
-        - tool: gofmt
-          options: -s
-        - tool: goimports
-          importpath: golang.org/x/tools/cmd/goimports
 
     steps:
-      - name: Set up Go 1.16.x
-        uses: actions/setup-go@v2
+      - uses: actions/checkout@v2
+      - uses: actions/setup-go@v2
         with:
           go-version: 1.16.x
-        id: go
-
-      - name: Check out code
-        uses: actions/checkout@v2
-
-      - name: Install Dependencies
-        if: ${{ matrix.importpath != '' }}
-        run: |
-          cd $(mktemp -d)
-          GO111MODULE=on go get ${{ matrix.importpath }}
-
-      - name: ${{ matrix.tool }} ${{ matrix.options }}
-        shell: bash
-        run: >
-          ${{ matrix.tool }} ${{ matrix.options }} -w
-          $(find .
+      - run: go install golang.org/x/tools/cmd/goimports@latest
+      - run: >
+          goimports -w $(find .
           -path './vendor' -prune
           -o -path './third_party' -prune
           -o -name '*.pb.go' -prune
           -o -name 'wire_gen.go' -prune
           -o -type f -name '*.go' -print)
 
-      - name: Verify ${{ matrix.tool }}
-        shell: bash
-        run: |
+      - run: |
           # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/
           # This is to leverage this workaround:
           # https://github.com/actions/toolkit/issues/193#issuecomment-605394935
@@ -60,9 +33,9 @@ jobs:
           }
           if [[ $(git diff-index --name-only HEAD --) ]]; then
               for x in $(git diff-index --name-only HEAD --); do
-                echo "::error file=$x::Please run ${{ matrix.tool }} ${{ matrix.options }}.%0A$(git diff $x | urlencode)"
+                echo "::error file=$x::Please run goimports.%0A$(git diff $x | urlencode)"
               done
-              echo "${{ github.repository }} is out of style. Please run ${{ matrix.tool }} ${{ matrix.options }}."
+              echo "${{ github.repository }} is out of style. Please run goimports."
               exit 1
           fi
           echo "${{ github.repository }} is formatted correctly."
@@ -72,14 +45,10 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - name: Set up Go 1.16.x
-        uses: actions/setup-go@v2
+      - uses: actions/checkout@v2
+      - uses: actions/setup-go@v2
         with:
           go-version: 1.16.x
-        id: go
-
-      - name: Check out code
-        uses: actions/checkout@v2
 
       - name: Install Tools
         env:
@@ -102,18 +71,11 @@ jobs:
 
           echo "${TEMP_PATH}" >> $GITHUB_PATH
 
-      - id: golangci_configuration
-        uses: andstor/file-existence-action@v1
-        with:
-          files: .golangci.yaml
-      - name: Go Lint
-        if: steps.golangci_configuration.outputs.files_exists == 'true'
-        uses: golangci/golangci-lint-action@v2
+      - uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.38
+          version: v1.42.1
 
       - name: misspell
-        shell: bash
         if: ${{ always() }}
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
@@ -141,7 +103,6 @@ jobs:
           echo '::endgroup::'
 
       - name: trailing whitespace
-        shell: bash
         if: ${{ always() }}
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
@@ -170,7 +131,6 @@ jobs:
           echo '::endgroup::'
 
       - name: EOF newline
-        shell: bash
         if: ${{ always() }}
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
@@ -213,7 +173,6 @@ jobs:
       # since their action is not yet released under a stable version.
       - name: Language
         if: ${{ always() && github.event_name == 'pull_request' }}
-        shell: bash
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
         run: |

.golangci.yaml

@@ -7,9 +7,21 @@ run:
 
 linters:
   enable:
-    - asciicheck
-    - depguard
-    - stylecheck
-    - unconvert
+  - asciicheck
+  - deadcode
+  - depguard
+  - errorlint
+  - gofmt
+  - goimports
+  - importas
+  - prealloc
+  - revive
+  - misspell
+  - stylecheck
+  - tparallel
+  - unconvert
+  - unparam
+  - whitespace
+
   disable:
-    - errcheck
+  - errcheck

cmd/crane/cmd/append.go

@@ -47,19 +47,19 @@ func NewCmdAppend(options *[]crane.Option) *cobra.Command {
 			} else {
 				base, err = crane.Pull(baseRef, *options...)
 				if err != nil {
-					return fmt.Errorf("pulling %s: %v", baseRef, err)
+					return fmt.Errorf("pulling %s: %w", baseRef, err)
 				}
 			}
 
 			img, err := crane.Append(base, newLayers...)
 			if err != nil {
-				return fmt.Errorf("appending %v: %v", newLayers, err)
+				return fmt.Errorf("appending %v: %w", newLayers, err)
 			}
 
 			if baseRef != "" && annotate {
 				ref, err := name.ParseReference(baseRef)
 				if err != nil {
-					return fmt.Errorf("parsing ref %q: %v", baseRef, err)
+					return fmt.Errorf("parsing ref %q: %w", baseRef, err)
 				}
 
 				baseDigest, err := base.Digest()
@@ -77,19 +77,19 @@ func NewCmdAppend(options *[]crane.Option) *cobra.Command {
 
 			if outFile != "" {
 				if err := crane.Save(img, newTag, outFile); err != nil {
-					return fmt.Errorf("writing output %q: %v", outFile, err)
+					return fmt.Errorf("writing output %q: %w", outFile, err)
 				}
 			} else {
 				if err := crane.Push(img, newTag, *options...); err != nil {
-					return fmt.Errorf("pushing image %s: %v", newTag, err)
+					return fmt.Errorf("pushing image %s: %w", newTag, err)
 				}
 				ref, err := name.ParseReference(newTag)
 				if err != nil {
-					return fmt.Errorf("parsing reference %s: %v", newTag, err)
+					return fmt.Errorf("parsing reference %s: %w", newTag, err)
 				}
 				d, err := img.Digest()
 				if err != nil {
-					return fmt.Errorf("digest: %v", err)
+					return fmt.Errorf("digest: %w", err)
 				}
 				fmt.Println(ref.Context().Digest(d.String()))
 			}

cmd/crane/cmd/auth.go

@@ -36,9 +36,7 @@ func NewCmdAuth(argv ...string) *cobra.Command {
 		Use:   "auth",
 		Short: "Log in or access credentials",
 		Args:  cobra.NoArgs,
-		Run: func(cmd *cobra.Command, args []string) {
-			cmd.Usage()
-		},
+		RunE:  func(cmd *cobra.Command, _ []string) error { return cmd.Usage() },
 	}
 	cmd.AddCommand(NewCmdAuthGet(argv...), NewCmdAuthLogin(argv...))
 	return cmd

cmd/crane/cmd/blob.go

@@ -33,14 +33,14 @@ func NewCmdBlob(options *[]crane.Option) *cobra.Command {
 			src := args[0]
 			layer, err := crane.PullLayer(src, *options...)
 			if err != nil {
-				return fmt.Errorf("pulling layer %s: %v", src, err)
+				return fmt.Errorf("pulling layer %s: %w", src, err)
 			}
 			blob, err := layer.Compressed()
 			if err != nil {
-				return fmt.Errorf("fetching blob %s: %v", src, err)
+				return fmt.Errorf("fetching blob %s: %w", src, err)
 			}
 			if _, err := io.Copy(cmd.OutOrStdout(), blob); err != nil {
-				return fmt.Errorf("copying blob %s: %v", src, err)
+				return fmt.Errorf("copying blob %s: %w", src, err)
 			}
 			return nil
 		},

cmd/crane/cmd/catalog.go

@@ -31,7 +31,7 @@ func NewCmdCatalog(options *[]crane.Option) *cobra.Command {
 			reg := args[0]
 			repos, err := crane.Catalog(reg, *options...)
 			if err != nil {
-				return fmt.Errorf("reading repos for %s: %v", reg, err)
+				return fmt.Errorf("reading repos for %s: %w", reg, err)
 			}
 
 			for _, repo := range repos {

cmd/crane/cmd/config.go

@@ -30,7 +30,7 @@ func NewCmdConfig(options *[]crane.Option) *cobra.Command {
 		RunE: func(_ *cobra.Command, args []string) error {
 			cfg, err := crane.Config(args[0], *options...)
 			if err != nil {
-				return fmt.Errorf("fetching config: %v", err)
+				return fmt.Errorf("fetching config: %w", err)
 			}
 			fmt.Print(string(cfg))
 			return nil

cmd/crane/cmd/digest.go

@@ -31,7 +31,9 @@ func NewCmdDigest(options *[]crane.Option) *cobra.Command {
 		Args:  cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if tarball == "" && len(args) == 0 {
-				cmd.Help()
+				if err := cmd.Help(); err != nil {
+					return err
+				}
 				return errors.New("image reference required without --tarball")
 			}
 
@@ -65,11 +67,11 @@ func getTarballDigest(tarball string, args []string, options *[]crane.Option) (s
 
 	img, err := crane.LoadTag(tarball, tag, *options...)
 	if err != nil {
-		return "", fmt.Errorf("loading image from %q: %v", tarball, err)
+		return "", fmt.Errorf("loading image from %q: %w", tarball, err)
 	}
 	digest, err := img.Digest()
 	if err != nil {
-		return "", fmt.Errorf("computing digest: %v", err)
+		return "", fmt.Errorf("computing digest: %w", err)
 	}
 	return digest.String(), nil
 }

cmd/crane/cmd/export.go

@@ -38,13 +38,13 @@ func NewCmdExport(options *[]crane.Option) *cobra.Command {
 
 			f, err := openFile(dst)
 			if err != nil {
-				return fmt.Errorf("failed to open %s: %v", dst, err)
+				return fmt.Errorf("failed to open %s: %w", dst, err)
 			}
 			defer f.Close()
 
 			img, err := crane.Pull(src, *options...)
 			if err != nil {
-				return fmt.Errorf("pulling %s: %v", src, err)
+				return fmt.Errorf("pulling %s: %w", src, err)
 			}
 
 			return crane.Export(img, f)

cmd/crane/cmd/flatten.go

@@ -93,7 +93,7 @@ func NewCmdFlatten(options *[]crane.Option) *cobra.Command {
 func flatten(ref name.Reference, repo name.Repository, use string, o crane.Options) (partial.Describable, error) {
 	desc, err := remote.Get(ref, o.Remote...)
 	if err != nil {
-		return nil, fmt.Errorf("pulling %s: %v", ref, err)
+		return nil, fmt.Errorf("pulling %s: %w", ref, err)
 	}
 
 	if desc.MediaType.IsIndex() {
@@ -201,16 +201,16 @@ func flattenChild(old partial.Describable, repo name.Repository, use string, o c
 func flattenImage(old v1.Image, repo name.Repository, use string, o crane.Options) (partial.Describable, error) {
 	digest, err := old.Digest()
 	if err != nil {
-		return nil, fmt.Errorf("getting old digest: %v", err)
+		return nil, fmt.Errorf("getting old digest: %w", err)
 	}
 	m, err := old.Manifest()
 	if err != nil {
-		return nil, fmt.Errorf("reading manifest: %v", err)
+		return nil, fmt.Errorf("reading manifest: %w", err)
 	}
 
 	cf, err := old.ConfigFile()
 	if err != nil {
-		return nil, fmt.Errorf("getting config: %v", err)
+		return nil, fmt.Errorf("getting config: %w", err)
 	}
 	cf = cf.DeepCopy()
 
@@ -225,13 +225,13 @@ func flattenImage(old v1.Image, repo name.Repository, use string, o crane.Option
 
 	img, err := mutate.ConfigFile(empty.Image, cf)
 	if err != nil {
-		return nil, fmt.Errorf("mutating config: %v", err)
+		return nil, fmt.Errorf("mutating config: %w", err)
 	}
 
 	// TODO: Make compression configurable?
 	layer := stream.NewLayer(mutate.Extract(old), stream.WithCompressionLevel(gzip.BestCompression))
 	if err := remote.WriteLayer(repo, layer, o.Remote...); err != nil {
-		return nil, fmt.Errorf("uploading layer: %v", err)
+		return nil, fmt.Errorf("uploading layer: %w", err)
 	}
 
 	img, err = mutate.Append(img, mutate.Addendum{
@@ -242,7 +242,7 @@ func flattenImage(old v1.Image, repo name.Repository, use string, o crane.Option
 		},
 	})
 	if err != nil {
-		return nil, fmt.Errorf("appending layers: %v", err)
+		return nil, fmt.Errorf("appending layers: %w", err)
 	}
 
 	// Retain any annotations from the original image.

cmd/crane/cmd/list.go

@@ -31,7 +31,7 @@ func NewCmdList(options *[]crane.Option) *cobra.Command {
 			repo := args[0]
 			tags, err := crane.ListTags(repo, *options...)
 			if err != nil {
-				return fmt.Errorf("reading tags for %s: %v", repo, err)
+				return fmt.Errorf("reading tags for %s: %w", repo, err)
 			}
 
 			for _, tag := range tags {

cmd/crane/cmd/manifest.go

@@ -31,7 +31,7 @@ func NewCmdManifest(options *[]crane.Option) *cobra.Command {
 			src := args[0]
 			manifest, err := crane.Manifest(src, *options...)
 			if err != nil {
-				return fmt.Errorf("fetching manifest %s: %v", src, err)
+				return fmt.Errorf("fetching manifest %s: %w", src, err)
 			}
 			fmt.Print(string(manifest))
 			return nil

cmd/crane/cmd/pull.go

@@ -37,7 +37,7 @@ func NewCmdPull(options *[]crane.Option) *cobra.Command {
 			for _, src := range srcList {
 				img, err := crane.Pull(src, *options...)
 				if err != nil {
-					return fmt.Errorf("pulling %s: %v", src, err)
+					return fmt.Errorf("pulling %s: %w", src, err)
 				}
 				if cachePath != "" {
 					img = cache.Image(img, cache.NewFilesystemCache(cachePath))
@@ -49,15 +49,15 @@ func NewCmdPull(options *[]crane.Option) *cobra.Command {
 			switch format {
 			case "tarball":
 				if err := crane.MultiSave(imageMap, path); err != nil {
-					return fmt.Errorf("saving tarball %s: %v", path, err)
+					return fmt.Errorf("saving tarball %s: %w", path, err)
 				}
 			case "legacy":
 				if err := crane.MultiSaveLegacy(imageMap, path); err != nil {
-					return fmt.Errorf("saving legacy tarball %s: %v", path, err)
+					return fmt.Errorf("saving legacy tarball %s: %w", path, err)
 				}
 			case "oci":
 				if err := crane.MultiSaveOCI(imageMap, path); err != nil {
-					return fmt.Errorf("saving oci image layout %s: %v", path, err)
+					return fmt.Errorf("saving oci image layout %s: %w", path, err)
 				}
 			default:
 				return fmt.Errorf("unexpected --format: %q (valid values are: tarball, legacy, and oci)", format)