golangci enable additional linters (prealloc predeclared whitespace t…

…agalign) and fix issues (#27959)

* golangci enable prealloc and predeclared linters and update config to go 1.22

* make whitespace & enable linter to see if it is still super slow

* fix linting

* tagalign

* try out nakedret

* make generate

* fix potential crash

.golangci.yml

@@ -3,7 +3,7 @@
 
 run:
   timeout: 60m
-  go: 1.21
+  go: 1.22
   modules-download-mode: vendor
   skip-dirs:
     - /sdk/ # Excluding sdk folders as these are externally generated
@@ -18,43 +18,50 @@ linters:
   disable-all: true
   enable:
     - asasalint # check for pass []any as any in variadic func(...any)
-    - asciicheck # code does not contain non-ASCII identifiers
-    - bidichk # Checks for dangerous unicode character sequences
-    - decorder # Check declaration order and count of types, constants, variables and functions.
-    - durationcheck # Check for common mistakes when working with time.Duration
-    - dupword #Check for duplicated words in comments
+    - asciicheck # ensure code does not contain non-ASCII identifiers
+    - bidichk # checks for dangerous unicode character sequences
+    - decorder # check declaration order and count of types, constants, variables and functions.
+    - durationcheck # check for common mistakes when working with time.Duration
+    - dupword # check for duplicated words in comments
     - errcheck # checking for unchecked errors
-    - gocritic # Linter for Go source code that specializes in simplifying code
-    - gofmt # Gofmt checks whether code was gofmt-ed
+    - gocritic # linter for Go source code that specializes in simplifying code
+    - gofmt # checks whether code was gofmt-ed
     #- gofumpt # Gofumpt is a stricter gofmt
-    - goimports # Check import statements are formatted according to the 'goimport' command
-    - gosimple # Linter for Go source code that specializes in simplifying code.
-    #- gosec # Gosec is a security linter for Go source code
+    - goimports # check import statements are formatted according to the 'goimport' command
+    - gosimple # linter for Go source code that specializes in simplifying code.
     - govet # reports suspicious constructs. It is roughly the same as 'go vet' (replaced vet and vetshadow)
-    - ineffassign # Detects when assignments to existing variables are not used
-    - misspell # Finds commonly misspelled English words.
-    #- nakedret # Checks that functions with naked returns are not longer than a maximum size
+    - ineffassign # detects when assignments to existing variables are not used
+    - misspell # finds commonly misspelled English words.
     #- nilerr # Finds the code that returns nil even if it checks that the error is not nil.
     #- nlreturn # Nlreturn checks for a new line before return and branch statements to increase code clarity.
-    #- paralleltest # Detects missing usage of t.Parallel() method in your Go test.
-    #- perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative.
-    #- prealloc # Finds slice declarations that could potentially be pre-allocated.
-    #- predeclared # Find code that shadows one of Go's predeclared identifiers.
-    - reassign # Checks that package variables are not reassigned.
+    #- paralleltest # detects missing usage of t.Parallel() method in your Go test.
+    - prealloc # finds slice declarations that could potentially be pre-allocated.
+    - predeclared # find code that shadows one of Go's predeclared identifiers.
+    - reassign # checks that package variables are not reassigned.
     #- revive #Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint.
-    - staticcheck # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary.
     #- tparallel # Tparallel detects inappropriate usage of t.Parallel() method in your Go test codes.
-    - typecheck # doesn't exist?!>?! XXXXXXXXX
-    - unused # Checks Go code for unused constants, variables, functions and types.
-    - unconvert # Remove unnecessary type conversions.
-    - unparam # Reports unused function parameters.
-    - wastedassign # Finds wasted assignment statements
-    #- wsl Add or remove empty lines.
+    - tagalign #  checks struct tags that do not align with the specified column in struct definitions.
+    - staticcheck # checks rules from staticcheck. It's not the same thing as the staticcheck binary.
+    - unused # checks Go code for unused constants, variables, functions and types.
+    - unconvert # checks for unnecessary type conversions.
+    - unparam # reports unused function parameters.
+    - wastedassign # finds wasted assignment statements
+    - whitespace # checks for unnecessary newlines at the start and end of functions, if, for, etc. (
+    #- wsl # add or remove empty lines.
+
+    ###### DISABLED because : integer overflow conversion int -> int32
+    # - gosec # Gosec is a security linter for Go source code
+
+    ##### DISABLED as i was fixing them with other linters, and not realizing how many there are - disabling for now even thou it is valid
+    #- perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative.
+
+    #### VALID but DISABLED as we have a lot of these in the codebase to switch over
+    #- nakedret # Checks that functions with naked returns are not longer than a maximum size
 
     #### DISABLED TO DO IN ITS OWN PR - should be enabled and done #####
     #- tenv #detects using os.Setenv instead of t.Setenv since Go1.17.
 
-    #### REQUIRES GO 1.22 #####
+    #### bunch of hits, need to confirm if errors or not #####
     #- copyloopvar #Detects range loop variables that are overwritten in the loop body
 
     #### DISABLED till %+v -> %w #####
@@ -64,12 +71,9 @@ linters:
     # disabled as it may be useful but there are a lot of switch statements in the codebase with unhandled inputs
     #- exhaustive #Check for missing cases in select statements
 
-    ###### DISABLED because golang will put the space back into //nolint: linter ######
+    ###### DISABLED because golang will put the space back into `//nolint: linter` -> `// nolint: linter` ######
     #- nolintlint  #Reports ill-formed or insufficient nolint directives.
 
-    # Disabled for performance reasons - Ignores cache and takes 12+ minutes to run on the repo for _any_ change
-    #- whitespace #checks for unnecessary newlines at the start and end of functions, if, for, etc.
-
 linters-settings:
   errcheck:
     ignore: github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema:ForceNew|Set,fmt:.*,io:Close
@@ -78,4 +82,17 @@ linters-settings:
       - hdinsight
       - exportfs
   nakedret:
-    max-func-lines: 40
+    max-func-lines: 30
+  tagalign:
+    sort: true
+    order:
+      - json
+      - tfschema
+      - computed
+  predeclared:
+    ignore: new,min,max # should we use newer, minimum, and maximum instead?
+  revive:
+    rules:
+      - name: var-naming
+        disabled: true
+

GNUmakefile

@@ -80,6 +80,10 @@ whitespace:
 	@echo "==> Fixing source code with whitespace linter..."
 	golangci-lint run ./... --no-config --disable-all --enable=whitespace --fix
 
+golangci-fix:
+	@echo "==> Fixing source code with all golangci linters..."
+	golangci-lint run ./... --fix
+
 test: fmtcheck
 	@TEST=$(TEST) ./scripts/run-gradually-deprecated.sh
 	@TEST=$(TEST) ./scripts/run-test.sh

internal/acceptance/data.go

@@ -89,41 +89,41 @@ func BuildTestData(t *testing.T, resourceType string, resourceLabel string) Test
 }
 
 // RandomIntOfLength is a random 8 to 18 digit integer which is unique to this test case
-func (td *TestData) RandomIntOfLength(len int) int {
-	// len should not be
+func (td *TestData) RandomIntOfLength(length int) int {
+	// length should not be
 	//  - greater then 18, longest a int can represent
 	//  - less then 8, as that gives us YYMMDDRR
-	if 8 > len || len > 18 {
-		panic("Invalid Test: RandomIntOfLength: len is not between 8 or 18 inclusive")
+	if 8 > length || length > 18 {
+		panic("Invalid Test: RandomIntOfLength: length is not between 8 or 18 inclusive")
 	}
 
 	// 18 - just return the int
-	if len >= 18 {
+	if length >= 18 {
 		return td.RandomInteger
 	}
 
 	// 16-17 just strip off the last 1-2 digits
-	if len >= 16 {
-		return td.RandomInteger / int(math.Pow10(18-len))
+	if length >= 16 {
+		return td.RandomInteger / int(math.Pow10(18-length))
 	}
 
-	// 8-15 keep len - 2 digits and add 2 characters of randomness on
+	// 8-15 keep length - 2 digits and add 2 characters of randomness on
 	s := strconv.Itoa(td.RandomInteger)
 	r := s[16:18]
-	v := s[0 : len-2]
+	v := s[0 : length-2]
 	i, _ := strconv.Atoi(v + r)
 
 	return i
 }
 
 // RandomStringOfLength is a random 1 to 1024 character string which is unique to this test case
-func (td *TestData) RandomStringOfLength(len int) string {
+func (td *TestData) RandomStringOfLength(length int) string {
 	// len should not be less then 1 or greater than 1024
-	if 1 > len || len > 1024 {
+	if 1 > length || length > 1024 {
 		panic("Invalid Test: RandomStringOfLength: length argument must be between 1 and 1024 characters")
 	}
 
-	return randString(len)
+	return randString(length)
 }
 
 // randString generates a random alphanumeric string of the length specified

internal/clients/auth.go

@@ -5,6 +5,7 @@ package clients
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -93,7 +94,7 @@ func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, sub
 		// Use the tenant ID from Azure CLI when otherwise unknown
 		if tenantId == "" {
 			if cli.TenantID == "" {
-				return nil, fmt.Errorf("azure-cli could not determine tenant ID to use")
+				return nil, errors.New("azure-cli could not determine tenant ID to use")
 			}
 			tenantId = cli.TenantID
 			log.Printf("[DEBUG] Using tenant ID from Azure CLI: %q", tenantId)
@@ -108,10 +109,10 @@ func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, sub
 
 	// We'll permit the provider to proceed with an unknown client ID since it only affects a small number of use cases when authenticating as a user
 	if tenantId == "" {
-		return nil, fmt.Errorf("unable to configure ResourceManagerAccount: tenant ID could not be determined and was not specified")
+		return nil, errors.New("unable to configure ResourceManagerAccount: tenant ID could not be determined and was not specified")
 	}
 	if subscriptionId == "" {
-		return nil, fmt.Errorf("unable to configure ResourceManagerAccount: subscription ID could not be determined and was not specified")
+		return nil, errors.New("unable to configure ResourceManagerAccount: subscription ID could not be determined and was not specified")
 	}
 
 	account := ResourceManagerAccount{

internal/clients/builder.go

@@ -114,7 +114,7 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) {
 
 	resourceManagerEndpoint, ok := builder.AuthConfig.Environment.ResourceManager.Endpoint()
 	if !ok {
-		return nil, fmt.Errorf("unable to determine resource manager endpoint for the current environment")
+		return nil, errors.New("unable to determine resource manager endpoint for the current environment")
 	}
 
 	client := Client{

internal/clients/client_gen.go

@@ -20,7 +20,6 @@ type autoClient struct {
 }
 
 func buildAutoClients(client *autoClient, o *common.ClientOptions) (err error) {
-
 	if client.ChaosStudio, err = chaosstudio.NewClient(o); err != nil {
 		return fmt.Errorf("building client for ChaosStudio: %+v", err)
 	}

internal/clients/graph/client.go

@@ -5,6 +5,7 @@ package graph
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -100,7 +101,7 @@ func ServicePrincipalObjectID(ctx context.Context, authorizer auth.Authorizer, e
 
 	id := model.ServicePrincipals[0].ID
 	if id == nil {
-		return nil, fmt.Errorf("returned object ID was nil")
+		return nil, errors.New("returned object ID was nil")
 	}
 
 	return id, nil

internal/provider/provider.go

@@ -553,7 +553,6 @@ func buildClient(ctx context.Context, p *schema.Provider, d *schema.ResourceData
 
 	if err = resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders); err != nil {
 		return nil, diag.FromErr(err)
-
 	}
 
 	return client, nil

internal/sdk/frameworkhelpers/list.go

@@ -83,7 +83,6 @@ func (w WrappedListValidator) ValidateList(ctx context.Context, request validato
 	default:
 		response.Diagnostics.AddError(fmt.Sprintf("unsupported list validation wrapper type for %s", path), fmt.Sprintf("%+v", request.ConfigValue))
 	}
-
 }
 
 var _ validator.List = &WrappedListValidator{}

internal/sdk/resource_decode.go

@@ -301,7 +301,6 @@ func setListValue(input interface{}, index int, fieldName string, v []interface{
 			}
 			tmp.Elem().Set(slice)
 			reflect.ValueOf(input).Elem().Field(index).Set(tmp)
-
 		}
 
 	default:

internal/sdk/resource_encode.go

@@ -236,7 +236,6 @@ func recurse(objType reflect.Type, objVal reflect.Value, debugLogger Logger) (ou
 							debugLogger.Infof("[SLICE] Setting %q to %+v", structTags.hclPath, attr)
 							output[structTags.hclPath] = attr
 						}
-
 					}
 				} else {
 					debugLogger.Infof("Setting %q to nil", structTags.hclPath)

internal/sdk/resource_encode_test.go

@@ -384,15 +384,15 @@ func TestResourceEncode_TopLevelOmitted(t *testing.T) {
 
 func TestResourceEncode_TopLevelComputed(t *testing.T) {
 	type SimpleType struct {
-		ComputedString        string             `tfschema:"computed_string" computed:"true"`
-		ComputedNumber        int64              `tfschema:"computed_number" computed:"true"`
-		ComputedBool          bool               `tfschema:"computed_bool" computed:"true"`
+		ComputedString        string             `tfschema:"computed_string"          computed:"true"`
+		ComputedNumber        int64              `tfschema:"computed_number"          computed:"true"`
+		ComputedBool          bool               `tfschema:"computed_bool"            computed:"true"`
 		ComputedListOfNumbers []int64            `tfschema:"computed_list_of_numbers" computed:"true"`
 		ComputedListOfStrings []string           `tfschema:"computed_list_of_strings" computed:"true"`
-		ComputedMapOfBools    map[string]bool    `tfschema:"computed_map_of_bools" computed:"true"`
-		ComputedMapOfFloats   map[string]float64 `tfschema:"computed_map_of_floats" computed:"true"`
-		ComputedMapOfInts     map[string]int64   `tfschema:"computed_map_of_ints" computed:"true"`
-		ComputedMapOfStrings  map[string]string  `tfschema:"computed_map_of_strings" computed:"true"`
+		ComputedMapOfBools    map[string]bool    `tfschema:"computed_map_of_bools"    computed:"true"`
+		ComputedMapOfFloats   map[string]float64 `tfschema:"computed_map_of_floats"   computed:"true"`
+		ComputedMapOfInts     map[string]int64   `tfschema:"computed_map_of_ints"     computed:"true"`
+		ComputedMapOfStrings  map[string]string  `tfschema:"computed_map_of_strings"  computed:"true"`
 	}
 	encodeTestData{
 		Input: &SimpleType{
@@ -573,9 +573,9 @@ func TestResourceEncode_NestedOneLevelDeepSingleOmittedValues(t *testing.T) {
 		MapOfBools           map[string]bool    `tfschema:"map_of_bools"`
 		MapOfNumbers         map[string]int64   `tfschema:"map_of_numbers"`
 		MapOfStrings         map[string]string  `tfschema:"map_of_strings"`
-		ComputedMapOfBools   map[string]bool    `tfschema:"computed_map_of_bools" computed:"true"`
-		ComputedMapOfFloats  map[string]float64 `tfschema:"computed_map_of_floats" computed:"true"`
-		ComputedMapOfInts    map[string]int64   `tfschema:"computed_map_of_ints" computed:"true"`
+		ComputedMapOfBools   map[string]bool    `tfschema:"computed_map_of_bools"   computed:"true"`
+		ComputedMapOfFloats  map[string]float64 `tfschema:"computed_map_of_floats"  computed:"true"`
+		ComputedMapOfInts    map[string]int64   `tfschema:"computed_map_of_ints"    computed:"true"`
 		ComputedMapOfStrings map[string]string  `tfschema:"computed_map_of_strings" computed:"true"`
 	}
 	type Type struct {

internal/sdk/resource_test.go

@@ -8,7 +8,7 @@ package sdk
 type ExampleObj struct {
 	Name        string            `tfschema:"name"`
 	Number      int               `tfschema:"number"`
-	Output      string            `tfschema:"output" computed:"true"`
+	Output      string            `tfschema:"output"       computed:"true"`
 	Enabled     bool              `tfschema:"enabled"`
 	Networks    []string          `tfschema:"networks"`
 	NetworksSet []string          `tfschema:"networks_set"`

internal/services/aadb2c/aadb2c_directory_resource.go

@@ -5,6 +5,7 @@ package aadb2c
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -144,10 +145,10 @@ func (r AadB2cDirectoryResource) Create() sdk.ResourceFunc {
 			}
 
 			if model.CountryCode == "" {
-				return fmt.Errorf("`country_code` is required when creating a new AADB2C directory")
+				return errors.New("`country_code` is required when creating a new AADB2C directory")
 			}
 			if model.DisplayName == "" {
-				return fmt.Errorf("`display_name` is required when creating a new AADB2C directory")
+				return errors.New("`display_name` is required when creating a new AADB2C directory")
 			}
 
 			id := tenants.NewB2CDirectoryID(subscriptionId, model.ResourceGroup, model.DomainName)

internal/services/advisor/advisor_recommendations_data_source.go

@@ -132,7 +132,7 @@ func dataSourceAdvisorRecommendationsRead(d *pluginsdk.ResourceData, meta interf
 		return fmt.Errorf("setting `recommendations`: %+v", err)
 	}
 
-	d.SetId(fmt.Sprintf("avdisor/recommendations/%s", time.Now().UTC().String()))
+	d.SetId("avdisor/recommendations/" + time.Now().UTC().String())
 
 	return nil
 }

internal/services/apimanagement/api_management_api_operation_policy_resource.go

@@ -4,6 +4,7 @@
 package apimanagement
 
 import (
+	"errors"
 	"fmt"
 	"html"
 	"log"
@@ -112,7 +113,7 @@ func resourceApiManagementAPIOperationPolicyCreateUpdate(d *pluginsdk.ResourceDa
 	}
 
 	if parameters.Properties == nil {
-		return fmt.Errorf("Either `xml_content` or `xml_link` must be set")
+		return errors.New("Either `xml_content` or `xml_link` must be set")
 	}
 
 	if _, err := client.CreateOrUpdate(ctx, id, parameters, apioperationpolicy.CreateOrUpdateOperationOptions{}); err != nil {

internal/services/apimanagement/api_management_api_policy_resource.go

@@ -4,6 +4,7 @@
 package apimanagement
 
 import (
+	"errors"
 	"fmt"
 	"html"
 	"log"
@@ -115,7 +116,7 @@ func resourceApiManagementAPIPolicyCreateUpdate(d *pluginsdk.ResourceData, meta
 	}
 
 	if parameters.Properties == nil {
-		return fmt.Errorf("Either `xml_content` or `xml_link` must be set")
+		return errors.New("Either `xml_content` or `xml_link` must be set")
 	}
 
 	if _, err := client.CreateOrUpdate(ctx, id, parameters, apipolicy.CreateOrUpdateOperationOptions{}); err != nil {