chore: add support for gocritic (#4063)

remyleone · web-flow · commit 2de0b33bf57a · 2024-08-22T16:00:49.000Z
diff --git a/.golangci.yml b/.golangci.yml
@@ -23,6 +23,7 @@ linters:
     - gocheckcompilerdirectives # Checks that go compiler directive comments (//go:) are valid. [fast: true, auto-fix: false]
     - gochecksumtype # Run exhaustiveness checks on Go "sum types" [fast: false, auto-fix: false]
     - goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false]
+    - gocritic # Provides diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: false]
     - gocyclo # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false]
     - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
     - gofumpt # Gofumpt checks whether code was gofumpt-ed. [fast: true, auto-fix: true]
diff --git a/cmd/scw/main.go b/cmd/scw/main.go
@@ -60,6 +60,11 @@ func buildVersion() string {
 }
 
 func main() {
+	exitCode := mainNoExit()
+	os.Exit(exitCode)
+}
+
+func mainNoExit() int {
 	buildInfo := &core.BuildInfo{
 		Version:   version.Must(version.NewSemver(buildVersion())), // panic when version does not respect semantic versioning
 		BuildDate: BuildDate,
@@ -82,5 +87,5 @@ func main() {
 		Platform:  terminal.NewPlatform(buildInfo.GetUserAgent()),
 	})
 
-	os.Exit(exitCode)
+	return exitCode
 }
diff --git a/internal/args/marshal.go b/internal/args/marshal.go
@@ -27,7 +27,7 @@ var marshalFuncs = map[reflect.Type]MarshalFunc{
 	reflect.TypeOf((*scw.Size)(nil)).Elem(): func(src interface{}) (s string, e error) {
 		v := src.(*scw.Size)
 		value := humanize.Bytes(uint64(*v))
-		value = strings.Replace(value, " ", "", -1)
+		value = strings.ReplaceAll(value, " ", "")
 		return value, nil
 	},
 	reflect.TypeOf((*time.Time)(nil)).Elem(): func(src interface{}) (string, error) {
diff --git a/internal/core/autocomplete.go b/internal/core/autocomplete.go
@@ -370,8 +370,7 @@ func AutoCompleteArgValue(ctx context.Context, cmd *Command, argSpec *ArgSpec, a
 	possibleValues := []string(nil)
 
 	if fieldType, err := args.GetArgType(cmd.ArgsType, argSpec.Name); err == nil {
-		switch fieldType.Kind() {
-		case reflect.Bool:
+		if fieldType.Kind() == reflect.Bool {
 			possibleValues = []string{"true", "false"}
 		}
 	}
diff --git a/internal/core/autocomplete_test.go b/internal/core/autocomplete_test.go
@@ -89,8 +89,8 @@ func runAutocompleteTest(ctx context.Context, tc *autoCompleteTestCase) func(*te
 		t.Helper()
 		words := tc.Words
 		if len(words) == 0 {
-			name := strings.Replace(t.Name(), "TestAutocomplete/", "", -1)
-			name = strings.Replace(name, "_", " ", -1)
+			name := strings.ReplaceAll(t.Name(), "TestAutocomplete/", "")
+			name = strings.ReplaceAll(name, "_", " ")
 			// Test can contain a sharp if duplicated
 			// MyTest/scw_-flag_#01
 			sharpIndex := strings.Index(name, "#")
diff --git a/internal/core/autocomplete_utils.go b/internal/core/autocomplete_utils.go
@@ -73,7 +73,7 @@ func AutocompleteProfileName() AutoCompleteArgFunc {
 			}
 		}
 
-		if strings.HasPrefix(scw.DefaultProfileName, prefix) {
+		if strings.HasPrefix(scw.DefaultProfileName, prefix) { //nolint:gocritic
 			res = append(res, scw.DefaultProfileName)
 		}
 		return res
diff --git a/internal/core/cobra_utils.go b/internal/core/cobra_utils.go
@@ -206,7 +206,7 @@ func handleUnmarshalErrors(cmd *Command, unmarshalErr *args.UnmarshalArgError) e
 
 	switch e := wrappedErr.(type) {
 	case *args.CannotUnmarshalError:
-		switch e.Err.(type) {
+		switch e.Err.(type) { //nolint:gocritic
 		case *args.CannotParseBoolError:
 			return &CliError{
 				Err:     errors.New(""),
diff --git a/internal/core/command.go b/internal/core/command.go
@@ -386,11 +386,12 @@ func (c *Commands) applyAliases(config *alias.Config) {
 	for _, command := range c.commands {
 		aliases := []alias.Alias(nil)
 		exists := false
-		if command.Verb != "" {
+		switch {
+		case command.Verb != "":
 			aliases, exists = config.ResolveAliasesByFirstWord(command.Verb)
-		} else if command.Resource != "" {
+		case command.Resource != "":
 			aliases, exists = config.ResolveAliasesByFirstWord(command.Resource)
-		} else if command.Namespace != "" {
+		case command.Namespace != "":
 			aliases, exists = config.ResolveAliasesByFirstWord(command.Namespace)
 		}
 		if exists {
diff --git a/internal/core/command_interceptor.go b/internal/core/command_interceptor.go
@@ -103,8 +103,7 @@ func sdkStdErrorInterceptor(ctx context.Context, args interface{}, runner Comman
 		}
 	case *scw.ResourceExpiredError:
 		var hint string
-		switch resourceName := sdkError.Resource; resourceName {
-		case "account_token":
+		if sdkError.Resource == "account_token" {
 			hint = "Try to generate a new token here https://console.scaleway.com/iam/api-keys"
 		}
 
@@ -124,8 +123,7 @@ func sdkStdTypeInterceptor(ctx context.Context, args interface{}, runner Command
 	if err != nil {
 		return res, err
 	}
-	switch sdkValue := res.(type) {
-	case *scw.File:
+	if sdkValue, ok := res.(*scw.File); ok {
 		ExtractLogger(ctx).Debug("Intercepting scw.File type, rendering as string")
 		fileContent, err := io.ReadAll(sdkValue.Content)
 		if err != nil {
diff --git a/internal/core/reflect_test.go b/internal/core/reflect_test.go
@@ -91,10 +91,8 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, tc.expectedError, err.Error())
-				} else {
-					if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
-						t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
-					}
+				} else if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
+					t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
 				}
 			},
 		},
@@ -122,10 +120,8 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, tc.expectedError, err.Error())
-				} else {
-					if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
-						t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
-					}
+				} else if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
+					t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
 				}
 			},
 		},
@@ -166,10 +162,8 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, nil, err.Error())
-				} else {
-					if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
-						t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
-					}
+				} else if tc.expectedValues != nil && !reflect.DeepEqual(tc.expectedValues[0].Interface(), values[0].Interface()) {
+					t.Errorf("Expected %v, got %v", tc.expectedValues[0].Interface(), values[0].Interface())
 				}
 			},
 		},
diff --git a/internal/core/shell.go b/internal/core/shell.go
@@ -77,11 +77,12 @@ func ArgIsOption(arg string) bool {
 }
 
 func argIsPositional(cmd *Command, arg string) bool {
-	if cmd.Verb != "" && cmd.Verb == arg {
+	switch {
+	case cmd.Verb != "" && cmd.Verb == arg:
 		return false
-	} else if cmd.Resource != "" && cmd.Resource == arg {
+	case cmd.Resource != "" && cmd.Resource == arg:
 		return false
-	} else if cmd.Namespace != "" && cmd.Resource == arg {
+	case cmd.Namespace != "" && cmd.Resource == arg:
 		return false
 	}
 
diff --git a/internal/core/testing.go b/internal/core/testing.go
@@ -204,7 +204,7 @@ func getTestFilePath(t *testing.T, suffix string) string {
 	specialChars := regexp.MustCompile(`[\\?%*:|"<>. ]`)
 
 	// Replace nested tests separators.
-	fileName := strings.Replace(t.Name(), "/", "-", -1)
+	fileName := strings.ReplaceAll(t.Name(), "/", "-")
 
 	fileName = strcase.ToBashArg(fileName)
 
@@ -712,7 +712,7 @@ func removeRandomPrefixFromOutput(output string) string {
 	end := strings.IndexByte(output[begin:], '\n')
 	actualBucketName := output[begin : begin+end]
 	normalizedBucketName := strings.TrimRight(actualBucketName, "0123456789")
-	return strings.Replace(output, actualBucketName, normalizedBucketName, -1)
+	return strings.ReplaceAll(output, actualBucketName, normalizedBucketName)
 }
 
 // TestCheckError asserts error
diff --git a/internal/gofields/gofields.go b/internal/gofields/gofields.go
@@ -145,7 +145,7 @@ func listFields(t reflect.Type, parents []string, filter ListFieldFilter) []stri
 
 			fieldParents := parents
 			if !field.Anonymous {
-				fieldParents = append(parents, field.Name)
+				fieldParents = append(parents, field.Name) //nolint:gocritic
 			}
 
 			res = append(res, listFields(field.Type, fieldParents, filter)...)
diff --git a/internal/human/marshal_func.go b/internal/human/marshal_func.go
@@ -172,8 +172,7 @@ func defaultMarshalerFunc(i interface{}, _ *MarshalOpt) (string, error) {
 		i = "-"
 	}
 
-	switch v := i.(type) {
-	case string:
+	if v, ok := i.(string); ok {
 		if v == "" {
 			i = "-"
 		}
diff --git a/internal/human/marshal_test.go b/internal/human/marshal_test.go
@@ -81,7 +81,7 @@ func TestMarshal(t *testing.T) {
 
 			// Format expected to allow indentation when writing test
 			expected := tc.result
-			expected = strings.Replace(expected, "\t", "", -1)
+			expected = strings.ReplaceAll(expected, "\t", "")
 			expected = strings.Trim(expected, "\n")
 
 			if tc.result != "" {
diff --git a/internal/interactive/list.go b/internal/interactive/list.go
@@ -28,9 +28,8 @@ func (m *ListPrompt) Init() tea.Cmd {
 }
 
 func (m *ListPrompt) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
-	switch msg := msg.(type) {
-	// Key is pressed
-	case tea.KeyMsg:
+	if msg, ok := msg.(tea.KeyMsg); ok {
+		// Key is pressed
 		switch msg.String() {
 		case "ctrl+c", "q":
 			m.cancelled = true
diff --git a/internal/interactive/prompt.go b/internal/interactive/prompt.go
@@ -36,9 +36,9 @@ func PromptBoolWithConfig(config *PromptBoolConfig) (bool, error) {
 	for {
 		prompt := terminal.Style(config.Prompt, color.Bold)
 		if config.DefaultValue {
-			prompt = prompt + " (Y/n): "
+			prompt += " (Y/n): "
 		} else {
-			prompt = prompt + " (y/N): "
+			prompt += " (y/N): "
 		}
 
 		str, err := Readline(&ReadlineConfig{
diff --git a/internal/namespaces/autocomplete/errors.go b/internal/namespaces/autocomplete/errors.go
@@ -13,9 +13,9 @@ func unsupportedShellError(shell string) *core.CliError {
 	}
 }
 
-func unsupportedOsError(OS string) *core.CliError {
+func unsupportedOsError(os string) *core.CliError {
 	return &core.CliError{
-		Err: fmt.Errorf("unsupported OS '%v'", OS),
+		Err: fmt.Errorf("unsupported OS '%v'", os),
 	}
 }
 
diff --git a/internal/namespaces/baremetal/v1/custom_offer.go b/internal/namespaces/baremetal/v1/custom_offer.go
@@ -41,8 +41,8 @@ func listOfferMarshalerFunc(i interface{}, opt *human.MarshalOpt) (string, error
 			Title:     "PrivateBandwidth(Mbit/s)",
 		},
 	}
-	baremetalOffer.PrivateBandwidth = baremetalOffer.PrivateBandwidth / 1000000
-	baremetalOffer.Bandwidth = baremetalOffer.Bandwidth / 1000000
+	baremetalOffer.PrivateBandwidth /= 1000000
+	baremetalOffer.Bandwidth /= 1000000
 	str, err := human.Marshal(baremetalOffer, opt)
 	if err != nil {
 		return "", err
diff --git a/internal/namespaces/billing/v2beta1/custom_invoice_download.go b/internal/namespaces/billing/v2beta1/custom_invoice_download.go
@@ -138,21 +138,15 @@ func invoiceDownloadBuilder(command *core.Command) *core.Command {
 }
 
 func addDownloadExt(fileName, contentType string) string {
-	switch contentType {
-	case "application/pdf":
+	if contentType == "application/pdf" {
 		fileName += ".pdf"
 	}
 
 	return fileName
 }
 
 func checkDownloadInvoiceExt(ext string) bool {
-	switch ext {
-	case ".pdf":
-		return true
-	}
-
-	return false
+	return ext == ".pdf"
 }
 
 func billingDownloadRun(ctx context.Context, argsI interface{}) (interface{}, error) {
diff --git a/internal/namespaces/billing/v2beta1/custom_invoice_export.go b/internal/namespaces/billing/v2beta1/custom_invoice_export.go
@@ -184,19 +184,13 @@ func billingExportRun(ctx context.Context, argsI interface{}) (interface{}, erro
 }
 
 func addExportExt(fileName, contentType string) string {
-	switch contentType {
-	case "text/csv":
+	if contentType == "text/csv" {
 		fileName += ".csv"
 	}
 
 	return fileName
 }
 
 func checkExportInvoiceExt(ext string) bool {
-	switch ext {
-	case ".csv":
-		return true
-	}
-
-	return false
+	return ext == ".csv"
 }
diff --git a/internal/namespaces/init/prompt.go b/internal/namespaces/init/prompt.go
@@ -136,8 +136,7 @@ func promptSecretKey(ctx context.Context) (string, error) {
 		Ctx: ctx,
 		PromptFunc: func(value string) string {
 			secretKey := "secret-key"
-			switch {
-			case validation.IsUUID(value):
+			if validation.IsUUID(value) {
 				secretKey = terminal.Style(secretKey, color.FgBlue)
 			}
 			return terminal.Style(fmt.Sprintf("Enter a valid %s: ", secretKey), color.Bold)
@@ -169,8 +168,7 @@ func promptAccessKey(ctx context.Context) (string, error) {
 		Ctx: ctx,
 		PromptFunc: func(value string) string {
 			accessKey := "access-key"
-			switch {
-			case validation.IsAccessKey(value):
+			if validation.IsAccessKey(value) {
 				accessKey = terminal.Style(accessKey, color.FgBlue)
 			}
 			return terminal.Style(fmt.Sprintf("Enter a valid %s: ", accessKey), color.Bold)
diff --git a/internal/namespaces/instance/v1/custom_server_create.go b/internal/namespaces/instance/v1/custom_server_create.go
@@ -45,7 +45,7 @@ type instanceCreateServerRequest struct {
 	CloudInit    string
 	BootType     string
 
-	// Deprecated, use project-id instead
+	// Deprecated: use project-id instead
 	OrganizationID *string
 }
 
@@ -513,7 +513,7 @@ func buildVolumeTemplateFromSnapshot(api *instance.API, zone scw.Zone, snapshotU
 	}, nil
 }
 
-func validateImageServerTypeCompatibility(image *instance.Image, serverType *instance.ServerType, CommercialType string) error {
+func validateImageServerTypeCompatibility(image *instance.Image, serverType *instance.ServerType, commercialType string) error {
 	// An instance might not have any constraints on the local volume size
 	if serverType.VolumesConstraint.MaxSize == 0 {
 		return nil
@@ -524,7 +524,7 @@ func validateImageServerTypeCompatibility(image *instance.Image, serverType *ins
 			humanize.Bytes(uint64(image.RootVolume.Size)),
 			humanize.Bytes(uint64(serverType.VolumesConstraint.MinSize)),
 			humanize.Bytes(uint64(serverType.VolumesConstraint.MaxSize)),
-			CommercialType,
+			commercialType,
 		)
 	}
 
@@ -641,7 +641,7 @@ func instanceServerCreateImageAutoCompleteFunc(ctx context.Context, prefix strin
 		completeListImagesCache = res
 	}
 
-	prefix = strings.ToLower(strings.Replace(prefix, "-", "_", -1))
+	prefix = strings.ToLower(strings.ReplaceAll(prefix, "-", "_"))
 
 	for _, image := range completeListImagesCache.Images {
 		if strings.HasPrefix(image.Label, prefix) {
diff --git a/internal/namespaces/instance/v1/custom_server_create_builder.go b/internal/namespaces/instance/v1/custom_server_create_builder.go
@@ -144,7 +144,7 @@ func (sb *ServerBuilder) AddImage(image string) (*ServerBuilder, error) {
 	case image == "none":
 		return sb, nil
 	case !validation.IsUUID(image):
-		imageLabel := strings.Replace(image, "-", "_", -1)
+		imageLabel := strings.ReplaceAll(image, "-", "_")
 
 		localImage, err := sb.apiMarketplace.GetLocalImageByLabel(&marketplace.GetLocalImageByLabelRequest{
 			ImageLabel:     imageLabel,
diff --git a/internal/namespaces/instance/v1/custom_ssh_key.go b/internal/namespaces/instance/v1/custom_ssh_key.go
@@ -112,7 +112,8 @@ Lookup /root/.ssh/authorized_keys on your server for more information`,
 				}
 			}
 
-			tags := append(server.Server.Tags, formattedKey)
+			tags := server.Server.Tags
+			tags = append(tags, formattedKey)
 
 			_, err = api.UpdateServer(&instance.UpdateServerRequest{
 				Zone:     args.Zone,
diff --git a/internal/namespaces/instance/v1/custom_ssh_key_test.go b/internal/namespaces/instance/v1/custom_ssh_key_test.go
@@ -15,7 +15,8 @@ import (
 func addSSHKey(serverKey string, sshKey string) core.BeforeFunc {
 	return func(ctx *core.BeforeFuncCtx) error {
 		server := ctx.Meta[serverKey].(*instanceSDK.Server)
-		tags := append(server.Tags, instance.FormatSSHKeyToTag(sshKey))
+		tags := server.Tags
+		tags = append(tags, instance.FormatSSHKeyToTag(sshKey))
 
 		resp, err := instanceSDK.NewAPI(ctx.Client).UpdateServer(&instanceSDK.UpdateServerRequest{
 			Zone:     server.Zone,
diff --git a/internal/namespaces/k8s/v1/custom_node.go b/internal/namespaces/k8s/v1/custom_node.go
@@ -43,8 +43,7 @@ func waitForNodeFunc(action int) core.WaitFunc {
 			Timeout:       scw.TimeDurationPtr(nodeActionTimeout),
 			RetryInterval: core.DefaultRetryInterval,
 		})
-		switch action {
-		case nodeActionReboot:
+		if action == nodeActionReboot {
 			return node, err
 		}
 		return nil, err
diff --git a/internal/namespaces/rdb/v1/custom_endpoint.go b/internal/namespaces/rdb/v1/custom_endpoint.go
diff --git a/internal/namespaces/rdb/v1/custom_url.go b/internal/namespaces/rdb/v1/custom_url.go
diff --git a/internal/namespaces/redis/v1/custom_cluster_test.go b/internal/namespaces/redis/v1/custom_cluster_test.go
diff --git a/internal/passwordgenerator/password_generator_test.go b/internal/passwordgenerator/password_generator_test.go

Original file line number	Diff line number	Diff line change
`@@ -370,8 +370,7 @@ func AutoCompleteArgValue(ctx context.Context, cmd Command, argSpec ArgSpec, a`
`370`	`370`	`possibleValues := []string(nil)`
`371`	`371`
`372`	`372`	`if fieldType, err := args.GetArgType(cmd.ArgsType, argSpec.Name); err == nil {`
`373`		`- switch fieldType.Kind() {`
`374`		`- case reflect.Bool:`
	`373`	`+ if fieldType.Kind() == reflect.Bool {`
`375`	`374`	`possibleValues = []string{"true", "false"}`
`376`	`375`	`}`
`377`	`376`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ func AutocompleteProfileName() AutoCompleteArgFunc {`
`73`	`73`	`}`
`74`	`74`	`}`
`75`	`75`
`76`		`- if strings.HasPrefix(scw.DefaultProfileName, prefix) {`
	`76`	`+ if strings.HasPrefix(scw.DefaultProfileName, prefix) { //nolint:gocritic`
`77`	`77`	`res = append(res, scw.DefaultProfileName)`
`78`	`78`	`}`
`79`	`79`	`return res`
Original file line number	Diff line number	Diff line change
`@@ -103,8 +103,7 @@ func sdkStdErrorInterceptor(ctx context.Context, args interface{}, runner Comman`
`103`	`103`	`}`
`104`	`104`	`case *scw.ResourceExpiredError:`
`105`	`105`	`var hint string`
`106`		`- switch resourceName := sdkError.Resource; resourceName {`
`107`		`- case "account_token":`
	`106`	`+ if sdkError.Resource == "account_token" {`
`108`	`107`	`hint = "Try to generate a new token here https://console.scaleway.com/iam/api-keys"`
`109`	`108`	`}`
`110`	`109`
`@@ -124,8 +123,7 @@ func sdkStdTypeInterceptor(ctx context.Context, args interface{}, runner Command`
`124`	`123`	`if err != nil {`
`125`	`124`	`return res, err`
`126`	`125`	`}`
`127`		`- switch sdkValue := res.(type) {`
`128`		`- case *scw.File:`
	`126`	`+ if sdkValue, ok := res.(*scw.File); ok {`
`129`	`127`	`ExtractLogger(ctx).Debug("Intercepting scw.File type, rendering as string")`
`130`	`128`	`fileContent, err := io.ReadAll(sdkValue.Content)`
`131`	`129`	`if err != nil {`