Add `lint-go-gopls` #30729

silverwind · 2024-04-27T10:08:20Z

Uses gopls check <files> as a linter. Tested locally and brings up 149 errors currently for me. I don't think I want to fix them in this PR, but I would like at least to get this analysis running on CI.

List of errors:

modules/indexer/code/indexer.go:181:11: impossible condition: nil != nil
routers/private/hook_post_receive.go:120:15: tautological condition: nil == nil
services/auth/source/oauth2/providers.go:185:9: tautological condition: nil == nil
services/convert/issue.go:216:11: tautological condition: non-nil != nil
tests/integration/git_test.go:332:9: impossible condition: nil != nil
services/migrations/migrate.go:179:24-43: unused parameter: ctx
services/repository/transfer.go:288:48-69: unused parameter: doer
tests/integration/api_repo_tags_test.go:75:41-61: unused parameter: session
tests/integration/git_test.go:696:64-74: unused parameter: baseBranch
tests/integration/gpg_git_test.go:265:27-39: unused parameter: t
tests/integration/gpg_git_test.go:284:23-29: unused parameter: tmpDir
tests/integration/gpg_git_test.go:284:31-35: unused parameter: name
tests/integration/gpg_git_test.go:284:37-42: unused parameter: email

silverwind · 2024-04-27T11:08:50Z

The gopls check command currently does not set a proper exit code when there are errors, so we would likely have to analyze the output.

Suggested by logs in #30729 - Remove `math/rand.Seed` `rand.Seed is deprecated: As of Go 1.20 there is no reason to call Seed with a random value.` - Replace `math/rand.Read` `rand.Read is deprecated: For almost all use cases, [crypto/rand.Read] is more appropriate.` - Replace `math/rand` with `math/rand/v2`, which is available since Go 1.22

This reverts commit f7b0d9a.

silverwind · 2024-04-27T19:07:33Z

Lint script added which performs error detection and error counting. It should now fail the CI like this and also be much less verbose with module downloads which it apparently does:

/Users/silverwind/git/gitea/modules/git/commit_info_nogogit.go:100:35-54: unused parameter: ctx [windows,amd64]
/Users/silverwind/git/gitea/modules/git/commit_info_nogogit.go:100:35-54: unused parameter: ctx
Found 149 'gopls check' errors
make: *** [Makefile:432: lint-go-gopls] Error 1

tools/lint-go-gopls.sh

silverwind · 2024-04-27T20:30:02Z

Output and failure detection looks good now. I will add || true so we can merge it without fixing the errors.

) Resolve all cases for `unused parameter` and `unnecessary type arguments` Related: #30729 --------- Co-authored-by: Giteabot <teabot@gitea.io>

silverwind · 2024-04-29T10:43:39Z

Will post an update later on the outstanding issues. I think we should only merge this after all issues have been fixed.

silverwind · 2024-04-29T17:21:35Z

49 errors left, #30735 will fix a few.

silverwind · 2024-04-29T17:31:18Z

It seems gopls has no support for //nolint currently, so we need to get more creative for technical debt like this one:

gitea/modules/graceful/manager_windows.go

Lines 44 to 45 in a21ca9b

    
           //lint:ignore SA1019 We use IsAnInteractiveSession because IsWindowsService has a different permissions profile 
        
           isAnInteractiveSession, err := svc.IsAnInteractiveSession() //nolint:staticcheck

silverwind · 2024-04-29T17:58:57Z

It seems all deprecations that are warned here already have //nolint comments, so I'll just ignore any error with "is deprecated". Some of these should defintely be looked at though.

As discovered by #30729. --------- Co-authored-by: Giteabot <teabot@gitea.io>

As discovered by go-gitea/gitea#30729. --------- Co-authored-by: Giteabot <teabot@gitea.io> (cherry picked from commit 610802df85933e7a190a705bc3f7800da87ce868) Conflicts: tests/integration/git_test.go trivial conflict because of https://codeberg.org/forgejo/forgejo/pulls/2834

silverwind · 2024-06-04T19:03:15Z

This is ready now. I fixed all non-deprecation errors and one golang deprecation as well.

For reference, here are the remaining deprecated errors that are ignored:

routers/api/v1/repo/branch.go:237:16-33: opt.OldBranchName is deprecated: true Name of the old branch to create from
routers/api/v1/repo/branch.go:238:37-54: opt.OldBranchName is deprecated: true Name of the old branch to create from
routers/api/v1/repo/branch.go:239:54-71: opt.OldBranchName is deprecated: true Name of the old branch to create from
routers/api/v1/repo/branch.go:520:14-29: form.BranchName is deprecated: true
modules/markup/common/footnote.go:200:13-29: util.FindClosure is deprecated: This function can not handle newlines. Many elements can be existed over multiple lines(e.g. link labels). Use text.Reader.FindClosure.
modules/markup/common/footnote.go:290:13-29: util.FindClosure is deprecated: This function can not handle newlines. Many elements can be existed over multiple lines(e.g. link labels). Use text.Reader.FindClosure.
models/db/sql_postgres_with_schema.go:42:25-38: driver.Execer is deprecated: Drivers should implement [ExecerContext] instead.
models/db/sql_postgres_with_schema.go:67:11-20: stmt.Exec is deprecated: Drivers should implement StmtExecContext instead (or additionally).

* giteaofficial/main: Add `lint-go-gopls` (go-gitea#30729) Make blockquote attention recognize more syntaxes (go-gitea#31240) Fix admin oauth2 custom URL settings (go-gitea#31246) Replace `gt-word-break` with `tw-break-anywhere` (go-gitea#31183) Make pasted "img" tag has the same behavior as markdown image (go-gitea#31235) Remove .segment from .project-column (go-gitea#31204) Fix overflow on push notification (go-gitea#31179) Fix NuGet Package API for $filter with Id equality (go-gitea#31188) Fix overflow on notifications (go-gitea#31178) Update chroma to v2.14.0 (go-gitea#31177) Update air package path (go-gitea#31233) Bump `@github/relative-time-element` to v4.4.1 (go-gitea#31232)

* origin/main: (231 commits) Allow including `Reviewed-on`/`Reviewed-by` lines for custom merge messages (go-gitea#31211) Add `MAX_ROWS` option for CSV rendering (go-gitea#30268) Update `golang.org/x/net` (go-gitea#31260) Add replacement module for `mholt/archiver` (go-gitea#31267) Fix Activity Page Contributors dropdown (go-gitea#31264) Optimize runner-tags layout to enhance visual experience (go-gitea#31258) fix: allow actions artifacts storage migration to complete succesfully (go-gitea#31251) Add `lint-go-gopls` (go-gitea#30729) Make blockquote attention recognize more syntaxes (go-gitea#31240) Fix admin oauth2 custom URL settings (go-gitea#31246) Replace `gt-word-break` with `tw-break-anywhere` (go-gitea#31183) Make pasted "img" tag has the same behavior as markdown image (go-gitea#31235) Remove .segment from .project-column (go-gitea#31204) Fix overflow on push notification (go-gitea#31179) Fix NuGet Package API for $filter with Id equality (go-gitea#31188) Fix overflow on notifications (go-gitea#31178) Update chroma to v2.14.0 (go-gitea#31177) Update air package path (go-gitea#31233) Bump `@github/relative-time-element` to v4.4.1 (go-gitea#31232) Add option for mailer to override mail headers (go-gitea#27860) ...

Add lint-go-gopls

aed16ec

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 27, 2024

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 27, 2024

github-actions bot added the modifies/internal label Apr 27, 2024

silverwind mentioned this pull request Apr 27, 2024

Remove unused parameter for some functions in services/mirror #30724

Merged

use go run

e86f46b

pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 27, 2024

silverwind added 2 commits April 27, 2024 12:19

use exact version

29774f7

fix cmd

2a7bd1a

silverwind added the topic/code-linting label Apr 27, 2024

add to deps-tools

57287d7

harryzcy mentioned this pull request Apr 27, 2024

Replace deprecated math/rand functions #30733

Merged

run build before lint-backend

f7b0d9a

silverwind mentioned this pull request Apr 27, 2024

Fix tautological conditions #30735

Merged

silverwind marked this pull request as draft April 27, 2024 17:15

Revert "run build before lint-backend"

cab07ab

This reverts commit f7b0d9a.

harryzcy mentioned this pull request Apr 27, 2024

Use ProtonMail/go-crypto for opengpg in tests #30736

Merged

add lint script

922fa2c

silverwind marked this pull request as ready for review April 27, 2024 19:07

silverwind commented Apr 27, 2024

View reviewed changes

tools/lint-go-gopls.sh Outdated Show resolved Hide resolved

silverwind added 2 commits April 27, 2024 21:09

Update tools/lint-go-gopls.sh

7e54674

always run script in repo root

399be95

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 27, 2024

hide stderr

3c48b39

Merge branch 'main' into lint-go-gopls

16c87e9

silverwind added 2 commits April 29, 2024 19:41

add IGNORE_PATTERNS

29625cd

ignore all deprecations

e278a0d

silverwind marked this pull request as draft April 29, 2024 21:40

silverwind added a commit that referenced this pull request Apr 30, 2024

Fix tautological conditions (#30735)

610802d

As discovered by #30729. --------- Co-authored-by: Giteabot <teabot@gitea.io>

silverwind added 2 commits April 30, 2024 14:41

Merge branch 'main' into lint-go-gopls

b8229cd

Merge branch 'main' into lint-go-gopls

a486dad

silverwind added 3 commits June 4, 2024 20:20

Merge branch 'main' into lint-go-gopls

132cfd7

fix remaining issues and sh

54a2ad6

comment

3c33cd6

github-actions bot added the modifies/go Pull requests that update Go code label Jun 4, 2024

silverwind marked this pull request as ready for review June 4, 2024 19:02

silverwind requested a review from lunny June 4, 2024 19:03

lunny approved these changes Jun 5, 2024

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 5, 2024

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 5, 2024

lunny merged commit 8162222 into go-gitea:main Jun 5, 2024
26 checks passed

GiteaBot added this to the 1.23.0 milestone Jun 5, 2024

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 5, 2024

silverwind deleted the lint-go-gopls branch June 5, 2024 07:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `lint-go-gopls` #30729

Add `lint-go-gopls` #30729

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 29, 2024

silverwind commented Apr 29, 2024

silverwind commented Apr 29, 2024 •

edited

silverwind commented Apr 29, 2024 •

edited

silverwind commented Jun 4, 2024 •

edited

Add lint-go-gopls #30729

Add lint-go-gopls #30729

Conversation

silverwind commented Apr 27, 2024 • edited

silverwind commented Apr 27, 2024 • edited

silverwind commented Apr 27, 2024 • edited

silverwind commented Apr 27, 2024 • edited

silverwind commented Apr 29, 2024

silverwind commented Apr 29, 2024

silverwind commented Apr 29, 2024 • edited

silverwind commented Apr 29, 2024 • edited

silverwind commented Jun 4, 2024 • edited

Add `lint-go-gopls` #30729

Add `lint-go-gopls` #30729

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 27, 2024 •

edited

silverwind commented Apr 29, 2024 •

edited

silverwind commented Apr 29, 2024 •

edited

silverwind commented Jun 4, 2024 •

edited