Skip to content

Commit 8162222

Browse files
authored
Add lint-go-gopls (#30729)
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 ```
1 parent bd80225 commit 8162222

File tree

7 files changed

+41
-10
lines changed

7 files changed

+41
-10
lines changed

Makefile

+9-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest
3636
GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1
3737
GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1
3838
ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@v1
39+
GOPLS_PACKAGE ?= golang.org/x/tools/gopls@v0.15.3
3940

4041
DOCKER_IMAGE ?= gitea/gitea
4142
DOCKER_TAG ?= latest
@@ -213,6 +214,7 @@ help:
213214
@echo " - lint-go lint go files"
214215
@echo " - lint-go-fix lint go files and fix issues"
215216
@echo " - lint-go-vet lint go files with vet"
217+
@echo " - lint-go-gopls lint go files with gopls"
216218
@echo " - lint-js lint js files"
217219
@echo " - lint-js-fix lint js files and fix issues"
218220
@echo " - lint-css lint css files"
@@ -366,7 +368,7 @@ lint-frontend: lint-js lint-css
366368
lint-frontend-fix: lint-js-fix lint-css-fix
367369

368370
.PHONY: lint-backend
369-
lint-backend: lint-go lint-go-vet lint-editorconfig
371+
lint-backend: lint-go lint-go-vet lint-go-gopls lint-editorconfig
370372

371373
.PHONY: lint-backend-fix
372374
lint-backend-fix: lint-go-fix lint-go-vet lint-editorconfig
@@ -424,6 +426,11 @@ lint-go-vet:
424426
@GOOS= GOARCH= $(GO) build code.gitea.io/gitea-vet
425427
@$(GO) vet -vettool=gitea-vet ./...
426428

429+
.PHONY: lint-go-gopls
430+
lint-go-gopls:
431+
@echo "Running gopls check..."
432+
@GO=$(GO) GOPLS_PACKAGE=$(GOPLS_PACKAGE) tools/lint-go-gopls.sh $(GO_SOURCES_NO_BINDATA)
433+
427434
.PHONY: lint-editorconfig
428435
lint-editorconfig:
429436
@$(GO) run $(EDITORCONFIG_CHECKER_PACKAGE) $(EDITORCONFIG_FILES)
@@ -864,6 +871,7 @@ deps-tools:
864871
$(GO) install $(GO_LICENSES_PACKAGE)
865872
$(GO) install $(GOVULNCHECK_PACKAGE)
866873
$(GO) install $(ACTIONLINT_PACKAGE)
874+
$(GO) install $(GOPLS_PACKAGE)
867875

868876
node_modules: package-lock.json
869877
npm install --no-save

services/migrations/migrate.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func newDownloader(ctx context.Context, ownerName string, opts base.MigrateOptio
176176
// migrateRepository will download information and then upload it to Uploader, this is a simple
177177
// process for small repository. For a big repository, save all the data to disk
178178
// before upload is better
179-
func migrateRepository(ctx context.Context, doer *user_model.User, downloader base.Downloader, uploader base.Uploader, opts base.MigrateOptions, messenger base.Messenger) error {
179+
func migrateRepository(_ context.Context, doer *user_model.User, downloader base.Downloader, uploader base.Uploader, opts base.MigrateOptions, messenger base.Messenger) error {
180180
if messenger == nil {
181181
messenger = base.NilMessenger
182182
}

services/repository/transfer.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName
285285
}
286286

287287
// changeRepositoryName changes all corresponding setting from old repository name to new one.
288-
func changeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, newRepoName string) (err error) {
288+
func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newRepoName string) (err error) {
289289
oldRepoName := repo.Name
290290
newRepoName = strings.ToLower(newRepoName)
291291
if err = repo_model.IsUsableRepoName(newRepoName); err != nil {
@@ -347,7 +347,7 @@ func ChangeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo
347347
// local copy's origin accordingly.
348348

349349
repoWorkingPool.CheckIn(fmt.Sprint(repo.ID))
350-
if err := changeRepositoryName(ctx, doer, repo, newRepoName); err != nil {
350+
if err := changeRepositoryName(ctx, repo, newRepoName); err != nil {
351351
repoWorkingPool.CheckOut(fmt.Sprint(repo.ID))
352352
return err
353353
}

tests/integration/api_repo_tags_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestAPIRepoTags(t *testing.T) {
4242
assert.Equal(t, setting.AppURL+"user2/repo1/archive/v1.1.zip", tags[0].ZipballURL)
4343
assert.Equal(t, setting.AppURL+"user2/repo1/archive/v1.1.tar.gz", tags[0].TarballURL)
4444

45-
newTag := createNewTagUsingAPI(t, session, token, user.Name, repoName, "gitea/22", "", "nice!\nand some text")
45+
newTag := createNewTagUsingAPI(t, token, user.Name, repoName, "gitea/22", "", "nice!\nand some text")
4646
resp = MakeRequest(t, req, http.StatusOK)
4747
DecodeJSON(t, resp, &tags)
4848
assert.Len(t, tags, 2)
@@ -72,7 +72,7 @@ func TestAPIRepoTags(t *testing.T) {
7272
MakeRequest(t, req, http.StatusNotFound)
7373
}
7474

75-
func createNewTagUsingAPI(t *testing.T, session *TestSession, token, ownerName, repoName, name, target, msg string) *api.Tag {
75+
func createNewTagUsingAPI(t *testing.T, token, ownerName, repoName, name, target, msg string) *api.Tag {
7676
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/tags", ownerName, repoName)
7777
req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateTagOption{
7878
TagName: name,

tests/integration/dump_restore_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func (c *compareDump) assertLoadFiles(beforeFilename, afterFilename string, t re
237237
//
238238
// Given []Something{} create afterPtr, beforePtr []*Something{}
239239
//
240-
sliceType := reflect.SliceOf(reflect.PtrTo(t.Elem()))
240+
sliceType := reflect.SliceOf(reflect.PointerTo(t.Elem()))
241241
beforeSlice := reflect.MakeSlice(sliceType, 0, 10)
242242
beforePtr = reflect.New(beforeSlice.Type())
243243
beforePtr.Elem().Set(beforeSlice)

tests/integration/gpg_git_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestGPGGit(t *testing.T) {
3535
defer os.Setenv("GNUPGHOME", oldGNUPGHome)
3636

3737
// Need to create a root key
38-
rootKeyPair, err := importTestingKey(tmpDir, "gitea", "gitea@fake.local")
38+
rootKeyPair, err := importTestingKey()
3939
if !assert.NoError(t, err, "importTestingKey") {
4040
return
4141
}
@@ -262,7 +262,7 @@ func TestGPGGit(t *testing.T) {
262262
})
263263
}
264264

265-
func crudActionCreateFile(t *testing.T, ctx APITestContext, user *user_model.User, from, to, path string, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
265+
func crudActionCreateFile(_ *testing.T, ctx APITestContext, user *user_model.User, from, to, path string, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
266266
return doAPICreateFile(ctx, path, &api.CreateFileOptions{
267267
FileOptions: api.FileOptions{
268268
BranchName: from,
@@ -281,7 +281,7 @@ func crudActionCreateFile(t *testing.T, ctx APITestContext, user *user_model.Use
281281
}, callback...)
282282
}
283283

284-
func importTestingKey(tmpDir, name, email string) (*openpgp.Entity, error) {
284+
func importTestingKey() (*openpgp.Entity, error) {
285285
if _, _, err := process.GetManager().Exec("gpg --import tests/integration/private-testing.key", "gpg", "--import", "tests/integration/private-testing.key"); err != nil {
286286
return nil, err
287287
}

tools/lint-go-gopls.sh

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/bin/bash
2+
set -uo pipefail
3+
4+
cd "$(dirname -- "${BASH_SOURCE[0]}")" && cd ..
5+
6+
IGNORE_PATTERNS=(
7+
"is deprecated" # TODO: fix these
8+
)
9+
10+
# lint all go files with 'gopls check' and look for lines starting with the
11+
# current absolute path, indicating a error was found. This is neccessary
12+
# because the tool does not set non-zero exit code when errors are found.
13+
# ref: https://github.com/golang/go/issues/67078
14+
ERROR_LINES=$("$GO" run "$GOPLS_PACKAGE" check "$@" 2>/dev/null | grep -E "^$PWD" | grep -vFf <(printf '%s\n' "${IGNORE_PATTERNS[@]}"));
15+
NUM_ERRORS=$(echo -n "$ERROR_LINES" | wc -l)
16+
17+
if [ "$NUM_ERRORS" -eq "0" ]; then
18+
exit 0;
19+
else
20+
echo "$ERROR_LINES"
21+
echo "Found $NUM_ERRORS 'gopls check' errors"
22+
exit 1;
23+
fi

0 commit comments

Comments
 (0)