Merge pull request #6942 from Luap99/lint

golangci-lint: enable nilnesserr, modernize and more govet checks

Author: TomSweeneyRedHat

.golangci.yml

@@ -12,6 +12,8 @@ formatters:
 
 linters:
   enable:
+    - modernize
+    - nilnesserr
     - nolintlint
     - revive
     - unconvert
@@ -27,3 +29,12 @@ linters:
       checks:
         - all
         - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression.
+    govet:
+      enable-all: true
+      disable:
+        - fieldalignment
+        - shadow
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0

add.go

@@ -636,8 +636,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				}()
 			}
 
-			wg.Add(1)
-			go func() {
+			wg.Go(func() {
 				b.ContentDigester.Start("")
 				hashCloser := b.ContentDigester.Hash()
 				hasher := io.Writer(hashCloser)
@@ -661,8 +660,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				}
 				hashCloser.Close()
 				pipeReader.Close()
-				wg.Done()
-			}()
+			})
 			wg.Wait()
 			if getErr != nil {
 				getErr = fmt.Errorf("reading %q: %w", src, getErr)
@@ -739,8 +737,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				latestTimestamp = st.ModTime
 			}
 			pipeReader, pipeWriter := io.Pipe()
-			wg.Add(1)
-			go func() {
+			wg.Go(func() {
 				renamedItems := 0
 				writer := io.WriteCloser(pipeWriter)
 				if renameTarget != "" {
@@ -796,10 +793,8 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				if renameTarget != "" && renamedItems > 1 {
 					renameErr = fmt.Errorf("internal error: renamed %d items when we expected to only rename 1", renamedItems)
 				}
-				wg.Done()
-			}()
-			wg.Add(1)
-			go func() {
+			})
+			wg.Go(func() {
 				if st.IsDir {
 					b.ContentDigester.Start("dir")
 				} else {
@@ -829,8 +824,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				}
 				hashCloser.Close()
 				pipeReader.Close()
-				wg.Done()
-			}()
+			})
 
 			wg.Wait()
 			if getErr != nil {

bind/mount.go

@@ -254,9 +254,6 @@ func UnmountMountpoints(mountpoint string, mountpointsToRemove []string) error {
 	// find the top of the tree we're unmounting
 	top := getMountByPoint(mountpoint)
 	if top == nil {
-		if err != nil {
-			return fmt.Errorf("%q is not mounted: %w", mountpoint, err)
-		}
 		return nil
 	}
 	// add all of the mounts that are hanging off of it

chroot/run_common.go

@@ -146,12 +146,10 @@ func RunUsingChroot(spec *specs.Spec, bundlePath, homeDir string, stdin io.Reade
 	}
 
 	logrus.Debugf("Running %#v in %#v", cmd.Cmd, cmd)
-	confwg.Add(1)
-	go func() {
+	confwg.Go(func() {
 		_, conferr = io.Copy(pwriter, bytes.NewReader(config))
 		pwriter.Close()
-		confwg.Done()
-	}()
+	})
 	cmd.ExtraFiles = append([]*os.File{preader}, cmd.ExtraFiles...)
 	err = cmd.Run()
 	confwg.Wait()
@@ -541,12 +539,10 @@ func runUsingChroot(spec *specs.Spec, bundlePath string, ctty *os.File, stdin io
 	}
 
 	logrus.Debugf("Running %#v in %#v", cmd.Cmd, cmd)
-	confwg.Add(1)
-	go func() {
+	confwg.Go(func() {
 		_, conferr = io.Copy(pwriter, bytes.NewReader(config))
 		pwriter.Close()
-		confwg.Done()
-	}()
+	})
 	err = cmd.Run()
 	confwg.Wait()
 	signal.Stop(interrupted)

convertcw.go

@@ -140,9 +140,6 @@ func CWConvertImage(ctx context.Context, systemContext *types.SystemContext, sto
 		}
 	}()
 	sourceInfo := GetBuildInfo(source)
-	if err != nil {
-		return "", nil, "", fmt.Errorf("retrieving info about source image: %w", err)
-	}
 	sourceImageID := sourceInfo.FromImageID
 	sourceSize, err := store.ImageSize(sourceImageID)
 	if err != nil {

copier/copier.go

@@ -837,20 +837,16 @@ func copierWithSubprocess(bulkReader io.Reader, bulkWriter io.Writer, req reques
 	stdoutRead = nil
 	var wg sync.WaitGroup
 	var readError, writeError error
-	wg.Add(1)
-	go func() {
+	wg.Go(func() {
 		_, writeError = io.Copy(bulkWriter, bulkWriterRead)
 		bulkWriterRead.Close()
 		bulkWriterRead = nil
-		wg.Done()
-	}()
-	wg.Add(1)
-	go func() {
+	})
+	wg.Go(func() {
 		_, readError = io.Copy(bulkReaderWrite, bulkReader)
 		bulkReaderWrite.Close()
 		bulkReaderWrite = nil
-		wg.Done()
-	}()
+	})
 	wg.Wait()
 	cmdToWaitFor = nil
 	if err = cmd.Wait(); err != nil {
@@ -1359,8 +1355,6 @@ func checkLinks(item string, req request, info os.FileInfo) (string, os.FileInfo
 }
 
 func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMatcher, idMappings *idtools.IDMappings) (*response, func() error, error) {
-	statRequest := req
-	statRequest.Request = requestStat
 	statResponse := copierHandlerStat(req, pm, idMappings)
 	errorResponse := func(fmtspec string, args ...any) (*response, func() error, error) {
 		return &response{Error: fmt.Sprintf(fmtspec, args...), Stat: statResponse.Stat, Get: getResponse{}}, nil, nil

copier/copier_test.go

@@ -798,12 +798,10 @@ func testGetSingle(t *testing.T) {
 						pipeReader, pipeWriter := io.Pipe()
 						var getErr error
 						var wg sync.WaitGroup
-						wg.Add(1)
-						go func() {
+						wg.Go(func() {
 							getErr = Get(root, topdir, getOptions, []string{name}, pipeWriter)
 							pipeWriter.Close()
-							wg.Done()
-						}()
+						})
 						tr := tar.NewReader(pipeReader)
 						hdr, err := tr.Next()
 						for err == nil {
@@ -823,12 +821,10 @@ func testGetSingle(t *testing.T) {
 											getOptions.StripSetgidBit = stripSetgidBit
 											getOptions.StripStickyBit = stripStickyBit
 											pipeReader, pipeWriter := io.Pipe()
-											wg.Add(1)
-											go func() {
+											wg.Go(func() {
 												getErr = Get(root, topdir, getOptions, []string{name}, pipeWriter)
 												pipeWriter.Close()
-												wg.Done()
-											}()
+											})
 											tr := tar.NewReader(pipeReader)
 											hdr, err := tr.Next()
 											for err == nil {
@@ -1605,12 +1601,10 @@ func testGetMultiple(t *testing.T) {
 					pipeReader, pipeWriter := io.Pipe()
 					var getErr error
 					var wg sync.WaitGroup
-					wg.Add(1)
-					go func() {
-						defer wg.Done()
+					wg.Go(func() {
 						getErr = Get(root, topdir, getOptions, []string{testCase.pattern}, pipeWriter)
 						pipeWriter.Close()
-					}()
+					})
 					tr := tar.NewReader(pipeReader)
 					hdr, err := tr.Next()
 					actualContents := []string{}
@@ -3096,13 +3090,11 @@ func testChmod(t *testing.T) {
 
 			pipeReader, pipeWriter := io.Pipe()
 			var wg sync.WaitGroup
-			wg.Add(1)
-			go func() {
+			wg.Go(func() {
 				opts := GetOptions{Chmod: v.chmod, ChmodDirs: v.chmodDirs, ChmodFiles: v.chmodFiles}
 				err = Get(testDir, "", opts, []string{name}, pipeWriter)
 				pipeWriter.Close()
-				wg.Done()
-			}()
+			})
 			tr := tar.NewReader(pipeReader)
 			hdr, tarErr := tr.Next()
 			for tarErr == nil {

digester.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"hash"
 	"io"
+	"strings"
 	"sync"
 	"time"
 
@@ -100,8 +101,7 @@ func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (sk
 	filterer := &tarFilterer{
 		pipeWriter: pipeWriter,
 	}
-	filterer.wg.Add(1)
-	go func() {
+	filterer.wg.Go(func() {
 		filterer.closedLock.Lock()
 		closed := filterer.closed
 		filterer.closedLock.Unlock()
@@ -161,8 +161,7 @@ func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (sk
 		} else {
 			pipeReader.Close()
 		}
-		filterer.wg.Done()
-	}()
+	})
 	return filterer
 }
 
@@ -270,17 +269,17 @@ func (c *CompositeDigester) Digest() (string, digest.Digest) {
 	case 1:
 		return c.digesters[0].ContentType(), c.digesters[0].Digest()
 	default:
-		content := ""
+		var content strings.Builder
 		for i, digester := range c.digesters {
 			if i > 0 {
-				content += ","
+				content.WriteString(",")
 			}
 			contentType := digester.ContentType()
 			if contentType != "" {
 				contentType += ":"
 			}
-			content += contentType + digester.Digest().Encoded()
+			content.WriteString(contentType + digester.Digest().Encoded())
 		}
-		return "multi", digest.Canonical.FromString(content)
+		return "multi", digest.Canonical.FromString(content.String())
 	}
 }

digester_test.go

@@ -280,8 +280,7 @@ func TestTarFilterer(t *testing.T) {
 			output := make(map[string]string)
 			pipeReader, pipeWriter := io.Pipe()
 			var wg sync.WaitGroup
-			wg.Add(1)
-			go func() {
+			wg.Go(func() {
 				tr := tar.NewReader(pipeReader)
 				hdr, err := tr.Next()
 				for err == nil {
@@ -295,8 +294,7 @@ func TestTarFilterer(t *testing.T) {
 				}
 				require.Equal(t, io.EOF, err, "unexpected error ended our tarstream read")
 				pipeReader.Close()
-				wg.Done()
-			}()
+			})
 			filterer := newTarFilterer(pipeWriter, test.filter)
 			_, err := io.Copy(filterer, &buffer)
 			require.Nil(t, err, "unexpected error copying archive through filter to reader")

imagebuildah/executor.go

@@ -600,32 +600,34 @@ func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageE
 		// processed like regular steps, and if no modification is done to
 		// layers, its easier to reuse cached layers.
 		if len(b.labels) > 0 {
-			labelLine := "LABEL"
+			var labelLine strings.Builder
+			labelLine.WriteString("LABEL")
 			for _, labelSpec := range b.labels {
 				key, value, _ := strings.Cut(labelSpec, "=")
 				// check only for an empty key since docker allows empty values
 				if key != "" {
-					labelLine += fmt.Sprintf(" %q=%q", key, value)
+					fmt.Fprintf(&labelLine, " %q=%q", key, value)
 				}
 			}
-			appendInstructions = slices.Concat(appendInstructions, []string{labelLine})
+			appendInstructions = slices.Concat(appendInstructions, []string{labelLine.String()})
 		}
 	}
 
 	// If we were given environment variables to set via the API, add them as instructions
 	// at the beginning of the stage so that they affect subsequent RUN instructions and
 	// factor into the image history.
 	if len(b.envs) > 0 {
-		envLine := "ENV"
+		var envLine strings.Builder
+		envLine.WriteString("ENV")
 		for _, envSpec := range b.envs {
 			key, value, hasValue := strings.Cut(envSpec, "=")
 			if hasValue {
-				envLine += fmt.Sprintf(" %q=%q", key, value)
+				fmt.Fprintf(&envLine, " %q=%q", key, value)
 			} else {
 				return "", nil, false, fmt.Errorf("BUG: unresolved environment variable: %q", key)
 			}
 		}
-		prependInstructions = slices.Concat([]string{envLine}, prependInstructions)
+		prependInstructions = slices.Concat([]string{envLine.String()}, prependInstructions)
 	}
 
 	// Create stage labels for all stage images including final stage