linting: address else/if/elseif statements found by gocritic

thaJeztah · thaJeztah · commit a2c9f3c6ce66 · 2023-11-20T16:02:16.000+01:00
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 &lt;github@gone.nl&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 		}
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,7 @@ const (`
`12`	`12`
`13`	`13`	`// NewFormat returns a format for use with a checkpoint Context`
`14`	`14`	`func NewFormat(source string) formatter.Format {`
`15`		`- switch source {`
`16`		`- case formatter.TableFormatKey:`
	`15`	`+ if source == formatter.TableFormatKey {`
`17`	`16`	`return defaultCheckpointFormat`
`18`	`17`	`}`
`19`	`18`	`return formatter.Format(source)`
Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,7 @@ const (`
`14`	`14`
`15`	`15`	`// NewDiffFormat returns a format for use with a diff Context`
`16`	`16`	`func NewDiffFormat(source string) formatter.Format {`
`17`		`- switch source {`
`18`		`- case formatter.TableFormatKey:`
	`17`	`+ if source == formatter.TableFormatKey {`
`19`	`18`	`return defaultDiffTableFormat`
`20`	`19`	`}`
`21`	`20`	`return formatter.Format(source)`
Original file line number	Diff line number	Diff line change
`@@ -1011,10 +1011,8 @@ func TestValidateDevice(t *testing.T) {`
`1011`	`1011`	`for path, expectedError := range invalid {`
`1012`	`1012`	`if _, err := validateDevice(path, runtime.GOOS); err == nil {`
`1013`	`1013`	t.Fatalf("ValidateDevice(`%q`) should have failed validation", path)
`1014`		`- } else {`
`1015`		`- if err.Error() != expectedError {`
`1016`		- t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error())
`1017`		`- }`
	`1014`	`+ } else if err.Error() != expectedError {`
	`1015`	+ t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error())
`1018`	`1016`	`}`
`1019`	`1017`	`}`
`1020`	`1018`	`}`
Original file line number	Diff line number	Diff line change
`@@ -576,18 +576,16 @@ func (b *Writer) Write(buf []byte) (n int, err error) {`
`576`	`576`	`b.startEscape(ch)`
`577`	`577`	`}`
`578`	`578`	`}`
`579`		`- } else {`
	`579`	`+ } else if ch == b.endChar {`
`580`	`580`	`// inside escape`
`581`		`- if ch == b.endChar {`
`582`		`- // end of tag/entity`
`583`		`- j := i + 1`
`584`		`- if ch == Escape && b.flags&StripEscape != 0 {`
`585`		`- j = i // strip Escape`
`586`		`- }`
`587`		`- b.append(buf[n:j])`
`588`		`- n = i + 1 // ch consumed`
`589`		`- b.endEscape()`
	`581`	`+ // end of tag/entity`
	`582`	`+ j := i + 1`
	`583`	`+ if ch == Escape && b.flags&StripEscape != 0 {`
	`584`	`+ j = i // strip Escape`
`590`	`585`	`}`
	`586`	`+ b.append(buf[n:j])`
	`587`	`+ n = i + 1 // ch consumed`
	`588`	`+ b.endEscape()`
`591`	`589`	`}`
`592`	`590`	`}`
`593`	`591`