chore: Fix golangci-lint Configuration #3645

X-Guardian · 2023-08-04T10:03:40Z

what

Fixes the golangci-lint configuration by making the following changes:

Adds a run timeout of 10 minutes. The current lint run takes longer than the default of 1 minute.
Removes the following deprecated linters:
- deadcode
- structcheck
- varcheck
Removes the separate revive and errcheck lint jobs, as these were always attempting to run all the linters each time, due to this repo having a .golintci.yml file with explicit enabled linters. See: a config file has high priority than command-line options --disable-all golangci/golangci-lint#1972
Fix the paths in the lint GitHub Workflow config.
Fix the paths-ignore in the lint-required GitHub Workflow config.

Note: linter / runner / revive and linter / runner / errcheck will need removing from the required status checks in any branch protection rules to merge this PR.

why

Fixes bug: golangci-lint job is failing in the PR Validation CI #3638

Tests

New golangci-lint output:

Details

/home/runner/work/_temp/reviewdog-lI3Dp4/golangci-lint-1.53.3-linux-amd64/golangci-lint run --out-format line-number
  server/events/vcs/gitlab_client_test.go:389:33: Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)
  							json.NewEncoder(w).Encode(v)
  							                         ^
  server/events/vcs/gitlab_client_test.go:498:36: Error return value of `(*encoding/json.Decoder).Decode` is not checked (errcheck)
  					json.NewDecoder(r.Body).Decode(&body)
  					                              ^
  server/core/config/raw/step.go:40: File is not `gofmt`-ed with `-s` (gofmt)
  //       name: test
  //       command: echo 312
  //       value: value
  server/events/markdown_renderer_test.go:567: File is not `gofmt`-ed with `-s` (gofmt)
  							models.PolicySetResult{
  server/events/webhooks/slack.go:44:29: unused-parameter: parameter 'log' seems to be unused, consider removing or renaming it as _ (revive)
  func (s *SlackWebhook) Send(log logging.SimpleLogging, applyResult ApplyResult) error {
                              ^
  server/events/vcs/bitbucketcloud/client.go:88:79: unused-parameter: parameter 'command' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, command string) error {
                                                                                ^
  server/events/vcs/bitbucketcloud/client.go:109:42: unused-parameter: parameter 'repo' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error {
                                           ^
  server/events/vcs/bitbucketcloud/client.go:141:77: unused-parameter: parameter 'vcsstatusname' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
                                                                              ^
  server/events/vcs/bitbucketcloud/client.go:210:53: unused-parameter: parameter 'pullOptions' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error {
                                                      ^
  server/events/vcs/bitbucketcloud/client.go:237:33: unused-parameter: parameter 'repo' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) DiscardReviews(repo models.Repo, pull models.PullRequest) error {
                                  ^
  server/events/vcs/bitbucketcloud/client.go:266:38: unused-parameter: parameter 'repo' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
                                       ^
  server/events/vcs/bitbucketcloud/client.go:277:33: unused-parameter: parameter 'pull' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) {
                                  ^
  server/events/vcs/bitbucketcloud/client.go:281:30: unused-parameter: parameter 'VCSHostType' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) {
                               ^
  server/events/vcs/bitbucketserver/client.go:136:79: unused-parameter: parameter 'command' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, command string) error {
                                                                                ^
  server/events/vcs/bitbucketserver/client.go:205:77: unused-parameter: parameter 'vcsstatusname' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
                                                                              ^
  server/events/vcs/bitbucketserver/client.go:361:33: unused-parameter: parameter 'pull' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) {
                                  ^
  server/events/vcs/bitbucketserver/client.go:365:30: unused-parameter: parameter 'VCSHostType' seems to be unused, consider removing or renaming it as _ (revive)
  func (b *Client) GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) {
                               ^
  server/core/locking/locking.go:[169](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:174):66: unused-parameter: parameter 'pull' seems to be unused, consider removing or renaming it as _ (revive)
  func (c *NoOpLocker) TryLock(p models.Project, workspace string, pull models.PullRequest, user models.User) (TryLockResponse, error) {
                                                                   ^
  server/core/locking/locking.go:177:29: unused-parameter: parameter 'key' seems to be unused, consider removing or renaming it as _ (revive)
  func (c *NoOpLocker) Unlock(key string) (*models.ProjectLock, error) {
                              ^
  server/core/locking/locking.go:189:35: unused-parameter: parameter 'repoFullName' seems to be unused, consider removing or renaming it as _ (revive)
  func (c *NoOpLocker) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
                                    ^
  server/core/locking/locking.go:197:30: unused-parameter: parameter 'key' seems to be unused, consider removing or renaming it as _ (revive)
  func (c *NoOpLocker) GetLock(key string) (*models.ProjectLock, error) {
                               ^
  server/jobs/project_command_output_handler.go:270:41: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) Send(ctx command.ProjectContext, msg string, isOperationComplete bool) {
                                          ^
  server/jobs/project_command_output_handler.go:273:53: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) SendWorkflowHook(ctx models.WorkflowHookCommandContext, msg string, operationComplete bool) {
                                                      ^
  server/jobs/project_command_output_handler.go:276:45: unused-parameter: parameter 'jobID' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) Register(jobID string, receiver chan string)   {}
                                              ^
  server/jobs/project_command_output_handler.go:277:47: unused-parameter: parameter 'jobID' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) Deregister(jobID string, receiver chan string) {}
                                                ^
  server/jobs/project_command_output_handler.go:282:44: unused-parameter: parameter 'pullInfo' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) CleanUp(pullInfo PullInfo) {
                                             ^
  server/jobs/project_command_output_handler.go:285:48: unused-parameter: parameter 'key' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *NoopProjectOutputHandler) IsKeyExists(key string) bool {
                                                 ^
  server/core/redis/redis.go:59:42: unused-parameter: parameter 'bucket' seems to be unused, consider removing or renaming it as _ (revive)
  func NewWithClient(client *redis.Client, bucket string, globalBucket string) (*RedisDB, error) {
                                           ^
  server/core/db/boltdb_test.go:289:2: redefines-builtin-id: redefinition of the built-in function new (revive)
  	new := lock
  	^
  server/core/db/boltdb_test.go:386:2: redefines-builtin-id: redefinition of the built-in function new (revive)
  	new := lock
  	^
  server/jobs/project_command_output_handler_test.go:194:4: empty-block: this block is empty, you can remove it (revive)
  			for range ch {
  			}
  server/jobs/project_command_output_handler_test.go:228:4: empty-block: this block is empty, you can remove it (revive)
  			for range ch {
  			}
  server/jobs/project_command_output_handler_test.go:243:4: empty-block: this block is empty, you can remove it (revive)
  			for range ch2 {
  			}
  server/core/redis/redis_test.go:325:2: redefines-builtin-id: redefinition of the built-in function new (revive)
  	new := lock
  	^
  server/core/runtime/runtime.go:67:53: unused-parameter: parameter 'extraArgs' seems to be unused, consider removing or renaming it as _ (revive)
  func (p NullRunner) Run(ctx command.ProjectContext, extraArgs []string, path string, envs map[string]string) (string, error) {
                                                      ^
  server/core/runtime/version_step_runner.go:17:61: unused-parameter: parameter 'extraArgs' seems to be unused, consider removing or renaming it as _ (revive)
  func (v *VersionStepRunner) Run(ctx command.ProjectContext, extraArgs []string, path string, envs map[string]string) (string, error) {
                                                              ^
  server/core/runtime/show_step_runner.go:30:58: unused-parameter: parameter 'extraArgs' seems to be unused, consider removing or renaming it as _ (revive)
  func (p *showStepRunner) Run(ctx command.ProjectContext, extraArgs []string, path string, envs map[string]string) (string, error) {
                                                           ^
  server/core/runtime/plan_step_runner_test.go:548:42: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
  func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line) {
                                           ^
  server/events/vcs/gitlab_client.go:[173](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:178):85: unused-parameter: parameter 'command' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment string, command string) error {
                                                                                      ^
  server/events/vcs/github_client.go:441:13: superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (revive)
  					} else {
  						return false, nil
  					}
  server/events/vcs/gitlab_client_test.go:128:7: increment-decrement: should replace numAttempts += 1 with numAttempts++ (revive)
  						numAttempts += 1
  						^
  server/events/vcs/github_client.go:202:57: unused-parameter: parameter 'pullNum' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *GithubClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error {
                                                          ^
  server/events/vcs/github_client.go:581:59: unused-parameter: parameter 'pullOptions' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *GithubClient) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error {
                                                            ^
  server/events/vcs/github_client.go:715:36: unused-parameter: parameter 'VCSHostType' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *GithubClient) GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) {
                                     ^
  server/events/vcs/azuredevops_client.go:[179](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:184):88: unused-parameter: parameter 'vcsstatusname' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *AzureDevopsClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
                                                                                         ^
  server/events/event_parser.go:602:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
  	} else {
  		return models.OtherPullEvent
  	}
  server/events/command_runner.go:364:2: unused-parameter: parameter 'user' seems to be unused, consider removing or renaming it as _ (revive)
  	user models.User,
  	^
  server/events/unlock_command_runner.go:30:2: unused-parameter: parameter 'cmd' seems to be unused, consider removing or renaming it as _ (revive)
  	cmd *CommentCommand,
  	^
  server/events/project_command_builder.go:115:2: unused-parameter: parameter 'logger' seems to be unused, consider removing or renaming it as _ (revive)
  	logger logging.SimpleLogging,
  	^
  server/controllers/locks_controller.go:35:60: unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it as _ (revive)
  func (l *LocksController) LockApply(w http.ResponseWriter, r *http.Request) {
                                                             ^
  server/controllers/github_app_controller.go:88:58: unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it as _ (revive)
  func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {
                                                           ^
  server/controllers/locks_controller.go:47:62: unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it as _ (revive)
  func (l *LocksController) UnlockApply(w http.ResponseWriter, r *http.Request) {
                                                               ^
  server/events/plan_command_runner_test.go:[195](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:200):26: error-strings: error strings should not be capitalized or end with punctuation or a newline (revive)
  					Error:   errors.New("Shabang!"),
  					                    ^
  server/events/plan_command_runner_test.go:224:26: error-strings: error strings should not be capitalized or end with punctuation or a newline (revive)
  					Error:   errors.New("Shabang!"),
  					                    ^
  server/events/plan_command_runner_test.go:499:9: type `RepoModel` is unused (unused)
  			type RepoModel interface{ models.Repo }
  			     ^
  server/core/config/raw/global_cfg.go:283:77: S1002: should omit comparison to bool constant, can be simplified to `!*r.PolicyCheck` (gosimple)
  		if globalReq == valid.PoliciesPassedCommandReq && r.PolicyCheck != nil && *r.PolicyCheck == false {
  		                                                                          ^
  server/core/config/raw/global_cfg.go:297:77: S1002: should omit comparison to bool constant, can be simplified to `!*r.PolicyCheck` (gosimple)
  		if globalReq == valid.PoliciesPassedCommandReq && r.PolicyCheck != nil && *r.PolicyCheck == false {
  		                                                                          ^
  server/core/config/raw/global_cfg.go:311:77: S1002: should omit comparison to bool constant, can be simplified to `!*r.PolicyCheck` (gosimple)
  		if globalReq == valid.PoliciesPassedCommandReq && r.PolicyCheck != nil && *r.PolicyCheck == false {
  		                                                                          ^
  server/events/project_command_context_builder.go:[197](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:202):32: S1002: should omit comparison to bool constant, can be simplified to `prjCfg.PolicyCheck` (gosimple)
  	if cmdName == command.Plan && prjCfg.PolicyCheck != false {
  	                              ^
  server/events/project_finder.go:[227](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:232):18: SA1019: pm.Matches is deprecated: This implementation is buggy (it only checks a single parent dir against the pattern) and will be removed soon. Use either MatchesOrParentMatches or MatchesUsingParentResults instead. (staticcheck)
  			match, err := pm.Matches(file)
  			              ^
  server/events/project_finder.go:[269](https://github.com/runatlantis/atlantis/actions/runs/5760958175/job/15617928555?pr=3645#step:3:274):17: SA1019: patternMatcher.Matches is deprecated: This implementation is buggy (it only checks a single parent dir against the pattern) and will be removed soon. Use either MatchesOrParentMatches or MatchesUsingParentResults instead. (staticcheck)
  		match, err := patternMatcher.Matches(fileName)
  		              ^
  server/events/vcs/gitlab_client.go:314:11: SA1019: mr.MergeStatus is deprecated: This parameter is replaced by DetailedMergeStatus in GitLab 15.6. (staticcheck)
  		(!ok && mr.MergeStatus == "can_be_merged")) &&
  		        ^
  server/events/project_command_builder.go:737:32: SA4001: *&x will be simplified to x. It will not copy x. (staticcheck)
  		abortOnExcecutionOrderFail = *&repoCfgPtr.AbortOnExcecutionOrderFail
  		                             ^
  /home/runner/work/_temp/reviewdog-lI3Dp4/reviewdog -f=golangci-lint -name=golangci-lint -reporter=github-pr-check -filter-mode=added -fail-on-error=false -level=error
  reviewdog: This GitHub token doesn't have write permission of Review API [1], 
  so reviewdog will report results via logging command [2] and create annotations similar to
  github-pr-check reporter as a fallback.
  [1]: https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request_target, 
  [2]: https://help.github.com/en/actions/automating-your-workflow-with-github-actions/development-tools-for-github-actions#logging-commands
  reviewdog: Reporting results for "golangci-lint"
  reviewdog: No results found for "golangci-lint". 63 results found outside diff.

GenPage

Thank you for cleaning this up. We'll work on getting the required checks removed before merging.

* Fix golang-ci * Update lint workflows * Removed required checks

X-Guardian added 2 commits August 4, 2023 10:41

Fix golang-ci

617a89f

Update lint workflows

19d097c

X-Guardian requested a review from a team as a code owner August 4, 2023 10:03

github-actions bot added the github-actions label Aug 4, 2023

GenPage approved these changes Aug 4, 2023

View reviewed changes

GenPage merged commit 9789aff into runatlantis:main Aug 8, 2023

X-Guardian mentioned this pull request Sep 5, 2023

chore(atlantis): fix linter errors #3690

Merged

ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024

chore: Fix golangci-lint Configuration (runatlantis#3645)

214351c

* Fix golang-ci * Update lint workflows * Removed required checks

ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024

chore: Fix golangci-lint Configuration (runatlantis#3645)

96c3d9a

* Fix golang-ci * Update lint workflows * Removed required checks

terakoya76 pushed a commit to terakoya76/atlantis that referenced this pull request Dec 31, 2024

chore: Fix golangci-lint Configuration (runatlantis#3645)

30a9820

* Fix golang-ci * Update lint workflows * Removed required checks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Fix golangci-lint Configuration #3645

chore: Fix golangci-lint Configuration #3645

X-Guardian commented Aug 4, 2023 •

edited

Loading

GenPage left a comment

chore: Fix golangci-lint Configuration #3645

chore: Fix golangci-lint Configuration #3645

Conversation

X-Guardian commented Aug 4, 2023 • edited Loading

what

why

Tests

GenPage left a comment

Choose a reason for hiding this comment

X-Guardian commented Aug 4, 2023 •

edited

Loading