chore(lint): linting the codebase (#4668)

Signed-off-by: Rui Chen <rui@chenrui.dev>

.gitattributes

@@ -1,11 +1,2 @@
 # Set the default behavior, in case people don't have core.autocrlf set.
-* text=auto
-
-# Explicitly declare text files you want to always be normalized and converted
-# to native line endings on checkout.
-*.go text
-*.json text
-*.yml text
-*.yaml text
-*.sh text
-*.tf text
+* text=auto eol=lf

.github/renovate.json5

@@ -104,8 +104,8 @@
     {
       customType: 'regex',
       fileMatch: [
-        '.circleci/config.yml$',
         '^\\.github/workflows/[^/]+\\.ya?ml$',
+        'Makefile$',
       ],
       matchStrings: [
         'renovate: datasource=(?<datasource>.*?) depName=(?<depName>.*?)( versioning=(?<versioning>.*?))?\\s.*?_VERSION: (?<currentValue>.*)\\s',

.github/workflows/lint.yml

@@ -15,6 +15,14 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
   cancel-in-progress: true
 
+permissions:
+  # Required: allow read access to the content for analysis.
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
+  # Optional: Allow write access to checks to allow the action to annotate code in the PR.
+  checks: write
+
 jobs:
   changes:
     outputs:
@@ -47,9 +55,10 @@ jobs:
         go-version-file: go.mod
 
     - name: golangci-lint
-      uses: reviewdog/action-golangci-lint@7708105983c614f7a2725e2172908b7709d1c3e4 # v2
+      uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6
       with:
-        tool_name: golangci-lint
+        # renovate: datasource=github-releases depName=golangci/golangci-lint
+        version: v1.59.1
 
   skip-lint:
     needs: [changes]

.golangci.yml

@@ -19,14 +19,14 @@ linters:
     - gofmt
     - gosec
     - gosimple
+    - govet
     - ineffassign
     - misspell
     - revive
     - staticcheck
+    - testifylint
     - typecheck
     - unconvert
     - unused
-    - vet
-    - vetshadow
 run:
   timeout: 10m

Makefile

@@ -6,6 +6,9 @@ IMAGE_NAME := runatlantis/atlantis
 
 .DEFAULT_GOAL := help
 
+# renovate: datasource=github-releases depName=golangci/golangci-lint
+GOLANGCI_LINT_VERSION := v1.59.1
+
 .PHONY: help
 help: ## List targets & descriptions
 	@cat Makefile* | grep -E '^[a-zA-Z\/_-]+:.*?## .*$$' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
@@ -93,7 +96,7 @@ lint: ## Run linter locally
 
 .PHONY: check-lint
 check-lint: ## Run linter in CI/CD. If running locally use 'lint'
-	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin v1.49.0
+	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin $(GOLANGCI_LINT_VERSION)
 	./bin/golangci-lint run -j 4 --timeout 5m
 
 .PHONY: check-fmt

server/controllers/events/events_controller_test.go

@@ -246,8 +246,10 @@ func TestPost_GitlabCommentNotAllowlisted(t *testing.T) {
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 
-	Equals(t, http.StatusForbidden, w.Result().StatusCode)
-	body, _ := io.ReadAll(w.Result().Body)
+	resp := w.Result()
+	defer resp.Body.Close()
+	Equals(t, http.StatusForbidden, resp.StatusCode)
+	body, _ := io.ReadAll(resp.Body)
 	exp := "Repo not allowlisted"
 	Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body))
 	expRepo, _ := models.NewRepo(models.Gitlab, "gitlabhq/gitlab-test", "https://example.com/gitlabhq/gitlab-test.git", "", "")
@@ -279,8 +281,10 @@ func TestPost_GitlabCommentNotAllowlistedWithSilenceErrors(t *testing.T) {
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 
-	Equals(t, http.StatusForbidden, w.Result().StatusCode)
-	body, _ := io.ReadAll(w.Result().Body)
+	resp := w.Result()
+	defer resp.Body.Close()
+	Equals(t, http.StatusForbidden, resp.StatusCode)
+	body, _ := io.ReadAll(resp.Body)
 	exp := "Repo not allowlisted"
 	Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body))
 	vcsClient.VerifyWasCalled(Never()).CreateComment(Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]())
@@ -311,8 +315,10 @@ func TestPost_GithubCommentNotAllowlisted(t *testing.T) {
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 
-	Equals(t, http.StatusForbidden, w.Result().StatusCode)
-	body, _ := io.ReadAll(w.Result().Body)
+	resp := w.Result()
+	defer resp.Body.Close()
+	Equals(t, http.StatusForbidden, resp.StatusCode)
+	body, _ := io.ReadAll(resp.Body)
 	exp := "Repo not allowlisted"
 	Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body))
 	expRepo, _ := models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "")
@@ -345,8 +351,10 @@ func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) {
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 
-	Equals(t, http.StatusForbidden, w.Result().StatusCode)
-	body, _ := io.ReadAll(w.Result().Body)
+	resp := w.Result()
+	defer resp.Body.Close()
+	Equals(t, http.StatusForbidden, resp.StatusCode)
+	body, _ := io.ReadAll(resp.Body)
 	exp := "Repo not allowlisted"
 	Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body))
 	vcsClient.VerifyWasCalled(Never()).CreateComment(Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]())

server/core/runtime/show_step_runner.go

@@ -49,7 +49,7 @@ func (p *showStepRunner) Run(ctx command.ProjectContext, _ []string, path string
 		return "", errors.Wrap(err, "running terraform show")
 	}
 
-	if err := os.WriteFile(showResultFile, []byte(output), os.ModePerm); err != nil {
+	if err := os.WriteFile(showResultFile, []byte(output), 0600); err != nil {
 		return "", errors.Wrap(err, "writing terraform show result")
 	}
 

server/events/command/name_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestName_TitleString(t *testing.T) {
@@ -182,7 +183,7 @@ func TestParseCommandName(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := command.ParseCommandName(tt.name)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, tt.exp, got)
 		})
 	}

server/events/modules_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 //go:embed testdata/fs
@@ -20,9 +21,9 @@ func Test_findModuleDependants(t *testing.T) {
 		autoplanModuleDependants string
 	}
 	a, err := fs.Sub(repos, "testdata/fs/repoA")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	b, err := fs.Sub(repos, "testdata/fs/repoB")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	tests := []struct {
 		name    string

server/events/repo_branch_test.go

@@ -80,7 +80,7 @@ projects:
 	repo, err := parser.ParseRepoCfg(tmp, global, "github.com/foo/bar", "main")
 	require.NoError(t, err)
 
-	require.Equal(t, 1, len(repo.Projects))
+	require.Len(t, repo.Projects, 1)
 
 	t.Logf("Projects: %+v", repo.Projects)
 }

server/events/vcs/bitbucketcloud/client_test.go

@@ -14,6 +14,8 @@ import (
 	. "github.com/runatlantis/atlantis/testing"
 )
 
+const diffstatURL = "/2.0/repositories/owner/repo/pullrequests/1/diffstat"
+
 // Should follow pagination properly.
 func TestClient_GetModifiedFilesPagination(t *testing.T) {
 	logger := logging.NewNoopLogger(t)
@@ -56,12 +58,12 @@ func TestClient_GetModifiedFilesPagination(t *testing.T) {
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch r.RequestURI {
 		// The first request should hit this URL.
-		case "/2.0/repositories/owner/repo/pullrequests/1/diffstat":
-			resp := firstResp + fmt.Sprintf(`,"next": "%s/2.0/repositories/owner/repo/pullrequests/1/diffstat?page=2"}`, serverURL)
+		case diffstatURL:
+			resp := firstResp + fmt.Sprintf(`,"next": "%s%s?page=2"}`, serverURL, diffstatURL)
 			w.Write([]byte(resp)) // nolint: errcheck
 			return
 			// The second should hit this URL.
-		case "/2.0/repositories/owner/repo/pullrequests/1/diffstat?page=2":
+		case fmt.Sprintf("%s?page=2", diffstatURL):
 			w.Write([]byte(secondResp + "}")) // nolint: errcheck
 		default:
 			t.Errorf("got unexpected request at %q", r.RequestURI)
@@ -125,7 +127,7 @@ func TestClient_GetModifiedFilesOldNil(t *testing.T) {
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch r.RequestURI {
 		// The first request should hit this URL.
-		case "/2.0/repositories/owner/repo/pullrequests/1/diffstat":
+		case diffstatURL:
 			w.Write([]byte(resp)) // nolint: errcheck
 			return
 		default:
@@ -322,7 +324,7 @@ func TestClient_PullIsMergeable(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				switch r.RequestURI {
-				case "/2.0/repositories/owner/repo/pullrequests/1/diffstat":
+				case diffstatURL:
 					w.Write([]byte(c.DiffStat)) // nolint: errcheck
 					return
 				default:

server/router_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/models"
 	. "github.com/runatlantis/atlantis/testing"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestRouter_GenerateLockURL(t *testing.T) {
@@ -107,6 +107,6 @@ func TestGenerateProjectJobURL_ShouldReturnErrorWhenJobIDNotSpecified(t *testing
 	}
 	expectedErrString := "no job id in ctx"
 	gotURL, err := router.GenerateProjectJobURL(ctx)
-	assert.EqualError(t, err, expectedErrString)
+	require.EqualError(t, err, expectedErrString)
 	Equals(t, "", gotURL)
 }

server/server_test.go

@@ -139,9 +139,12 @@ func TestHealthz(t *testing.T) {
 	req, _ := http.NewRequest("GET", "/healthz", bytes.NewBuffer(nil))
 	w := httptest.NewRecorder()
 	s.Healthz(w, req)
-	Equals(t, http.StatusOK, w.Result().StatusCode)
-	body, _ := io.ReadAll(w.Result().Body)
-	Equals(t, "application/json", w.Result().Header["Content-Type"][0])
+
+	resp := w.Result()
+	defer resp.Body.Close()
+	Equals(t, http.StatusOK, resp.StatusCode)
+	body, _ := io.ReadAll(resp.Body)
+	Equals(t, "application/json", resp.Header["Content-Type"][0])
 	Equals(t,
 		`{
   "status": "ok"

server/user_config_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/runatlantis/atlantis/server/logging"
 	. "github.com/runatlantis/atlantis/testing"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserConfig_ToAllowCommandNames(t *testing.T) {
@@ -61,7 +62,7 @@ func TestUserConfig_ToAllowCommandNames(t *testing.T) {
 			}
 			got, err := u.ToAllowCommandNames()
 			if err != nil {
-				assert.ErrorContains(t, err, tt.wantErr, "ToAllowCommandNames()")
+				require.ErrorContains(t, err, tt.wantErr, "ToAllowCommandNames()")
 			}
 			assert.Equalf(t, tt.want, got, "ToAllowCommandNames()")
 		})