Merge pull request #326 from hashicorp/sebasslash/linters-oh-my

Add more linters to our CI pipeline

.golangci.yml

@@ -10,10 +10,14 @@ linters:
     - nestif #https://github.com/nakabonne/nestif
     - exportloopref #https://github.com/kyoh86/exportloopref
     - bodyclose #https://github.com/timakin/bodyclose
+    - goconst #https://github.com/jgautheron/goconst
     - errcheck #https://github.com/kisielk/errcheck
     - stylecheck #https://github.com/dominikh/go-tools/tree/master/stylecheck
     - revive #golint is deprecated and golangci-lint recommends to use revive instead https://github.com/mgechev/revive
     #other deprecated lint libraries: maligned, scopelint, interfacer
+    - gocritic #https://github.com/go-critic/go-critic
+    - unparam #https://github.com/mvdan/unparam
+    - misspell #https://github.com/client9/misspell
 issues:
   exclude-rules:
   - path: _test\.go
@@ -25,6 +29,20 @@ linters-settings:
     # https://github.com/kisielk/errcheck#excluding-functions
     check-type-assertions: true
     check-blank: true  
+  goconst:
+    min-len: 20 
+    min-occurrences: 5 
+    ignore-calls: false
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - opinionated
+      - performance
+    disabled-checks:
+      - unnamedResult
+      - hugeParam
+      - singleCaseSwitch
+      - ifElseChain
   revive:
     # see https://github.com/mgechev/revive#available-rules for details.
     ignore-generated-header: false #recommended in their configuration
@@ -33,4 +51,4 @@ linters-settings:
       - name: indent-error-flow #Prevents redundant else statements
         severity: warning
       - name: useless-break
-        severity: warning
+        severity: warning

admin_organization.go

@@ -102,8 +102,8 @@ type AdminOrganizationID struct {
 
 // List all the organizations visible to the current user.
 func (s *adminOrganizations) List(ctx context.Context, options *AdminOrganizationListOptions) (*AdminOrganizationList, error) {
-	url := "admin/organizations"
-	req, err := s.client.newRequest("GET", url, options)
+	u := "admin/organizations"
+	req, err := s.client.newRequest("GET", u, options)
 	if err != nil {
 		return nil, err
 	}
@@ -122,9 +122,9 @@ func (s *adminOrganizations) ListModuleConsumers(ctx context.Context, organizati
 		return nil, ErrInvalidOrg
 	}
 
-	url := fmt.Sprintf("admin/organizations/%s/relationships/module-consumers", url.QueryEscape(organization))
+	u := fmt.Sprintf("admin/organizations/%s/relationships/module-consumers", url.QueryEscape(organization))
 
-	req, err := s.client.newRequest("GET", url, nil)
+	req, err := s.client.newRequest("GET", u, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -183,7 +183,7 @@ func (s *adminOrganizations) UpdateModuleConsumers(ctx context.Context, organiza
 		return ErrInvalidOrg
 	}
 
-	url := fmt.Sprintf("admin/organizations/%s/relationships/module-consumers", url.QueryEscape(organization))
+	u := fmt.Sprintf("admin/organizations/%s/relationships/module-consumers", url.QueryEscape(organization))
 
 	var organizations []*AdminOrganizationID
 	for _, id := range consumerOrganizationIDs {
@@ -193,7 +193,7 @@ func (s *adminOrganizations) UpdateModuleConsumers(ctx context.Context, organiza
 		organizations = append(organizations, &AdminOrganizationID{ID: id})
 	}
 
-	req, err := s.client.newRequest("PATCH", url, organizations)
+	req, err := s.client.newRequest("PATCH", u, organizations)
 	if err != nil {
 		return err
 	}

admin_user.go

@@ -27,10 +27,10 @@ type AdminUsers interface {
 	// Unsuspend a user by its ID.
 	Unsuspend(ctx context.Context, userID string) (*AdminUser, error)
 
-	// GrantAdmin grants admin privilages to a user by its ID.
+	// GrantAdmin grants admin privileges to a user by its ID.
 	GrantAdmin(ctx context.Context, userID string) (*AdminUser, error)
 
-	// RevokeAdmin revokees admin privilages to a user by its ID.
+	// RevokeAdmin revokees admin privileges to a user by its ID.
 	RevokeAdmin(ctx context.Context, userID string) (*AdminUser, error)
 
 	// Disable2FA disables a user's two-factor authentication in the situation
@@ -163,7 +163,7 @@ func (a *adminUsers) Unsuspend(ctx context.Context, userID string) (*AdminUser,
 	return au, nil
 }
 
-// GrantAdmin grants admin privilages to a user by its ID.
+// GrantAdmin grants admin privileges to a user by its ID.
 func (a *adminUsers) GrantAdmin(ctx context.Context, userID string) (*AdminUser, error) {
 	if !validStringID(&userID) {
 		return nil, ErrInvalidUserValue
@@ -184,7 +184,7 @@ func (a *adminUsers) GrantAdmin(ctx context.Context, userID string) (*AdminUser,
 	return au, nil
 }
 
-// RevokeAdmin revokes admin privilages to a user by its ID.
+// RevokeAdmin revokes admin privileges to a user by its ID.
 func (a *adminUsers) RevokeAdmin(ctx context.Context, userID string) (*AdminUser, error) {
 	if !validStringID(&userID) {
 		return nil, ErrInvalidUserValue

apply.go

@@ -31,7 +31,7 @@ type applies struct {
 // ApplyStatus represents an apply state.
 type ApplyStatus string
 
-//List all available apply statuses.
+// List all available apply statuses.
 const (
 	ApplyCanceled    ApplyStatus = "canceled"
 	ApplyCreated     ApplyStatus = "created"

configuration_version.go

@@ -47,7 +47,7 @@ type configurationVersions struct {
 // ConfigurationStatus represents a configuration version status.
 type ConfigurationStatus string
 
-//List all available configuration version statuses.
+// List all available configuration version statuses.
 const (
 	ConfigurationErrored  ConfigurationStatus = "errored"
 	ConfigurationPending  ConfigurationStatus = "pending"
@@ -234,7 +234,7 @@ func (s *configurationVersions) ReadWithOptions(ctx context.Context, cvID string
 // Upload packages and uploads Terraform configuration files. It requires the
 // upload URL from a configuration version and the path to the configuration
 // files on disk.
-func (s *configurationVersions) Upload(ctx context.Context, url, path string) error {
+func (s *configurationVersions) Upload(ctx context.Context, u, path string) error {
 	file, err := os.Stat(path)
 	if err != nil {
 		return err
@@ -250,7 +250,7 @@ func (s *configurationVersions) Upload(ctx context.Context, url, path string) er
 		return err
 	}
 
-	req, err := s.client.newRequest("PUT", url, body)
+	req, err := s.client.newRequest("PUT", u, body)
 	if err != nil {
 		return err
 	}

helper_test.go

@@ -23,7 +23,7 @@ var _testAccountDetails *TestAccountDetails
 
 func testClient(t *testing.T) *Client {
 	client, err := NewClient(nil)
-	client.RetryServerErrors(true) // because ocasionally we get a 500 internal when deleting an organization's workspace
+	client.RetryServerErrors(true) // because occasionally we get a 500 internal when deleting an organization's workspace
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -457,7 +457,7 @@ func createOrganizationToken(t *testing.T, client *Client, org *Organization) (*
 	}
 }
 
-func createRunTrigger(t *testing.T, client *Client, w *Workspace, sourceable *Workspace) (*RunTrigger, func()) {
+func createRunTrigger(t *testing.T, client *Client, w, sourceable *Workspace) (*RunTrigger, func()) {
 	var wCleanup func()
 	var sourceableCleanup func()
 

organization_tags_integration_test.go

@@ -122,7 +122,7 @@ func TestOrganizationTagsDelete(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		//sanity check ensure tags were deleted from the organization
+		// sanity check ensure tags were deleted from the organization
 		tags, err = client.OrganizationTags.List(ctx, orgTest.Name, nil)
 		require.NoError(t, err)
 
@@ -175,7 +175,7 @@ func TestOrganizationTagsAddWorkspace(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		//Ensure the tag was properly associated with the workspaces
+		// Ensure the tag was properly associated with the workspaces
 		fetched, err := client.Workspaces.ListTags(ctx, workspaceToAdd1.ID, nil)
 		require.NoError(t, err)
 		assert.Equal(t, fetched.Items[0].ID, tagID)

plan.go

@@ -35,7 +35,7 @@ type plans struct {
 // PlanStatus represents a plan state.
 type PlanStatus string
 
-//List all available plan statuses.
+// List all available plan statuses.
 const (
 	PlanCanceled    PlanStatus = "canceled"
 	PlanCreated     PlanStatus = "created"

policy_check.go

@@ -48,7 +48,7 @@ const (
 // PolicyStatus represents a policy check state.
 type PolicyStatus string
 
-//List all available policy check statuses.
+// List all available policy check statuses.
 const (
 	PolicyCanceled    PolicyStatus = "canceled"
 	PolicyErrored     PolicyStatus = "errored"

policy_set.go

@@ -236,7 +236,7 @@ type PolicySetUpdateOptions struct {
 	// https://jsonapi.org/format/#crud-creating
 	Type string `jsonapi:"primary,policy-sets"`
 
-	/// The name of the policy set.
+	// The name of the policy set.
 	Name *string `jsonapi:"attr,name,omitempty"`
 
 	// The description of the policy set.
@@ -293,7 +293,7 @@ func (s *policySets) Update(ctx context.Context, policySetID string, options Pol
 // PolicySetAddPoliciesOptions represents the options for adding policies
 // to a policy set.
 type PolicySetAddPoliciesOptions struct {
-	/// The policies to add to the policy set.
+	// The policies to add to the policy set.
 	Policies []*Policy
 }
 
@@ -328,7 +328,7 @@ func (s *policySets) AddPolicies(ctx context.Context, policySetID string, option
 // PolicySetRemovePoliciesOptions represents the options for removing
 // policies from a policy set.
 type PolicySetRemovePoliciesOptions struct {
-	/// The policies to remove from the policy set.
+	// The policies to remove from the policy set.
 	Policies []*Policy
 }
 
@@ -363,7 +363,7 @@ func (s *policySets) RemovePolicies(ctx context.Context, policySetID string, opt
 // PolicySetAddWorkspacesOptions represents the options for adding workspaces
 // to a policy set.
 type PolicySetAddWorkspacesOptions struct {
-	/// The workspaces to add to the policy set.
+	// The workspaces to add to the policy set.
 	Workspaces []*Workspace
 }
 
@@ -398,7 +398,7 @@ func (s *policySets) AddWorkspaces(ctx context.Context, policySetID string, opti
 // PolicySetRemoveWorkspacesOptions represents the options for removing
 // workspaces from a policy set.
 type PolicySetRemoveWorkspacesOptions struct {
-	/// The workspaces to remove from the policy set.
+	// The workspaces to remove from the policy set.
 	Workspaces []*Workspace
 }
 

policy_set_parameter.go

@@ -138,7 +138,7 @@ func (s *policySetParameters) Create(ctx context.Context, policySetID string, op
 }
 
 // Read a parameter by its ID.
-func (s *policySetParameters) Read(ctx context.Context, policySetID string, parameterID string) (*PolicySetParameter, error) {
+func (s *policySetParameters) Read(ctx context.Context, policySetID, parameterID string) (*PolicySetParameter, error) {
 	if !validStringID(&policySetID) {
 		return nil, ErrInvalidPolicySetID
 	}
@@ -180,7 +180,7 @@ type PolicySetParameterUpdateOptions struct {
 }
 
 // Update values of an existing parameter.
-func (s *policySetParameters) Update(ctx context.Context, policySetID string, parameterID string, options PolicySetParameterUpdateOptions) (*PolicySetParameter, error) {
+func (s *policySetParameters) Update(ctx context.Context, policySetID, parameterID string, options PolicySetParameterUpdateOptions) (*PolicySetParameter, error) {
 	if !validStringID(&policySetID) {
 		return nil, ErrInvalidPolicySetID
 	}
@@ -204,7 +204,7 @@ func (s *policySetParameters) Update(ctx context.Context, policySetID string, pa
 }
 
 // Delete a parameter by its ID.
-func (s *policySetParameters) Delete(ctx context.Context, policySetID string, parameterID string) error {
+func (s *policySetParameters) Delete(ctx context.Context, policySetID, parameterID string) error {
 	if !validStringID(&policySetID) {
 		return ErrInvalidPolicySetID
 	}

policy_set_version.go

@@ -47,7 +47,7 @@ const (
 // PolicySetVersionStatus represents a policy set version status.
 type PolicySetVersionStatus string
 
-//List all available policy set version statuses.
+// List all available policy set version statuses.
 const (
 	PolicySetVersionErrored    PolicySetVersionStatus = "errored"
 	PolicySetVersionIngressing PolicySetVersionStatus = "ingressing"

registry_module.go

@@ -214,7 +214,7 @@ func (o RegistryModuleCreateVersionOptions) valid() error {
 }
 
 // Create a new registry module version
-func (r *registryModules) CreateVersion(ctx context.Context, organization string, name string, provider string, options RegistryModuleCreateVersionOptions) (*RegistryModuleVersion, error) {
+func (r *registryModules) CreateVersion(ctx context.Context, organization, name, provider string, options RegistryModuleCreateVersionOptions) (*RegistryModuleVersion, error) {
 	if !validStringID(&organization) {
 		return nil, ErrInvalidOrg
 	}
@@ -313,7 +313,7 @@ func (r *registryModules) CreateWithVCSConnection(ctx context.Context, options R
 }
 
 // Read a specific registry module
-func (r *registryModules) Read(ctx context.Context, organization string, name string, provider string) (*RegistryModule, error) {
+func (r *registryModules) Read(ctx context.Context, organization, name, provider string) (*RegistryModule, error) {
 	if !validStringID(&organization) {
 		return nil, ErrInvalidOrg
 	}
@@ -351,7 +351,7 @@ func (r *registryModules) Read(ctx context.Context, organization string, name st
 }
 
 // Delete is used to delete the entire registry module
-func (r *registryModules) Delete(ctx context.Context, organization string, name string) error {
+func (r *registryModules) Delete(ctx context.Context, organization, name string) error {
 	if !validStringID(&organization) {
 		return ErrInvalidOrg
 	}
@@ -376,7 +376,7 @@ func (r *registryModules) Delete(ctx context.Context, organization string, name
 }
 
 // DeleteProvider is used to delete the specific registry module provider
-func (r *registryModules) DeleteProvider(ctx context.Context, organization string, name string, provider string) error {
+func (r *registryModules) DeleteProvider(ctx context.Context, organization, name, provider string) error {
 	if !validStringID(&organization) {
 		return ErrInvalidOrg
 	}
@@ -408,7 +408,7 @@ func (r *registryModules) DeleteProvider(ctx context.Context, organization strin
 }
 
 // DeleteVersion is used to delete the specific registry module version
-func (r *registryModules) DeleteVersion(ctx context.Context, organization string, name string, provider string, version string) error {
+func (r *registryModules) DeleteVersion(ctx context.Context, organization, name, provider, version string) error {
 	if !validStringID(&organization) {
 		return ErrInvalidOrg
 	}

run.go

@@ -48,7 +48,7 @@ type runs struct {
 // RunStatus represents a run state.
 type RunStatus string
 
-//List all available run statuses.
+// List all available run statuses.
 const (
 	RunApplied            RunStatus = "applied"
 	RunApplyQueued        RunStatus = "apply_queued"

run_task.go

@@ -267,7 +267,7 @@ func (s *runTasks) Delete(ctx context.Context, runTaskID string) error {
 }
 
 // Convenient method to attach a run task to a workspace. See: WorkspaceRunTasks.Create()
-func (s *runTasks) AttachToWorkspace(ctx context.Context, workspaceID string, runTaskID string, enforcement TaskEnforcementLevel) (*WorkspaceRunTask, error) {
+func (s *runTasks) AttachToWorkspace(ctx context.Context, workspaceID, runTaskID string, enforcement TaskEnforcementLevel) (*WorkspaceRunTask, error) {
 	return s.client.WorkspaceRunTasks.Create(ctx, workspaceID, WorkspaceRunTaskCreateOptions{
 		EnforcementLevel: enforcement,
 		RunTask:          &RunTask{ID: runTaskID},

state_version.go

@@ -74,7 +74,6 @@ type StateVersionListOptions struct {
 	Workspace    string `url:"filter[workspace][name]"`
 }
 
-//check that StateVersionListOptions fields had valid values
 func (o *StateVersionListOptions) valid() error {
 	if o == nil {
 		return ErrRequiredStateVerListOps
@@ -247,8 +246,8 @@ func (s *stateVersions) ReadCurrent(ctx context.Context, workspaceID string) (*S
 }
 
 // Download retrieves the actual stored state of a state version
-func (s *stateVersions) Download(ctx context.Context, url string) ([]byte, error) {
-	req, err := s.client.newRequest("GET", url, nil)
+func (s *stateVersions) Download(ctx context.Context, u string) ([]byte, error) {
+	req, err := s.client.newRequest("GET", u, nil)
 	if err != nil {
 		return nil, err
 	}

state_version_output.go

@@ -9,9 +9,9 @@ import (
 // Compile-time proof of interface implementation.
 var _ StateVersionOutputs = (*stateVersionOutputs)(nil)
 
-//State version outputs are the output values from a Terraform state file.
-//They include the name and value of the output, as well as a sensitive boolean
-//if the value should be hidden by default in UIs.
+// State version outputs are the output values from a Terraform state file.
+// They include the name and value of the output, as well as a sensitive boolean
+// if the value should be hidden by default in UIs.
 //
 // TFE API docs: https://www.terraform.io/docs/cloud/api/state-version-outputs.html
 type StateVersionOutputs interface {

task_result.go

@@ -21,10 +21,10 @@ type taskResults struct {
 	client *Client
 }
 
-//TaskResultStatus is an enum that represents all possible statuses for a task result
+// TaskResultStatus is an enum that represents all possible statuses for a task result
 type TaskResultStatus string
 
-//TaskEnforcementLevel is an enum that describes the enforcement levels for a run task
+// TaskEnforcementLevel is an enum that describes the enforcement levels for a run task
 type TaskEnforcementLevel string
 
 const (