chore(atlantis): fix linter errors (#3690)

* chore(atlantis): fix linter errors

* fix superfluous-else

* revert gitlab check

* ignore stub funcs on azuredevops_client

* remove fetch-depth

* remove fail_on_error

* fix plan_step_runner_test.go

* more lint fixes

---------

Co-authored-by: Dylan Page <dylan.page@autodesk.com>

.golangci.yml

@@ -7,22 +7,26 @@ linters-settings:
     ignore-words:
       # for gitlab notes api
       - noteable
+  revive:
+    rules:
+      - name: dot-imports
+        disabled: true
 
 linters:
   enable:
-  - errcheck
-  - gochecknoinits
-  - gofmt
-  - gosec
-  - gosimple
-  - ineffassign
-  - misspell
-  - revive
-  - staticcheck
-  - typecheck
-  - unconvert
-  - unused
-  - vet
-  - vetshadow
+    - errcheck
+    - gochecknoinits
+    - gofmt
+    - gosec
+    - gosimple
+    - ineffassign
+    - misspell
+    - revive
+    - staticcheck
+    - typecheck
+    - unconvert
+    - unused
+    - vet
+    - vetshadow
 run:
   timeout: 10m

cmd/server_test.go

@@ -38,7 +38,7 @@ var passedConfig server.UserConfig
 
 type ServerCreatorMock struct{}
 
-func (s *ServerCreatorMock) NewServer(userConfig server.UserConfig, config server.Config) (ServerStarter, error) {
+func (s *ServerCreatorMock) NewServer(userConfig server.UserConfig, _ server.Config) (ServerStarter, error) {
 	passedConfig = userConfig
 	return &ServerStarterMock{}, nil
 }

server/controllers/events/events_controller_e2e_test.go

@@ -53,11 +53,11 @@ var mockPreWorkflowHookRunner *runtimemocks.MockPreWorkflowHookRunner
 
 var mockPostWorkflowHookRunner *runtimemocks.MockPostWorkflowHookRunner
 
-func (m *NoopTFDownloader) GetFile(dst, src string) error {
+func (m *NoopTFDownloader) GetFile(_, _ string) error {
 	return nil
 }
 
-func (m *NoopTFDownloader) GetAny(dst, src string) error {
+func (m *NoopTFDownloader) GetAny(_, _ string) error {
 	return nil
 }
 
@@ -1572,13 +1572,13 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 
 type mockLockURLGenerator struct{}
 
-func (m *mockLockURLGenerator) GenerateLockURL(lockID string) string {
+func (m *mockLockURLGenerator) GenerateLockURL(_ string) string {
 	return "lock-url"
 }
 
 type mockWebhookSender struct{}
 
-func (w *mockWebhookSender) Send(log logging.SimpleLogging, result webhooks.ApplyResult) error {
+func (w *mockWebhookSender) Send(_ logging.SimpleLogging, _ webhooks.ApplyResult) error {
 	return nil
 }
 

server/controllers/github_app_controller.go

@@ -85,7 +85,7 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques
 }
 
 // New redirects the user to create a new GitHub app
-func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {
+func (g *GithubAppController) New(w http.ResponseWriter, _ *http.Request) {
 
 	if g.GithubSetupComplete {
 		g.respond(w, logging.Error, http.StatusBadRequest, "Atlantis already has GitHub credentials")

server/controllers/locks_controller.go

@@ -32,7 +32,7 @@ type LocksController struct {
 
 // LockApply handles creating a global apply lock.
 // If Lock already exists it will be a no-op
-func (l *LocksController) LockApply(w http.ResponseWriter, r *http.Request) {
+func (l *LocksController) LockApply(w http.ResponseWriter, _ *http.Request) {
 	lock, err := l.ApplyLocker.LockApply()
 	if err != nil {
 		l.respond(w, logging.Error, http.StatusInternalServerError, "creating apply lock failed with: %s", err)
@@ -44,7 +44,7 @@ func (l *LocksController) LockApply(w http.ResponseWriter, r *http.Request) {
 
 // UnlockApply handles releasing a global apply lock.
 // If Lock doesn't exists it will be a no-op
-func (l *LocksController) UnlockApply(w http.ResponseWriter, r *http.Request) {
+func (l *LocksController) UnlockApply(w http.ResponseWriter, _ *http.Request) {
 	err := l.ApplyLocker.UnlockApply()
 	if err != nil {
 		l.respond(w, logging.Error, http.StatusInternalServerError, "deleting apply lock failed with: %s", err)

server/controllers/status_controller.go

@@ -23,7 +23,7 @@ type StatusResponse struct {
 }
 
 // Get is the GET /status route.
-func (d *StatusController) Get(w http.ResponseWriter, r *http.Request) {
+func (d *StatusController) Get(w http.ResponseWriter, _ *http.Request) {
 	status := d.Drainer.GetStatus()
 	data, err := json.MarshalIndent(&StatusResponse{
 		ShuttingDown:    status.ShuttingDown,

server/core/config/raw/step.go

@@ -37,16 +37,16 @@ const (
 //
 // 2. A map for an env step with name and command or value, or a run step with a command and output config
 //   - env:
-//       name: test
-//       command: echo 312
-//       value: value
+//     name: test
+//     command: echo 312
+//     value: value
 //   - run:
-//       command: my custom command
-//       output: hide
+//     command: my custom command
+//     output: hide
 //
 // 3. A map for a built-in command and extra_args:
 //   - plan:
-//       extra_args: [-var-file=staging.tfvars]
+//     extra_args: [-var-file=staging.tfvars]
 //
 // 4. A map for a custom run command:
 //   - run: my custom command

server/core/db/boltdb_test.go

@@ -286,9 +286,9 @@ func TestUnlockingMultiple(t *testing.T) {
 	_, _, err := b.TryLock(lock)
 	Ok(t, err)
 
-	new := lock
-	new.Project.RepoFullName = "new/repo"
-	_, _, err = b.TryLock(new)
+	new1 := lock
+	new1.Project.RepoFullName = "new/repo"
+	_, _, err = b.TryLock(new1)
 	Ok(t, err)
 
 	new2 := lock
@@ -306,7 +306,7 @@ func TestUnlockingMultiple(t *testing.T) {
 	Ok(t, err)
 	_, err = b.Unlock(new2.Project, workspace)
 	Ok(t, err)
-	_, err = b.Unlock(new.Project, workspace)
+	_, err = b.Unlock(new1.Project, workspace)
 	Ok(t, err)
 	_, err = b.Unlock(project, workspace)
 	Ok(t, err)
@@ -383,9 +383,9 @@ func TestUnlockByPullMatching(t *testing.T) {
 	Ok(t, err)
 
 	// add additional locks with the same repo and pull num but different paths/workspaces
-	new := lock
-	new.Project.Path = "dif/path"
-	_, _, err = b.TryLock(new)
+	new1 := lock
+	new1.Project.Path = "dif/path"
+	_, _, err = b.TryLock(new1)
 	Ok(t, err)
 	new2 := lock
 	new2.Workspace = "new-workspace"

server/core/locking/locking.go

@@ -166,15 +166,15 @@ func NewNoOpLocker() *NoOpLocker {
 }
 
 // TryLock attempts to acquire a lock to a project and workspace.
-func (c *NoOpLocker) TryLock(p models.Project, workspace string, pull models.PullRequest, user models.User) (TryLockResponse, error) {
+func (c *NoOpLocker) TryLock(p models.Project, workspace string, _ models.PullRequest, _ models.User) (TryLockResponse, error) {
 	return TryLockResponse{true, models.ProjectLock{}, c.key(p, workspace)}, nil
 }
 
 // Unlock attempts to unlock a project and workspace. If successful,
 // a pointer to the now deleted lock will be returned. Else, that
 // pointer will be nil. An error will only be returned if there was
 // an error deleting the lock (i.e. not if there was no lock).
-func (c *NoOpLocker) Unlock(key string) (*models.ProjectLock, error) {
+func (c *NoOpLocker) Unlock(_ string) (*models.ProjectLock, error) {
 	return &models.ProjectLock{}, nil
 }
 
@@ -186,15 +186,15 @@ func (c *NoOpLocker) List() (map[string]models.ProjectLock, error) {
 }
 
 // UnlockByPull deletes all locks associated with that pull request.
-func (c *NoOpLocker) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
+func (c *NoOpLocker) UnlockByPull(_ string, _ int) ([]models.ProjectLock, error) {
 	return []models.ProjectLock{}, nil
 }
 
 // GetLock attempts to get the lock stored at key. If successful,
 // a pointer to the lock will be returned. Else, the pointer will be nil.
 // An error will only be returned if there was an error getting the lock
 // (i.e. not if there was no lock).
-func (c *NoOpLocker) GetLock(key string) (*models.ProjectLock, error) {
+func (c *NoOpLocker) GetLock(_ string) (*models.ProjectLock, error) {
 	return nil, nil
 }
 

server/core/redis/redis.go

@@ -56,7 +56,7 @@ func New(hostname string, port int, password string, tlsEnabled bool, insecureSk
 }
 
 // NewWithClient is used for testing.
-func NewWithClient(client *redis.Client, bucket string, globalBucket string) (*RedisDB, error) {
+func NewWithClient(client *redis.Client, _ string, _ string) (*RedisDB, error) {
 	return &RedisDB{
 		client: client,
 	}, nil
@@ -82,12 +82,12 @@ func (r *RedisDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock,
 	} else if err != nil {
 		// otherwise the lock fails, return to caller the run that's holding the lock
 		return false, currLock, errors.Wrap(err, "db transaction failed")
-	} else {
-		if err := json.Unmarshal([]byte(val), &currLock); err != nil {
-			return false, currLock, errors.Wrap(err, "failed to deserialize current lock")
-		}
-		return false, currLock, nil
 	}
+
+	if err := json.Unmarshal([]byte(val), &currLock); err != nil {
+		return false, currLock, errors.Wrap(err, "failed to deserialize current lock")
+	}
+	return false, currLock, nil
 }
 
 // Unlock attempts to unlock the project and workspace.
@@ -103,13 +103,13 @@ func (r *RedisDB) Unlock(project models.Project, workspace string) (*models.Proj
 		return nil, nil
 	} else if err != nil {
 		return nil, errors.Wrap(err, "db transaction failed")
-	} else {
-		if err := json.Unmarshal([]byte(val), &lock); err != nil {
-			return nil, errors.Wrap(err, "failed to deserialize current lock")
-		}
-		r.client.Del(ctx, key)
-		return &lock, nil
 	}
+
+	if err := json.Unmarshal([]byte(val), &lock); err != nil {
+		return nil, errors.Wrap(err, "failed to deserialize current lock")
+	}
+	r.client.Del(ctx, key)
+	return &lock, nil
 }
 
 // List lists all current locks.
@@ -144,15 +144,15 @@ func (r *RedisDB) GetLock(project models.Project, workspace string) (*models.Pro
 		return nil, nil
 	} else if err != nil {
 		return nil, errors.Wrap(err, "db transaction failed")
-	} else {
-		var lock models.ProjectLock
-		if err := json.Unmarshal([]byte(val), &lock); err != nil {
-			return nil, errors.Wrapf(err, "deserializing lock at key %q", key)
-		}
-		// need to set it to Local after deserialization due to https://github.com/golang/go/issues/19486
-		lock.Time = lock.Time.Local()
-		return &lock, nil
 	}
+
+	var lock models.ProjectLock
+	if err := json.Unmarshal([]byte(val), &lock); err != nil {
+		return nil, errors.Wrapf(err, "deserializing lock at key %q", key)
+	}
+	// need to set it to Local after deserialization due to https://github.com/golang/go/issues/19486
+	lock.Time = lock.Time.Local()
+	return &lock, nil
 }
 
 // UnlockByPull deletes all locks associated with that pull request and returns them.
@@ -203,9 +203,9 @@ func (r *RedisDB) LockCommand(cmdName command.Name, lockTime time.Time) (*comman
 		return &lock, errors.Wrap(err, "db transaction failed")
 	} else if err != nil {
 		return nil, errors.Wrap(err, "db transaction failed")
-	} else {
-		return nil, errors.New("db transaction failed: lock already exists")
 	}
+
+	return nil, errors.New("db transaction failed: lock already exists")
 }
 
 func (r *RedisDB) UnlockCommand(cmdName command.Name) error {
@@ -215,9 +215,10 @@ func (r *RedisDB) UnlockCommand(cmdName command.Name) error {
 		return errors.New("db transaction failed: no lock exists")
 	} else if err != nil {
 		return errors.Wrap(err, "db transaction failed")
-	} else {
-		return r.client.Del(ctx, cmdLockKey).Err()
 	}
+
+	return r.client.Del(ctx, cmdLockKey).Err()
+
 }
 
 func (r *RedisDB) CheckCommandLock(cmdName command.Name) (*command.Lock, error) {
@@ -229,12 +230,12 @@ func (r *RedisDB) CheckCommandLock(cmdName command.Name) (*command.Lock, error)
 		return nil, nil
 	} else if err != nil {
 		return nil, errors.Wrap(err, "db transaction failed")
-	} else {
-		if err := json.Unmarshal([]byte(val), &cmdLock); err != nil {
-			return nil, errors.Wrap(err, "failed to deserialize Lock")
-		}
-		return &cmdLock, err
 	}
+
+	if err := json.Unmarshal([]byte(val), &cmdLock); err != nil {
+		return nil, errors.Wrap(err, "failed to deserialize Lock")
+	}
+	return &cmdLock, err
 }
 
 // UpdatePullWithResults updates pull's status with the latest project results.
@@ -367,13 +368,13 @@ func (r *RedisDB) getPull(key string) (*models.PullStatus, error) {
 		return nil, nil
 	} else if err != nil {
 		return nil, errors.Wrap(err, "db transaction failed")
-	} else {
-		var p models.PullStatus
-		if err := json.Unmarshal([]byte(val), &p); err != nil {
-			return nil, errors.Wrapf(err, "deserializing pull at %q with contents %q", key, val)
-		}
-		return &p, nil
 	}
+
+	var p models.PullStatus
+	if err := json.Unmarshal([]byte(val), &p); err != nil {
+		return nil, errors.Wrapf(err, "deserializing pull at %q with contents %q", key, val)
+	}
+	return &p, nil
 }
 
 func (r *RedisDB) writePull(key string, pull models.PullStatus) error {

server/core/redis/redis_test.go

@@ -322,9 +322,9 @@ func TestUnlockingMultiple(t *testing.T) {
 	_, _, err := rdb.TryLock(lock)
 	Ok(t, err)
 
-	new := lock
-	new.Project.RepoFullName = "new/repo"
-	_, _, err = rdb.TryLock(new)
+	new1 := lock
+	new1.Project.RepoFullName = "new/repo"
+	_, _, err = rdb.TryLock(new1)
 	Ok(t, err)
 
 	new2 := lock
@@ -342,7 +342,7 @@ func TestUnlockingMultiple(t *testing.T) {
 	Ok(t, err)
 	_, err = rdb.Unlock(new2.Project, workspace)
 	Ok(t, err)
-	_, err = rdb.Unlock(new.Project, workspace)
+	_, err = rdb.Unlock(new1.Project, workspace)
 	Ok(t, err)
 	_, err = rdb.Unlock(project, workspace)
 	Ok(t, err)
@@ -419,9 +419,9 @@ func TestUnlockByPullMatching(t *testing.T) {
 	Ok(t, err)
 
 	// add additional locks with the same repo and pull num but different paths/workspaces
-	new := lock
-	new.Project.Path = "dif/path"
-	_, _, err = rdb.TryLock(new)
+	new1 := lock
+	new1.Project.Path = "dif/path"
+	_, _, err = rdb.TryLock(new1)
 	Ok(t, err)
 	new2 := lock
 	new2.Workspace = "new-workspace"

server/core/runtime/apply_step_runner_test.go

@@ -361,7 +361,7 @@ type remoteApplyMock struct {
 }
 
 // RunCommandAsync fakes out running terraform async.
-func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line) {
+func (r *remoteApplyMock) RunCommandAsync(_ command.ProjectContext, _ string, args []string, _ map[string]string, _ *version.Version, _ string) (chan<- string, <-chan runtimemodels.Line) {
 	r.CalledArgs = args
 
 	in := make(chan string)

server/core/runtime/plan_step_runner.go

@@ -296,7 +296,7 @@ locally at this time.
 // using Terraform Cloud remote operations in TF 1.1.0 and above
 var remoteOpsErr110 = `╷
 │ Error: Saving a generated plan is currently not supported
-│ 
+│
 │ Terraform Cloud does not support saving the generated execution plan
 │ locally at this time.
 ╵