Merge pull request #2430 from tonistiigi/linter-updates

linter updates and gopls linting

.github/workflows/validate.yml

@@ -24,6 +24,7 @@ jobs:
       matrix:
         target:
           - lint
+          - lint-gopls
           - validate-vendor
           - validate-docs
           - validate-generated-files

.golangci.yml

@@ -25,6 +25,14 @@ linters:
   disable-all: true
 
 linters-settings:
+  govet:
+    enable:
+      - nilness
+      - unusedwrite
+  # enable-all: true
+  # disable:
+  #   - fieldalignment
+  #   - shadow
   depguard:
     rules:
       main:

Makefile

@@ -43,6 +43,10 @@ validate-all: lint test validate-vendor validate-docs validate-generated-files
 lint:
 	$(BUILDX_CMD) bake lint
 
+.PHONY: lint-gopls
+lint-gopls:
+	$(BUILDX_CMD) bake lint-gopls
+
 .PHONY: test
 test:
 	./hack/test

build/build.go

@@ -52,10 +52,8 @@ var (
 )
 
 const (
-	//nolint:gosec // G101: false-positive
 	printFallbackImage = "docker/dockerfile:1.5@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56"
 	// https://github.com/moby/buildkit/commit/71f99c52a669dc0322b5ea57bc28a09c20427227
-	//nolint:gosec // G101: false-positive
 	printLintFallbackImage = "docker.io/docker/dockerfile-upstream@sha256:47663570b6cc49ed90dc6e3215090a366989ab934d12dc93856a8ae0d27a95e7"
 )
 

build/invoke.go

@@ -37,7 +37,7 @@ func NewContainer(ctx context.Context, resultCtx *ResultHandle, cfg *controllera
 				cancel()
 			}()
 
-			containerCfg, err := resultCtx.getContainerConfig(ctx, c, cfg)
+			containerCfg, err := resultCtx.getContainerConfig(cfg)
 			if err != nil {
 				return nil, err
 			}

build/result.go

@@ -292,10 +292,10 @@ func (r *ResultHandle) build(buildFunc gateway.BuildFunc) (err error) {
 	return err
 }
 
-func (r *ResultHandle) getContainerConfig(ctx context.Context, c gateway.Client, cfg *controllerapi.InvokeConfig) (containerCfg gateway.NewContainerRequest, _ error) {
+func (r *ResultHandle) getContainerConfig(cfg *controllerapi.InvokeConfig) (containerCfg gateway.NewContainerRequest, _ error) {
 	if r.res != nil && r.solveErr == nil {
 		logrus.Debugf("creating container from successful build")
-		ccfg, err := containerConfigFromResult(ctx, r.res, c, *cfg)
+		ccfg, err := containerConfigFromResult(r.res, *cfg)
 		if err != nil {
 			return containerCfg, err
 		}
@@ -327,7 +327,7 @@ func (r *ResultHandle) getProcessConfig(cfg *controllerapi.InvokeConfig, stdin i
 	return processCfg, nil
 }
 
-func containerConfigFromResult(ctx context.Context, res *gateway.Result, c gateway.Client, cfg controllerapi.InvokeConfig) (*gateway.NewContainerRequest, error) {
+func containerConfigFromResult(res *gateway.Result, cfg controllerapi.InvokeConfig) (*gateway.NewContainerRequest, error) {
 	if cfg.Initial {
 		return nil, errors.Errorf("starting from the container from the initial state of the step is supported only on the failed steps")
 	}

builder/node.go

@@ -186,7 +186,7 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N
 						if pl := di.DriverInfo.DynamicNodes[i].Platforms; len(pl) > 0 {
 							diClone.Platforms = pl
 						}
-						nodes = append(nodes, di)
+						nodes = append(nodes, diClone)
 					}
 					dynamicNodes = append(dynamicNodes, di.DriverInfo.DynamicNodes...)
 				}

commands/build.go

@@ -340,7 +340,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 	if confutil.IsExperimental() {
 		resp, retErr = runControllerBuild(ctx, dockerCli, opts, options, printer)
 	} else {
-		resp, retErr = runBasicBuild(ctx, dockerCli, opts, options, printer)
+		resp, retErr = runBasicBuild(ctx, dockerCli, opts, printer)
 	}
 
 	if err := printer.Wait(); retErr == nil {
@@ -387,7 +387,7 @@ func getImageID(resp map[string]string) string {
 	return dgst
 }
 
-func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
+func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
 	resp, res, err := cbuild.RunBuild(ctx, dockerCli, *opts, dockerCli.In(), printer, false)
 	if res != nil {
 		res.Done()

commands/install.go

@@ -15,7 +15,7 @@ import (
 type installOptions struct {
 }
 
-func runInstall(dockerCli command.Cli, in installOptions) error {
+func runInstall(_ command.Cli, _ installOptions) error {
 	dir := config.Dir()
 	if err := os.MkdirAll(dir, 0755); err != nil {
 		return errors.Wrap(err, "could not create docker config")

commands/uninstall.go

@@ -15,7 +15,7 @@ import (
 type uninstallOptions struct {
 }
 
-func runUninstall(dockerCli command.Cli, in uninstallOptions) error {
+func runUninstall(_ command.Cli, _ uninstallOptions) error {
 	dir := config.Dir()
 	cfg, err := config.Load(dir)
 	if err != nil {

commands/version.go

@@ -11,7 +11,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func runVersion(dockerCli command.Cli) error {
+func runVersion(_ command.Cli) error {
 	fmt.Println(version.Package, version.Version, version.Revision)
 	return nil
 }

controller/build/build.go

@@ -189,7 +189,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
 		return nil, nil, err
 	}
 
-	resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progress, generateResult)
+	resp, res, err := buildTargets(ctx, dockerCli, nodes, map[string]build.Options{defaultTargetName: opts}, progress, generateResult)
 	err = wrapBuildError(err, false)
 	if err != nil {
 		// NOTE: buildTargets can return *build.ResultHandle even on error.
@@ -203,7 +203,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
 // NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle,
 // this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
 // inspect the result and debug the cause of that error.
-func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
+func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
 	var res *build.ResultHandle
 	var resp map[string]*client.SolveResponse
 	var err error

controller/remote/client.go

@@ -210,7 +210,7 @@ func (c *Client) build(ctx context.Context, ref string, options pb.BuildOptions,
 						}
 						return err
 					} else if n > 0 {
-						if stream.Send(&pb.InputMessage{
+						if err := stream.Send(&pb.InputMessage{
 							Input: &pb.InputMessage_Data{
 								Data: &pb.DataMessage{
 									Data: buf[:n],

controller/remote/io.go

@@ -358,7 +358,7 @@ func copyToStream(fd uint32, snd msgStream, r io.Reader) error {
 			}
 			return err
 		} else if n > 0 {
-			if snd.Send(&pb.Message{
+			if err := snd.Send(&pb.Message{
 				Input: &pb.Message_File{
 					File: &pb.FdMessage{
 						Fd:   fd,

docker-bake.hcl

@@ -28,7 +28,7 @@ group "default" {
 }
 
 group "validate" {
-  targets = ["lint", "validate-vendor", "validate-docs"]
+  targets = ["lint", "lint-gopls", "validate-vendor", "validate-docs"]
 }
 
 target "lint" {
@@ -48,6 +48,11 @@ target "lint" {
   ] : []
 }
 
+target "lint-gopls" {
+  inherits = ["lint"]
+  target = "gopls-analyze"
+}
+
 target "validate-vendor" {
   inherits = ["_common"]
   dockerfile = "./hack/dockerfiles/vendor.Dockerfile"

driver/docker-container/driver.go

@@ -77,7 +77,7 @@ func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error {
 			return err
 		}
 		return sub.Wrap("starting container "+d.Name, func() error {
-			if err := d.start(ctx, sub); err != nil {
+			if err := d.start(ctx); err != nil {
 				return err
 			}
 			return d.wait(ctx, sub)
@@ -188,7 +188,7 @@ func (d *Driver) create(ctx context.Context, l progress.SubLogger) error {
 			if err := d.copyToContainer(ctx, d.InitConfig.Files); err != nil {
 				return err
 			}
-			if err := d.start(ctx, l); err != nil {
+			if err := d.start(ctx); err != nil {
 				return err
 			}
 		}
@@ -203,14 +203,12 @@ func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error {
 		bufStderr := &bytes.Buffer{}
 		if err := d.run(ctx, []string{"buildctl", "debug", "workers"}, bufStdout, bufStderr); err != nil {
 			if try > 15 {
-				if err != nil {
+				d.copyLogs(context.TODO(), l)
-					d.copyLogs(context.TODO(), l)
+				if bufStdout.Len() != 0 {
-					if bufStdout.Len() != 0 {
+					l.Log(1, bufStdout.Bytes())
-						l.Log(1, bufStdout.Bytes())
+				}
-					}
+				if bufStderr.Len() != 0 {
-					if bufStderr.Len() != 0 {
+					l.Log(2, bufStderr.Bytes())
-						l.Log(2, bufStderr.Bytes())
-					}
 				}
 				return err
 			}
@@ -304,7 +302,7 @@ func (d *Driver) run(ctx context.Context, cmd []string, stdout, stderr io.Writer
 	return nil
 }
 
-func (d *Driver) start(ctx context.Context, l progress.SubLogger) error {
+func (d *Driver) start(ctx context.Context) error {
 	return d.DockerAPI.ContainerStart(ctx, d.Name, container.StartOptions{})
 }
 

hack/dockerfiles/lint.Dockerfile

@@ -2,11 +2,18 @@
 
 ARG GO_VERSION=1.21
 ARG XX_VERSION=1.3.0
-ARG GOLANGCI_LINT_VERSION=1.54.2
+ARG GOLANGCI_LINT_VERSION=1.57.2
+ARG GOPLS_VERSION=v0.20.0
+# disabled: deprecated unusedvariable simplifyrange
+ARG GOPLS_ANALYZERS="embeddirective fillreturns infertypeargs nonewvars noresultvalues simplifycompositelit simplifyslice stubmethods undeclaredname unusedparams useany"
+
 
 FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx
-FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine
+
+FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine AS golang-base
 RUN apk add --no-cache git gcc musl-dev
+
+FROM golang-base AS lint
 ENV GOFLAGS="-buildvcs=false"
 ARG GOLANGCI_LINT_VERSION
 RUN wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v${GOLANGCI_LINT_VERSION}
@@ -17,3 +24,49 @@ RUN --mount=target=/go/src/github.com/docker/buildx \
     --mount=target=/root/.cache,type=cache,id=lint-cache-$TARGETPLATFORM \
     xx-go --wrap && \
     golangci-lint run
+
+FROM golang-base AS gopls
+RUN apk add --no-cache git
+ARG GOPLS_VERSION
+WORKDIR /src
+RUN git clone https://github.com/golang/tools.git && \
+  cd tools && git checkout ${GOPLS_VERSION}
+WORKDIR tools/gopls
+ARG GOPLS_ANALYZERS
+RUN <<'EOF'
+  set -ex
+  mkdir -p /out
+  for analyzer in ${GOPLS_ANALYZERS}; do
+    mkdir -p internal/cmd/$analyzer
+    cat <<eot > internal/cmd/$analyzer/main.go
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/singlechecker"
+	analyzer "golang.org/x/tools/gopls/internal/analysis/$analyzer"
+)
+
+func main() { singlechecker.Main(analyzer.Analyzer) }
+eot
+    echo "Analyzing with ${analyzer}..."
+    go build -o /out/$analyzer ./internal/cmd/$analyzer
+  done
+EOF
+
+FROM golang-base AS gopls-analyze
+COPY --link --from=xx / /
+ARG GOPLS_ANALYZERS
+ARG TARGETNAME
+ARG TARGETPLATFORM
+WORKDIR /go/src/github.com/docker/buildx
+RUN --mount=target=. \
+  --mount=target=/root/.cache,type=cache,id=lint-cache-${TARGETNAME}-${TARGETPLATFORM} \
+  --mount=target=/gopls-analyzers,from=gopls,source=/out <<EOF
+  set -ex
+  xx-go --wrap
+  for analyzer in ${GOPLS_ANALYZERS}; do
+    go vet -vettool=/gopls-analyzers/$analyzer ./...
+  done
+EOF
+
+FROM lint

tests/workers/docker.go

@@ -60,12 +60,9 @@ func (c dockerWorker) New(ctx context.Context, cfg *integration.BackendConfig) (
 	}
 
 	cl = func() error {
-		var err error
+		err := bkclose()
-		if err1 := bkclose(); err == nil {
-			err = err1
-		}
 		cmd := exec.Command("docker", "context", "rm", "-f", name)
-		if err1 := cmd.Run(); err1 != nil {
+		if err1 := cmd.Run(); err == nil {
 			err = errors.Wrapf(err1, "failed to remove buildx instance %s", name)
 		}
 		return err

tests/workers/remote.go

@@ -54,10 +54,7 @@ func (w remoteWorker) New(ctx context.Context, cfg *integration.BackendConfig) (
 	}
 
 	cl = func() error {
-		var err error
+		err := bkclose()
-		if err1 := bkclose(); err == nil {
-			err = err1
-		}
 		cmd := exec.Command("buildx", "rm", "-f", name)
 		if err1 := cmd.Run(); err == nil {
 			err = err1

util/gitutil/path_windows.go

@@ -4,6 +4,6 @@ import (
 	"os/exec"
 )
 
-func gitPath(wd string) (string, error) {
+func gitPath(_ string) (string, error) {
 	return exec.LookPath("git.exe")
 }

util/ioset/mux_test.go

@@ -120,14 +120,14 @@ func TestMuxIO(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			inBuf, end, in := newTestIn(t)
+			inBuf, end, in := newTestIn()
 			var outBufs []*outBuf
 			var outs []MuxOut
 			if tt.outputsNum != len(tt.wants) {
 				t.Fatalf("wants != outputsNum")
 			}
 			for i := 0; i < tt.outputsNum; i++ {
-				outBuf, out := newTestOut(t, i)
+				outBuf, out := newTestOut(i)
 				outBufs = append(outBufs, outBuf)
 				outs = append(outs, MuxOut{out, nil, nil})
 			}
@@ -223,7 +223,7 @@ type inBuf struct {
 	doneCh chan struct{}
 }
 
-func newTestIn(t *testing.T) (*inBuf, Out, In) {
+func newTestIn() (*inBuf, Out, In) {
 	ti := &inBuf{
 		doneCh: make(chan struct{}),
 	}
@@ -262,7 +262,7 @@ type outBuf struct {
 	doneCh chan struct{}
 }
 
-func newTestOut(t *testing.T, idx int) (*outBuf, Out) {
+func newTestOut(idx int) (*outBuf, Out) {
 	to := &outBuf{
 		idx:    idx,
 		doneCh: make(chan struct{}),
@@ -285,7 +285,7 @@ func newTestOut(t *testing.T, idx int) (*outBuf, Out) {
 			errW.CloseWithError(err)
 			return
 		}
-		to.stdin = string(buf.Bytes())
+		to.stdin = buf.String()
 		outW.Close()
 		errW.Close()
 		close(to.doneCh)