Merge pull request #2798 from tonistiigi/linter-updates

Improve linter checks
docker · Nov 20, 2024 · 26bbddb · 26bbddb
2 parents a6ef9db + 58fd190
commit 26bbddb
Show file tree

Hide file tree

Showing 34 changed files with 125 additions and 102 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -5,22 +5,45 @@ run:
 
 linters:
   enable:
-    - gofmt
-    - govet
+    - bodyclose
     - depguard
+    - forbidigo
+    - gocritic
+    - gofmt
     - goimports
+    - gosec
+    - gosimple
+    - govet
     - ineffassign
+    - makezero
     - misspell
-    - unused
+    - noctx
+    - nolintlint
     - revive
     - staticcheck
     - typecheck
-    - nolintlint
-    - gosec
-    - forbidigo
+    - unused
+    - whitespace
   disable-all: true
 
 linters-settings:
+  gocritic:
+    disabled-checks:
+      - "ifElseChain"
+      - "assignOp"
+      - "appendAssign"
+      - "singleCaseSwitch"
+      - "exitAfterDefer" # FIXME
+  importas:
+    alias:
+      # Enforce alias to prevent it accidentally being used instead of
+      # buildkit errdefs package (or vice-versa).
+      - pkg: "github.com/containerd/errdefs"
+        alias: "cerrdefs"
+      - pkg: "github.com/opencontainers/image-spec/specs-go/v1"
+        alias: "ocispecs"
+      - pkg: "github.com/opencontainers/go-digest"
+        alias: "digest"
   govet:
     enable:
       - nilness
@@ -43,6 +66,10 @@ linters-settings:
             desc: The io/ioutil package has been deprecated.
   forbidigo:
     forbid:
+      - '^context\.WithCancel(# use context\.WithCancelCause instead)?$'
+      - '^context\.WithDeadline(# use context\.WithDeadline instead)?$'
+      - '^context\.WithTimeout(# use context\.WithTimeoutCause instead)?$'
+      - '^ctx\.Err(# use context\.Cause instead)?$'
       - '^fmt\.Errorf(# use errors\.Errorf instead)?$'
       - '^platforms\.DefaultString(# use platforms\.Format(platforms\.DefaultSpec()) instead\.)?$'
   gosec:

diff --git a/bake/compose.go b/bake/compose.go
@@ -179,7 +179,6 @@ func ParseCompose(cfgs []composetypes.ConfigFile, envs map[string]string) (*Conf
 			c.Targets = append(c.Targets, t)
 		}
 		c.Groups = append(c.Groups, g)
-
 	}
 
 	return &c, nil

diff --git a/bake/hclparser/stdlib.go b/bake/hclparser/stdlib.go
@@ -170,7 +170,6 @@ func indexOfFunc() function.Function {
 				}
 			}
 			return cty.NilVal, errors.New("item not found")
-
 		},
 	})
 }

diff --git a/build/invoke.go b/build/invoke.go
@@ -16,7 +16,7 @@ import (
 
 type Container struct {
 	cancelOnce      sync.Once
-	containerCancel func()
+	containerCancel func(error)
 	isUnavailable   atomic.Bool
 	initStarted     atomic.Bool
 	container       gateway.Container
@@ -31,18 +31,18 @@ func NewContainer(ctx context.Context, resultCtx *ResultHandle, cfg *controllera
 	errCh := make(chan error)
 	go func() {
 		err := resultCtx.build(func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
-			ctx, cancel := context.WithCancel(ctx)
+			ctx, cancel := context.WithCancelCause(ctx)
 			go func() {
 				<-mainCtx.Done()
-				cancel()
+				cancel(errors.WithStack(context.Canceled))
 			}()
 
 			containerCfg, err := resultCtx.getContainerConfig(cfg)
 			if err != nil {
 				return nil, err
 			}
-			containerCtx, containerCancel := context.WithCancel(ctx)
-			defer containerCancel()
+			containerCtx, containerCancel := context.WithCancelCause(ctx)
+			defer containerCancel(errors.WithStack(context.Canceled))
 			bkContainer, err := c.NewContainer(containerCtx, containerCfg)
 			if err != nil {
 				return nil, err
@@ -83,7 +83,7 @@ func (c *Container) Cancel() {
 	c.markUnavailable()
 	c.cancelOnce.Do(func() {
 		if c.containerCancel != nil {
-			c.containerCancel()
+			c.containerCancel(errors.WithStack(context.Canceled))
 		}
 		close(c.releaseCh)
 	})

diff --git a/build/result.go b/build/result.go
@@ -82,7 +82,7 @@ func NewResultHandle(ctx context.Context, cc *client.Client, opt client.SolveOpt
 	var respHandle *ResultHandle
 
 	go func() {
-		defer cancel(context.Canceled) // ensure no dangling processes
+		defer func() { cancel(errors.WithStack(context.Canceled)) }() // ensure no dangling processes
 
 		var res *gateway.Result
 		var err error
@@ -181,7 +181,7 @@ func NewResultHandle(ctx context.Context, cc *client.Client, opt client.SolveOpt
 			case <-respHandle.done:
 			case <-ctx.Done():
 			}
-			return nil, ctx.Err()
+			return nil, context.Cause(ctx)
 		}, nil)
 		if respHandle != nil {
 			return

diff --git a/builder/builder.go b/builder/builder.go
@@ -288,7 +288,15 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) {
 		return nil, err
 	}
 
-	builders := make([]*Builder, len(storeng))
+	contexts, err := dockerCli.ContextStore().List()
+	if err != nil {
+		return nil, err
+	}
+	sort.Slice(contexts, func(i, j int) bool {
+		return contexts[i].Name < contexts[j].Name
+	})
+
+	builders := make([]*Builder, len(storeng), len(storeng)+len(contexts))
 	seen := make(map[string]struct{})
 	for i, ng := range storeng {
 		b, err := New(dockerCli,
@@ -303,14 +311,6 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) {
 		seen[b.NodeGroup.Name] = struct{}{}
 	}
 
-	contexts, err := dockerCli.ContextStore().List()
-	if err != nil {
-		return nil, err
-	}
-	sort.Slice(contexts, func(i, j int) bool {
-		return contexts[i].Name < contexts[j].Name
-	})
-
 	for _, c := range contexts {
 		// if a context has the same name as an instance from the store, do not
 		// add it to the builders list. An instance from the store takes
@@ -522,8 +522,9 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
 		return nil, err
 	}
 
-	timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
-	defer cancel()
+	cancelCtx, cancel := context.WithCancelCause(ctx)
+	timeoutCtx, _ := context.WithTimeoutCause(cancelCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 
 	nodes, err := b.LoadNodes(timeoutCtx, WithData())
 	if err != nil {

diff --git a/commands/bake.go b/commands/bake.go
@@ -108,8 +108,8 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba
 		return err
 	}
 
-	ctx2, cancel := context.WithCancel(context.TODO())
-	defer cancel()
+	ctx2, cancel := context.WithCancelCause(context.TODO())
+	defer cancel(errors.WithStack(context.Canceled))
 
 	var nodes []builder.Node
 	var progressConsoleDesc, progressTextDesc string

diff --git a/commands/build.go b/commands/build.go
@@ -325,8 +325,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 	}
 	attributes := buildMetricAttributes(dockerCli, driverType, &options)
 
-	ctx2, cancel := context.WithCancel(context.TODO())
-	defer cancel()
+	ctx2, cancel := context.WithCancelCause(context.TODO())
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 	progressMode, err := options.toDisplayMode()
 	if err != nil {
 		return err
@@ -885,7 +885,6 @@ func printWarnings(w io.Writer, warnings []client.VertexWarning, mode progressui
 			src.Print(w)
 		}
 		fmt.Fprintf(w, "\n")
-
 	}
 }
 

diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go
@@ -42,7 +42,7 @@ func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, arg
 		return errors.Errorf("can't push with no tags specified, please set --tag or --dry-run")
 	}
 
-	fileArgs := make([]string, len(in.files))
+	fileArgs := make([]string, len(in.files), len(in.files)+len(args))
 	for i, f := range in.files {
 		dt, err := os.ReadFile(f)
 		if err != nil {
@@ -173,8 +173,8 @@ func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, arg
 	// new resolver cause need new auth
 	r = imagetools.New(imageopt)
 
-	ctx2, cancel := context.WithCancel(context.TODO())
-	defer cancel()
+	ctx2, cancel := context.WithCancelCause(context.TODO())
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 	printer, err := progress.NewPrinter(ctx2, os.Stderr, progressui.DisplayMode(in.progress))
 	if err != nil {
 		return err

diff --git a/commands/inspect.go b/commands/inspect.go
@@ -17,6 +17,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/debug"
 	"github.com/docker/go-units"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -34,8 +35,9 @@ func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) e
 		return err
 	}
 
-	timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
-	defer cancel()
+	timeoutCtx, cancel := context.WithCancelCause(ctx)
+	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 
 	nodes, err := b.LoadNodes(timeoutCtx, builder.WithData())
 	if in.bootstrap {

diff --git a/commands/ls.go b/commands/ls.go
@@ -18,6 +18,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/formatter"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
 )
@@ -57,8 +58,9 @@ func runLs(ctx context.Context, dockerCli command.Cli, in lsOptions) error {
 		return err
 	}
 
-	timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
-	defer cancel()
+	timeoutCtx, cancel := context.WithCancelCause(ctx)
+	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 
 	eg, _ := errgroup.WithContext(timeoutCtx)
 	for _, b := range builders {
@@ -319,7 +321,7 @@ func (tp truncatedPlatforms) String() string {
 		if tpf, ok := tp.res[mpf]; ok {
 			seen[mpf] = struct{}{}
 			if len(tpf) == 1 {
-				out = append(out, fmt.Sprintf("%s", tpf[0]))
+				out = append(out, tpf[0])
 				count++
 			} else {
 				hasPreferredPlatform := false
@@ -347,7 +349,7 @@ func (tp truncatedPlatforms) String() string {
 			continue
 		}
 		if len(tp.res[mpf]) == 1 {
-			out = append(out, fmt.Sprintf("%s", tp.res[mpf][0]))
+			out = append(out, tp.res[mpf][0])
 			count++
 		} else {
 			hasPreferredPlatform := false

diff --git a/commands/rm.go b/commands/rm.go
@@ -150,8 +150,9 @@ func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, i
 		return err
 	}
 
-	timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
-	defer cancel()
+	timeoutCtx, cancel := context.WithCancelCause(ctx)
+	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
+	defer func() { cancel(errors.WithStack(context.Canceled)) }()
 
 	eg, _ := errgroup.WithContext(timeoutCtx)
 	for _, b := range builders {

diff --git a/commands/use.go b/commands/use.go
@@ -46,7 +46,6 @@ func runUse(dockerCli command.Cli, in useOptions) error {
 					return errors.Errorf("run `docker context use %s` to switch to context %s", in.builder, in.builder)
 				}
 			}
-
 		}
 		return errors.Wrapf(err, "failed to find instance %q", in.builder)
 	}

diff --git a/controller/local/controller.go b/controller/local/controller.go
@@ -109,15 +109,15 @@ func (b *localController) Invoke(ctx context.Context, sessionID string, pid stri
 
 	// Attach containerIn to this process
 	ioCancelledCh := make(chan struct{})
-	proc.ForwardIO(&ioset.In{Stdin: ioIn, Stdout: ioOut, Stderr: ioErr}, func() { close(ioCancelledCh) })
+	proc.ForwardIO(&ioset.In{Stdin: ioIn, Stdout: ioOut, Stderr: ioErr}, func(error) { close(ioCancelledCh) })
 
 	select {
 	case <-ioCancelledCh:
 		return errors.Errorf("io cancelled")
 	case err := <-proc.Done():
 		return err
 	case <-ctx.Done():
-		return ctx.Err()
+		return context.Cause(ctx)
 	}
 }
 

diff --git a/controller/pb/path.go b/controller/pb/path.go
@@ -153,7 +153,6 @@ func ResolveOptionPaths(options *BuildOptions) (_ *BuildOptions, err error) {
 				}
 			}
 			ps = append(ps, p)
-
 		}
 		s.Paths = ps
 		ssh = append(ssh, s)

diff --git a/controller/pb/progress.go b/controller/pb/progress.go
@@ -22,9 +22,7 @@ func (w *writer) Write(status *client.SolveStatus) {
 	w.ch <- ToControlStatus(status)
 }
 
-func (w *writer) WriteBuildRef(target string, ref string) {
-	return
-}
+func (w *writer) WriteBuildRef(target string, ref string) {}
 
 func (w *writer) ValidateLogSource(digest.Digest, interface{}) bool {
 	return true

diff --git a/controller/processes/processes.go b/controller/processes/processes.go
@@ -18,16 +18,16 @@ type Process struct {
 	invokeConfig  *pb.InvokeConfig
 	errCh         chan error
 	processCancel func()
-	serveIOCancel func()
+	serveIOCancel func(error)
 }
 
 // ForwardIO forwards process's io to the specified reader/writer.
 // Optionally specify ioCancelCallback which will be called when
 // the process closes the specified IO. This will be useful for additional cleanup.
-func (p *Process) ForwardIO(in *ioset.In, ioCancelCallback func()) {
+func (p *Process) ForwardIO(in *ioset.In, ioCancelCallback func(error)) {
 	p.inEnd.SetIn(in)
 	if f := p.serveIOCancel; f != nil {
-		f()
+		f(errors.WithStack(context.Canceled))
 	}
 	p.serveIOCancel = ioCancelCallback
 }
@@ -124,9 +124,16 @@ func (m *Manager) StartProcess(pid string, resultCtx *build.ResultHandle, cfg *p
 	f.SetOut(&out)
 
 	// Register process
-	ctx, cancel := context.WithCancel(context.TODO())
+	ctx, cancel := context.WithCancelCause(context.TODO())
 	var cancelOnce sync.Once
-	processCancelFunc := func() { cancelOnce.Do(func() { cancel(); f.Close(); in.Close(); out.Close() }) }
+	processCancelFunc := func() {
+		cancelOnce.Do(func() {
+			cancel(errors.WithStack(context.Canceled))
+			f.Close()
+			in.Close()
+			out.Close()
+		})
+	}
 	p := &Process{
 		inEnd:         f,
 		invokeConfig:  cfg,