From a2c9f3c6ce661e56d01a69e82da0fc8ed8dfb713 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 20 Nov 2023 13:54:53 +0100 Subject: [PATCH] linting: address else/if/elseif statements found by gocritic cli/command/formatter/tabwriter/tabwriter.go:579:10: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/connhelper/connhelper.go:43:2: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch scheme := u.Scheme; scheme { ^ cli/compose/loader/loader.go:666:10: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ opts/hosts_test.go:173:10: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli-plugins/manager/candidate_test.go:78:4: ifElseChain: rewrite if-else to switch statement (gocritic) if tc.err != "" { ^ cli/command/checkpoint/formatter.go:15:2: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch source { ^ cli/command/image/formatter_history.go:25:2: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch source { ^ cli/command/service/scale.go:107:2: ifElseChain: rewrite if-else to switch statement (gocritic) if serviceMode.Replicated != nil { ^ cli/command/service/update.go:804:9: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/command/service/update.go:222:2: ifElseChain: rewrite if-else to switch statement (gocritic) if sendAuth { ^ cli/command/container/formatter_diff.go:17:2: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch source { ^ cli/command/container/start.go:79:2: ifElseChain: rewrite if-else to switch statement (gocritic) if opts.Attach || opts.OpenStdin { ^ cli/command/container/utils.go:84:11: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/command/container/exec_test.go:200:11: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/command/container/logs_test.go:52:11: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/command/container/opts_test.go:1014:10: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ cli/command/system/info.go:297:7: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch o.Key { ^ cli/command/system/version.go:164:4: singleCaseSwitch: should rewrite switch statement to if statement (gocritic) switch component.Name { ^ cli/command/system/info_test.go:478:4: ifElseChain: rewrite if-else to switch statement (gocritic) if tc.expectedOut != "" { ^ Signed-off-by: Sebastiaan van Stijn --- cli-plugins/manager/candidate_test.go | 7 ++++--- cli/command/checkpoint/formatter.go | 3 +-- cli/command/container/exec_test.go | 6 ++---- cli/command/container/formatter_diff.go | 3 +-- cli/command/container/logs_test.go | 6 ++---- cli/command/container/opts_test.go | 6 ++---- cli/command/container/start.go | 11 +++++------ cli/command/container/utils.go | 18 ++++++++---------- cli/command/formatter/tabwriter/tabwriter.go | 18 ++++++++---------- cli/command/image/formatter_history.go | 3 +-- cli/command/service/scale.go | 7 ++++--- cli/command/service/update.go | 9 +++++---- cli/command/system/info.go | 3 +-- cli/command/system/info_test.go | 7 ++++--- cli/command/system/version.go | 3 +-- cli/compose/loader/loader.go | 6 ++---- cli/connhelper/connhelper.go | 3 +-- opts/hosts_test.go | 6 ++---- 18 files changed, 54 insertions(+), 71 deletions(-) diff --git a/cli-plugins/manager/candidate_test.go b/cli-plugins/manager/candidate_test.go index 0ec7beda64..acf52e54ff 100644 --- a/cli-plugins/manager/candidate_test.go +++ b/cli-plugins/manager/candidate_test.go @@ -75,13 +75,14 @@ func TestValidateCandidate(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { p, err := newPlugin(tc.c, fakeroot.Commands()) - if tc.err != "" { + switch { + case tc.err != "": assert.ErrorContains(t, err, tc.err) - } else if tc.invalid != "" { + case tc.invalid != "": assert.NilError(t, err) assert.Assert(t, cmp.ErrorType(p.Err, reflect.TypeOf(&pluginError{}))) assert.ErrorContains(t, p.Err, tc.invalid) - } else { + default: assert.NilError(t, err) assert.Equal(t, NamePrefix+p.Name, goodPluginName) assert.Equal(t, p.SchemaVersion, "0.1.0") diff --git a/cli/command/checkpoint/formatter.go b/cli/command/checkpoint/formatter.go index e7d252100f..47ee77635b 100644 --- a/cli/command/checkpoint/formatter.go +++ b/cli/command/checkpoint/formatter.go @@ -12,8 +12,7 @@ const ( // NewFormat returns a format for use with a checkpoint Context func NewFormat(source string) formatter.Format { - switch source { - case formatter.TableFormatKey: + if source == formatter.TableFormatKey { return defaultCheckpointFormat } return formatter.Format(source) diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index e1ff1e61b4..2d79590ab8 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -197,10 +197,8 @@ func TestRunExec(t *testing.T) { err := RunExec(context.Background(), cli, "thecontainer", testcase.options) if testcase.expectedError != "" { assert.ErrorContains(t, err, testcase.expectedError) - } else { - if !assert.Check(t, err) { - return - } + } else if !assert.Check(t, err) { + return } assert.Check(t, is.Equal(testcase.expectedOut, cli.OutBuffer().String())) assert.Check(t, is.Equal(testcase.expectedErr, cli.ErrBuffer().String())) diff --git a/cli/command/container/formatter_diff.go b/cli/command/container/formatter_diff.go index 8baf11bfa0..822e1eef51 100644 --- a/cli/command/container/formatter_diff.go +++ b/cli/command/container/formatter_diff.go @@ -14,8 +14,7 @@ const ( // NewDiffFormat returns a format for use with a diff Context func NewDiffFormat(source string) formatter.Format { - switch source { - case formatter.TableFormatKey: + if source == formatter.TableFormatKey { return defaultDiffTableFormat } return formatter.Format(source) diff --git a/cli/command/container/logs_test.go b/cli/command/container/logs_test.go index d615badd96..a12fbde37e 100644 --- a/cli/command/container/logs_test.go +++ b/cli/command/container/logs_test.go @@ -49,10 +49,8 @@ func TestRunLogs(t *testing.T) { err := runLogs(cli, testcase.options) if testcase.expectedError != "" { assert.ErrorContains(t, err, testcase.expectedError) - } else { - if !assert.Check(t, err) { - return - } + } else if !assert.Check(t, err) { + return } assert.Check(t, is.Equal(testcase.expectedOut, cli.OutBuffer().String())) assert.Check(t, is.Equal(testcase.expectedErr, cli.ErrBuffer().String())) diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index a665c81169..89fa2d5838 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -1011,10 +1011,8 @@ func TestValidateDevice(t *testing.T) { for path, expectedError := range invalid { if _, err := validateDevice(path, runtime.GOOS); err == nil { t.Fatalf("ValidateDevice(`%q`) should have failed validation", path) - } else { - if err.Error() != expectedError { - t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) - } + } else if err.Error() != expectedError { + t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) } } } diff --git a/cli/command/container/start.go b/cli/command/container/start.go index f4a6421f5c..f750e36b5d 100644 --- a/cli/command/container/start.go +++ b/cli/command/container/start.go @@ -76,7 +76,8 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { ctx, cancelFun := context.WithCancel(context.Background()) defer cancelFun() - if opts.Attach || opts.OpenStdin { + switch { + case opts.Attach || opts.OpenStdin: // We're going to attach to a container. // 1. Ensure we only have one container. if len(opts.Containers) > 1 { @@ -180,7 +181,8 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { if status := <-statusChan; status != 0 { return cli.StatusError{StatusCode: status} } - } else if opts.Checkpoint != "" { + return nil + case opts.Checkpoint != "": if len(opts.Containers) > 1 { return errors.New("you cannot restore multiple containers at once") } @@ -189,14 +191,11 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { CheckpointID: opts.Checkpoint, CheckpointDir: opts.CheckpointDir, }) - - } else { + default: // We're not going to attach to anything. // Start as many containers as we want. return startContainersWithoutAttachments(ctx, dockerCli, opts.Containers) } - - return nil } func startContainersWithoutAttachments(ctx context.Context, dockerCli command.Cli, containers []string) error { diff --git a/cli/command/container/utils.go b/cli/command/container/utils.go index bbe9724f96..8a94a182cd 100644 --- a/cli/command/container/utils.go +++ b/cli/command/container/utils.go @@ -81,18 +81,16 @@ func legacyWaitExitOrRemoved(ctx context.Context, apiClient client.APIClient, co } if !waitRemove { stopProcessing = true - } else { + } else if versions.LessThan(apiClient.ClientVersion(), "1.25") { // If we are talking to an older daemon, `AutoRemove` is not supported. // We need to fall back to the old behavior, which is client-side removal - if versions.LessThan(apiClient.ClientVersion(), "1.25") { - go func() { - removeErr = apiClient.ContainerRemove(ctx, containerID, container.RemoveOptions{RemoveVolumes: true}) - if removeErr != nil { - logrus.Errorf("error removing container: %v", removeErr) - cancel() // cancel the event Q - } - }() - } + go func() { + removeErr = apiClient.ContainerRemove(ctx, containerID, container.RemoveOptions{RemoveVolumes: true}) + if removeErr != nil { + logrus.Errorf("error removing container: %v", removeErr) + cancel() // cancel the event Q + } + }() } case "detach": exitCode = 0 diff --git a/cli/command/formatter/tabwriter/tabwriter.go b/cli/command/formatter/tabwriter/tabwriter.go index b5f6793ac4..079be60c83 100644 --- a/cli/command/formatter/tabwriter/tabwriter.go +++ b/cli/command/formatter/tabwriter/tabwriter.go @@ -576,18 +576,16 @@ func (b *Writer) Write(buf []byte) (n int, err error) { b.startEscape(ch) } } - } else { + } else if ch == b.endChar { // inside escape - if ch == b.endChar { - // end of tag/entity - j := i + 1 - if ch == Escape && b.flags&StripEscape != 0 { - j = i // strip Escape - } - b.append(buf[n:j]) - n = i + 1 // ch consumed - b.endEscape() + // end of tag/entity + j := i + 1 + if ch == Escape && b.flags&StripEscape != 0 { + j = i // strip Escape } + b.append(buf[n:j]) + n = i + 1 // ch consumed + b.endEscape() } } diff --git a/cli/command/image/formatter_history.go b/cli/command/image/formatter_history.go index a36ab42576..08b7febe3b 100644 --- a/cli/command/image/formatter_history.go +++ b/cli/command/image/formatter_history.go @@ -22,8 +22,7 @@ const ( // NewHistoryFormat returns a format for rendering an HistoryContext func NewHistoryFormat(source string, quiet bool, human bool) formatter.Format { - switch source { - case formatter.TableFormatKey: + if source == formatter.TableFormatKey { switch { case quiet: return formatter.DefaultQuietFormat diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go index f2625445eb..bdc53ae401 100644 --- a/cli/command/service/scale.go +++ b/cli/command/service/scale.go @@ -104,11 +104,12 @@ func runServiceScale(ctx context.Context, dockerCli command.Cli, serviceID strin } serviceMode := &service.Spec.Mode - if serviceMode.Replicated != nil { + switch { + case serviceMode.Replicated != nil: serviceMode.Replicated.Replicas = &scale - } else if serviceMode.ReplicatedJob != nil { + case serviceMode.ReplicatedJob != nil: serviceMode.ReplicatedJob.TotalCompletions = &scale - } else { + default: return errors.Errorf("scale can only be used with replicated or replicated-job mode") } diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 938edfc405..c4672493aa 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -219,7 +219,8 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOpti if err != nil { return err } - if sendAuth { + switch { + case sendAuth: // Retrieve encoded auth token from the image reference // This would be the old image if it didn't change in this update image := spec.TaskTemplate.ContainerSpec.Image @@ -228,9 +229,9 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOpti return err } updateOpts.EncodedRegistryAuth = encodedAuth - } else if clientSideRollback { + case clientSideRollback: updateOpts.RegistryAuthFrom = types.RegistryAuthFromPreviousSpec - } else { + default: updateOpts.RegistryAuthFrom = types.RegistryAuthFromSpec } @@ -801,7 +802,7 @@ func getUpdatedConfigs(apiClient client.ConfigAPIClient, flags *pflag.FlagSet, s if flags.Changed(flagCredentialSpec) { credSpec := flags.Lookup(flagCredentialSpec).Value.(*credentialSpecOpt).Value() credSpecConfigName = credSpec.Config - } else { + } else { //nolint:gocritic // ignore elseif: can replace 'else {if cond {}}' with 'else if cond {}' // if the credential spec flag has not changed, then check if there // already is a credentialSpec. if there is one, and it's for a Config, // then it's from the old object, and its value is the config ID. we diff --git a/cli/command/system/info.go b/cli/command/system/info.go index 6de1fcecd3..25bca56daf 100644 --- a/cli/command/system/info.go +++ b/cli/command/system/info.go @@ -294,8 +294,7 @@ func prettyPrintServerInfo(streams command.Streams, info *info) []error { for _, so := range kvs { fprintln(output, " "+so.Name) for _, o := range so.Options { - switch o.Key { - case "profile": + if o.Key == "profile" { fprintln(output, " Profile:", o.Value) } } diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index aa8fa4c33e..af26d3199b 100644 --- a/cli/command/system/info_test.go +++ b/cli/command/system/info_test.go @@ -475,12 +475,13 @@ func TestFormatInfo(t *testing.T) { ClientInfo: &clientInfo{Debug: true}, } err := formatInfo(cli.Out(), info, tc.template) - if tc.expectedOut != "" { + switch { + case tc.expectedOut != "": assert.NilError(t, err) assert.Equal(t, cli.OutBuffer().String(), tc.expectedOut) - } else if tc.expectedError != "" { + case tc.expectedError != "": assert.Error(t, err, tc.expectedError) - } else { + default: t.Fatal("test expected to neither pass nor fail") } }) diff --git a/cli/command/system/version.go b/cli/command/system/version.go index a84b93fe34..4cb34a0273 100644 --- a/cli/command/system/version.go +++ b/cli/command/system/version.go @@ -161,8 +161,7 @@ func runVersion(dockerCli command.Cli, opts *versionOptions) error { vd.Server = &sv foundEngine := false for _, component := range sv.Components { - switch component.Name { - case "Engine": + if component.Name == "Engine" { foundEngine = true buildTime, ok := component.Details["BuildTime"] if ok { diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 3ac728b3e6..c5edd5c51f 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -663,10 +663,8 @@ func loadFileObjectConfig(name string, objType string, obj types.FileObjectConfi } obj.Name = obj.External.Name obj.External.Name = "" - } else { - if obj.Name == "" { - obj.Name = name - } + } else if obj.Name == "" { + obj.Name = name } // if not "external: true" case obj.Driver != "": diff --git a/cli/connhelper/connhelper.go b/cli/connhelper/connhelper.go index b98d97c25d..1797abaed4 100644 --- a/cli/connhelper/connhelper.go +++ b/cli/connhelper/connhelper.go @@ -40,8 +40,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper if err != nil { return nil, err } - switch scheme := u.Scheme; scheme { - case "ssh": + if u.Scheme == "ssh" { sp, err := ssh.ParseURL(daemonURL) if err != nil { return nil, errors.Wrap(err, "ssh host connection is not valid") diff --git a/opts/hosts_test.go b/opts/hosts_test.go index f1b2ca6b12..4da5f5625c 100644 --- a/opts/hosts_test.go +++ b/opts/hosts_test.go @@ -170,10 +170,8 @@ func TestValidateExtraHosts(t *testing.T) { for extraHost, expectedError := range invalid { if _, err := ValidateExtraHost(extraHost); err == nil { t.Fatalf("ValidateExtraHost(`%q`) should have failed validation", extraHost) - } else { - if !strings.Contains(err.Error(), expectedError) { - t.Fatalf("ValidateExtraHost(`%q`) error should contain %q", extraHost, expectedError) - } + } else if !strings.Contains(err.Error(), expectedError) { + t.Fatalf("ValidateExtraHost(`%q`) error should contain %q", extraHost, expectedError) } } }