From c23a7e7281b183c7eb4406dff02028c5b756ac84 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn <github@gone.nl> Date: Mon, 10 Feb 2025 13:15:23 +0100 Subject: [PATCH] golangci-lint: enable copyloopvar linter capturing loop variables is no longer needed in go1.22 and higher; https://go.dev/blog/loopvar-preview This path enables the copyloopvar linter, which finds places where capturing is no longer needed, and removes locations where they could be removed. Also made some minor changes, and renamed some vars in places where we could use a shorter name that's less likely to conflict with imports. Signed-off-by: Sebastiaan van Stijn <github@gone.nl> --- .golangci.yml | 1 + pkg/compose/convergence.go | 2 -- pkg/compose/dependencies.go | 1 - pkg/compose/down.go | 10 ++++------ pkg/compose/image_pruner.go | 1 - pkg/compose/images.go | 1 - pkg/compose/logs.go | 7 +++---- pkg/compose/logs_test.go | 1 - pkg/compose/ps.go | 1 - pkg/compose/pull.go | 3 +-- pkg/compose/push.go | 2 -- pkg/compose/remove.go | 7 +++---- pkg/compose/restart.go | 14 +++++++------- pkg/compose/top.go | 9 ++++----- pkg/compose/viz.go | 1 - pkg/compose/wait.go | 7 +++---- 16 files changed, 26 insertions(+), 42 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 49cacaac8..090e12151 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,6 +5,7 @@ linters: enable-all: false disable-all: true enable: + - copyloopvar - depguard - errcheck - errorlint diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index 23992d8a1..016c14f25 100644 --- a/pkg/compose/convergence.go +++ b/pkg/compose/convergence.go @@ -205,7 +205,6 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project, // Scale UP number := next + i name := getContainerName(project.Name, service, number) - i := i eventOpts := tracing.SpanOptions{trace.WithAttributes(attribute.String("container.name", name))} eg.Go(tracing.EventWrapFuncForErrGroup(ctx, "service/scale/up", eventOpts, func(ctx context.Context) error { opts := createOptions{ @@ -470,7 +469,6 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr continue } - dep, config := dep, config eg.Go(func() error { ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() diff --git a/pkg/compose/dependencies.go b/pkg/compose/dependencies.go index a664deab3..ba8bb960a 100644 --- a/pkg/compose/dependencies.go +++ b/pkg/compose/dependencies.go @@ -172,7 +172,6 @@ func (t *graphTraversal) run(ctx context.Context, graph *Graph, eg *errgroup.Gro continue } - node := node if !t.consume(node.Key) { // another worker already visited this node continue diff --git a/pkg/compose/down.go b/pkg/compose/down.go index c3fc35bdc..23fe4641c 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -322,10 +322,9 @@ func (s *composeService) stopContainer(ctx context.Context, w progress.Writer, s func (s *composeService) stopContainers(ctx context.Context, w progress.Writer, serv *types.ServiceConfig, containers []moby.Container, timeout *time.Duration) error { eg, ctx := errgroup.WithContext(ctx) - for _, container := range containers { - container := container + for _, ctr := range containers { eg.Go(func() error { - return s.stopContainer(ctx, w, serv, container, timeout) + return s.stopContainer(ctx, w, serv, ctr, timeout) }) } return eg.Wait() @@ -333,10 +332,9 @@ func (s *composeService) stopContainers(ctx context.Context, w progress.Writer, func (s *composeService) removeContainers(ctx context.Context, containers []moby.Container, service *types.ServiceConfig, timeout *time.Duration, volumes bool) error { eg, _ := errgroup.WithContext(ctx) - for _, container := range containers { - container := container + for _, ctr := range containers { eg.Go(func() error { - return s.stopAndRemoveContainer(ctx, container, service, timeout, volumes) + return s.stopAndRemoveContainer(ctx, ctr, service, timeout, volumes) }) } return eg.Wait() diff --git a/pkg/compose/image_pruner.go b/pkg/compose/image_pruner.go index a4e72319e..2cb34930c 100644 --- a/pkg/compose/image_pruner.go +++ b/pkg/compose/image_pruner.go @@ -202,7 +202,6 @@ func (p *ImagePruner) filterImagesByExistence(ctx context.Context, imageNames [] eg, ctx := errgroup.WithContext(ctx) for _, img := range imageNames { - img := img eg.Go(func() error { _, _, err := p.client.ImageInspectWithRaw(ctx, img) if errdefs.IsNotFound(err) { diff --git a/pkg/compose/images.go b/pkg/compose/images.go index a85a18288..c307c4f63 100644 --- a/pkg/compose/images.go +++ b/pkg/compose/images.go @@ -82,7 +82,6 @@ func (s *composeService) getImageSummaries(ctx context.Context, repoTags []strin l := sync.Mutex{} eg, ctx := errgroup.WithContext(ctx) for _, repoTag := range repoTags { - repoTag := repoTag eg.Go(func() error { inspect, _, err := s.apiClient().ImageInspectWithRaw(ctx, repoTag) if err != nil { diff --git a/pkg/compose/logs.go b/pkg/compose/logs.go index dee4b8bf3..4efb0cfb3 100644 --- a/pkg/compose/logs.go +++ b/pkg/compose/logs.go @@ -62,13 +62,12 @@ func (s *composeService) Logs( } eg, ctx := errgroup.WithContext(ctx) - for _, c := range containers { - c := c + for _, ctr := range containers { eg.Go(func() error { - err := s.logContainers(ctx, consumer, c, options) + err := s.logContainers(ctx, consumer, ctr, options) var notImplErr errdefs.ErrNotImplemented if errors.As(err, ¬ImplErr) { - logrus.Warnf("Can't retrieve logs for %q: %s", getCanonicalContainerName(c), err.Error()) + logrus.Warnf("Can't retrieve logs for %q: %s", getCanonicalContainerName(ctr), err.Error()) return nil } return err diff --git a/pkg/compose/logs_test.go b/pkg/compose/logs_test.go index 561d395a3..5792e235a 100644 --- a/pkg/compose/logs_test.go +++ b/pkg/compose/logs_test.go @@ -134,7 +134,6 @@ func TestComposeService_Logs_ServiceFiltering(t *testing.T) { ) for _, id := range []string{"c1", "c2", "c4"} { - id := id api.EXPECT(). ContainerInspect(anyCancellableContext(), id). Return( diff --git a/pkg/compose/ps.go b/pkg/compose/ps.go index 93f5013ce..72b47e30f 100644 --- a/pkg/compose/ps.go +++ b/pkg/compose/ps.go @@ -43,7 +43,6 @@ func (s *composeService) Ps(ctx context.Context, projectName string, options api summary := make([]api.ContainerSummary, len(containers)) eg, ctx := errgroup.WithContext(ctx) for i, container := range containers { - i, container := i, container eg.Go(func() error { publishers := make([]api.PortPublisher, len(container.Ports)) sort.Slice(container.Ports, func(i, j int) bool { diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index de9f5f5de..4707e7c60 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -113,7 +113,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts imagesBeingPulled[service.Image] = service.Name - idx, name, service := i, name, service + idx := i eg.Go(func() error { _, err := s.pullServiceImage(ctx, service, s.configFile(), w, opts.Quiet, project.Environment["DOCKER_DEFAULT_PLATFORM"]) if err != nil { @@ -316,7 +316,6 @@ func (s *composeService) pullRequiredImages(ctx context.Context, project *types. eg.SetLimit(s.maxConcurrency) pulledImages := make([]string, len(needPull)) for i, service := range needPull { - i, service := i, service eg.Go(func() error { id, err := s.pullServiceImage(ctx, service, s.configFile(), w, quietPull, project.Environment["DOCKER_DEFAULT_PLATFORM"]) pulledImages[i] = id diff --git a/pkg/compose/push.go b/pkg/compose/push.go index 257bc5da1..8d0e2099e 100644 --- a/pkg/compose/push.go +++ b/pkg/compose/push.go @@ -72,14 +72,12 @@ func (s *composeService) push(ctx context.Context, project *types.Project, optio }) continue } - service := service tags := []string{service.Image} if service.Build != nil { tags = append(tags, service.Build.Tags...) } for _, tag := range tags { - tag := tag eg.Go(func() error { err := s.pushServiceImage(ctx, tag, info, s.configFile(), w, options.Quiet) if err != nil { diff --git a/pkg/compose/remove.go b/pkg/compose/remove.go index 993398be8..1d13790aa 100644 --- a/pkg/compose/remove.go +++ b/pkg/compose/remove.go @@ -102,12 +102,11 @@ func (s *composeService) Remove(ctx context.Context, projectName string, options func (s *composeService) remove(ctx context.Context, containers Containers, options api.RemoveOptions) error { w := progress.ContextWriter(ctx) eg, ctx := errgroup.WithContext(ctx) - for _, container := range containers { - container := container + for _, ctr := range containers { eg.Go(func() error { - eventName := getContainerProgressName(container) + eventName := getContainerProgressName(ctr) w.Event(progress.RemovingEvent(eventName)) - err := s.apiClient().ContainerRemove(ctx, container.ID, containerType.RemoveOptions{ + err := s.apiClient().ContainerRemove(ctx, ctr.ID, containerType.RemoveOptions{ RemoveVolumes: options.Volumes, Force: options.Force, }) diff --git a/pkg/compose/restart.go b/pkg/compose/restart.go index ffb40e44d..2f987aeff 100644 --- a/pkg/compose/restart.go +++ b/pkg/compose/restart.go @@ -78,17 +78,17 @@ func (s *composeService) restart(ctx context.Context, projectName string, option w := progress.ContextWriter(ctx) return InDependencyOrder(ctx, project, func(c context.Context, service string) error { eg, ctx := errgroup.WithContext(ctx) - for _, container := range containers.filter(isService(service)) { - container := container + for _, ctr := range containers.filter(isService(service)) { eg.Go(func() error { - eventName := getContainerProgressName(container) + eventName := getContainerProgressName(ctr) w.Event(progress.RestartingEvent(eventName)) timeout := utils.DurationSecondToInt(options.Timeout) - err := s.apiClient().ContainerRestart(ctx, container.ID, containerType.StopOptions{Timeout: timeout}) - if err == nil { - w.Event(progress.StartedEvent(eventName)) + err := s.apiClient().ContainerRestart(ctx, ctr.ID, containerType.StopOptions{Timeout: timeout}) + if err != nil { + return err } - return err + w.Event(progress.StartedEvent(eventName)) + return nil }) } return eg.Wait() diff --git a/pkg/compose/top.go b/pkg/compose/top.go index 1ac4cd4c4..b615870a1 100644 --- a/pkg/compose/top.go +++ b/pkg/compose/top.go @@ -36,16 +36,15 @@ func (s *composeService) Top(ctx context.Context, projectName string, services [ } summary := make([]api.ContainerProcSummary, len(containers)) eg, ctx := errgroup.WithContext(ctx) - for i, container := range containers { - i, container := i, container + for i, ctr := range containers { eg.Go(func() error { - topContent, err := s.apiClient().ContainerTop(ctx, container.ID, []string{}) + topContent, err := s.apiClient().ContainerTop(ctx, ctr.ID, []string{}) if err != nil { return err } summary[i] = api.ContainerProcSummary{ - ID: container.ID, - Name: getCanonicalContainerName(container), + ID: ctr.ID, + Name: getCanonicalContainerName(ctr), Processes: topContent.Processes, Titles: topContent.Titles, } diff --git a/pkg/compose/viz.go b/pkg/compose/viz.go index 02fe6ecf4..8b092adcc 100644 --- a/pkg/compose/viz.go +++ b/pkg/compose/viz.go @@ -31,7 +31,6 @@ type vizGraph map[*types.ServiceConfig][]*types.ServiceConfig func (s *composeService) Viz(_ context.Context, project *types.Project, opts api.VizOptions) (string, error) { graph := make(vizGraph) for _, service := range project.Services { - service := service graph[&service] = make([]*types.ServiceConfig, 0, len(service.DependsOn)) for dependencyName := range service.DependsOn { // no error should be returned since dependencyName should exist diff --git a/pkg/compose/wait.go b/pkg/compose/wait.go index ae0f5518e..e7628abd2 100644 --- a/pkg/compose/wait.go +++ b/pkg/compose/wait.go @@ -35,15 +35,14 @@ func (s *composeService) Wait(ctx context.Context, projectName string, options a eg, waitCtx := errgroup.WithContext(ctx) var statusCode int64 - for _, c := range containers { - c := c + for _, ctr := range containers { eg.Go(func() error { var err error - resultC, errC := s.dockerCli.Client().ContainerWait(waitCtx, c.ID, "") + resultC, errC := s.dockerCli.Client().ContainerWait(waitCtx, ctr.ID, "") select { case result := <-resultC: - _, _ = fmt.Fprintf(s.dockerCli.Out(), "container %q exited with status code %d\n", c.ID, result.StatusCode) + _, _ = fmt.Fprintf(s.dockerCli.Out(), "container %q exited with status code %d\n", ctr.ID, result.StatusCode) statusCode = result.StatusCode case err = <-errC: }