golangci-lint: enable modernize

This linter is useful to stay up to date on the latest go std library
features. It applies more or less the same set of fixes like go fix
does.

Changes here were created with "golangci-lint run --fix" automatically.
Plus some minor manual cleanup to keep the code more readable.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Author: Luap99

.golangci.yml

@@ -12,6 +12,7 @@ formatters:
 
 linters:
   enable:
+    - modernize
     - nilnesserr
     - nolintlint
     - revive

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 {

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)

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 {

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

imagebuildah/stage_executor.go

@@ -361,8 +361,7 @@ func joinExcludePatternWithCopySource(srcNorm, excl string) string {
 	if srcNorm == "." {
 		return excl
 	}
-	if strings.HasPrefix(excl, "!") {
-		rest := strings.TrimPrefix(excl, "!")
+	if rest, ok := strings.CutPrefix(excl, "!"); ok {
 		if rest == "" {
 			return excl
 		}
@@ -840,11 +839,12 @@ func (s *stageExecutor) Run(run imagebuilder.Run, config docker.Config) error {
 				args = []string{run.Files[0].Data}
 			}
 		} else {
-			full := args[0]
+			var full strings.Builder
+			full.WriteString(args[0])
 			for _, file := range run.Files {
-				full += file.Data + "\n" + file.Name
+				full.WriteString(file.Data + "\n" + file.Name)
 			}
-			args = []string{full}
+			args = []string{full.String()}
 		}
 	}
 	stageMountPoints, err := s.runStageMountPoints(slices.Concat(run.Mounts, s.executor.transientRunMounts))
@@ -2045,7 +2045,7 @@ func (s *stageExecutor) getCreatedBy(node *parser.Node, addedContentSummary stri
 	case "RUN":
 		shArg := ""
 		buildArgs := s.getBuildArgsResolvedForRun()
-		appendCheckSum := ""
+		var appendCheckSum strings.Builder
 		for _, flag := range node.Flags {
 			var err error
 			mountOptionSource := ""
@@ -2108,7 +2108,7 @@ func (s *stageExecutor) getCreatedBy(node *parser.Node, addedContentSummary stri
 			}
 			if mountCheckSum != "" {
 				// add a separator to appendCheckSum
-				appendCheckSum += ":" + mountCheckSum
+				appendCheckSum.WriteString(":" + mountCheckSum)
 			}
 		}
 		if len(node.Original) > 4 {
@@ -2126,7 +2126,7 @@ func (s *stageExecutor) getCreatedBy(node *parser.Node, addedContentSummary stri
 		if buildArgs != "" {
 			result = result + "|" + strconv.Itoa(len(strings.Split(buildArgs, " "))) + " " + buildArgs + " "
 		}
-		result = result + "/bin/sh -c " + shArg + heredoc + appendCheckSum + labelsAndAnnotations
+		result = result + "/bin/sh -c " + shArg + heredoc + appendCheckSum.String() + labelsAndAnnotations
 		return result, nil
 	case "ADD", "COPY":
 		destination := node
@@ -2804,9 +2804,9 @@ func (s *stageExecutor) EnsureContainerPathAs(path, user string, mode *os.FileMo
 // flag set differently should be reflected in its result.  Some build settings
 // only take affect at the final step, so only note those when they're applied.
 func (s *stageExecutor) buildMetadata(isLastStep bool, isAddOrCopy bool) string {
-	unsetLabels := ""
+	var unsetLabels strings.Builder
 	inheritLabels := ""
-	unsetAnnotations := ""
+	var unsetAnnotations strings.Builder
 	inheritAnnotations := ""
 	newAnnotations := ""
 	layerMutations := ""
@@ -2817,12 +2817,12 @@ func (s *stageExecutor) buildMetadata(isLastStep bool, isAddOrCopy bool) string
 	}
 	// If --unsetlabel was used to clear a label, make a note of it.
 	for _, label := range s.executor.unsetLabels {
-		unsetLabels += "|unsetLabel=" + label
+		unsetLabels.WriteString("|unsetLabel=" + label)
 	}
 	if isLastStep {
 		// If --unsetannotation was used to clear an annotation, make a note of it.
 		for _, annotation := range s.executor.unsetAnnotations {
-			unsetAnnotations += "|unsetAnnotation=" + annotation
+			unsetAnnotations.WriteString("|unsetAnnotation=" + annotation)
 		}
 		// If --inherit-annotation was manually set to false then we cleared the inherited annotations.
 		if s.executor.inheritAnnotations == types.OptionalBoolFalse {
@@ -2855,7 +2855,7 @@ func (s *stageExecutor) buildMetadata(isLastStep bool, isAddOrCopy bool) string
 	}
 
 	if isAddOrCopy {
-		return unsetLabels + " " + inheritLabels + " " + unsetAnnotations + " " + inheritAnnotations + " " + layerMutations + " " + newAnnotations
+		return unsetLabels.String() + " " + inheritLabels + " " + unsetAnnotations.String() + " " + inheritAnnotations + " " + layerMutations + " " + newAnnotations
 	}
-	return unsetLabels + inheritLabels + unsetAnnotations + inheritAnnotations + layerMutations + newAnnotations
+	return unsetLabels.String() + inheritLabels + unsetAnnotations.String() + inheritAnnotations + layerMutations + newAnnotations
 }

internal/mkcw/archive.go

@@ -552,8 +552,7 @@ func slop(size int64, slop string) int64 {
 		if factor == "" {
 			continue
 		}
-		if strings.HasSuffix(factor, "%") {
-			percentage := strings.TrimSuffix(factor, "%")
+		if percentage, ok := strings.CutSuffix(factor, "%"); ok {
 			percent, err := strconv.ParseInt(percentage, 10, 8)
 			if err != nil {
 				logrus.Warnf("parsing percentage %q: %v", factor, err)