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 ``` (cherry picked from commit 816222243af523316041692622be6f48ef068693) Conflicts: Makefile trivial context conflict and also ask renovate to watch over it do not include it in lint-backend because the errors are not fixed
This commit is contained in:
		
							parent
							
								
									23a82bcd7a
								
							
						
					
					
						commit
						6582f0029b
					
				
					 7 changed files with 41 additions and 10 deletions
				
			
		
							
								
								
									
										8
									
								
								Makefile
									
										
									
									
									
								
							
							
						
						
									
										8
									
								
								Makefile
									
										
									
									
									
								
							| 
						 | 
				
			
			@ -38,6 +38,7 @@ GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasour
 | 
			
		|||
GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go
 | 
			
		||||
DEADCODE_PACKAGE ?= golang.org/x/tools/cmd/deadcode@v0.22.0 # renovate: datasource=go
 | 
			
		||||
GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.4.0 # renovate: datasource=go
 | 
			
		||||
GOPLS_PACKAGE ?= golang.org/x/tools/gopls@v0.15.3 # renovate: datasource=go
 | 
			
		||||
 | 
			
		||||
DOCKER_IMAGE ?= gitea/gitea
 | 
			
		||||
DOCKER_TAG ?= latest
 | 
			
		||||
| 
						 | 
				
			
			@ -228,6 +229,7 @@ help:
 | 
			
		|||
	@echo " - lint-go                          lint go files"
 | 
			
		||||
	@echo " - lint-go-fix                      lint go files and fix issues"
 | 
			
		||||
	@echo " - lint-go-vet                      lint go files with vet"
 | 
			
		||||
	@echo " - lint-go-gopls                    lint go files with gopls"
 | 
			
		||||
	@echo " - lint-js                          lint js files"
 | 
			
		||||
	@echo " - lint-js-fix                      lint js files and fix issues"
 | 
			
		||||
	@echo " - lint-css                         lint css files"
 | 
			
		||||
| 
						 | 
				
			
			@ -468,6 +470,11 @@ lint-go-vet:
 | 
			
		|||
	@echo "Running go vet..."
 | 
			
		||||
	@$(GO) vet ./...
 | 
			
		||||
 | 
			
		||||
.PHONY: lint-go-gopls
 | 
			
		||||
lint-go-gopls:
 | 
			
		||||
	@echo "Running gopls check..."
 | 
			
		||||
	@GO=$(GO) GOPLS_PACKAGE=$(GOPLS_PACKAGE) tools/lint-go-gopls.sh $(GO_SOURCES_NO_BINDATA)
 | 
			
		||||
 | 
			
		||||
.PHONY: lint-editorconfig
 | 
			
		||||
lint-editorconfig:
 | 
			
		||||
	$(GO) run $(EDITORCONFIG_CHECKER_PACKAGE) templates .forgejo/workflows
 | 
			
		||||
| 
						 | 
				
			
			@ -879,6 +886,7 @@ deps-tools:
 | 
			
		|||
	$(GO) install $(GO_LICENSES_PACKAGE)
 | 
			
		||||
	$(GO) install $(GOVULNCHECK_PACKAGE)
 | 
			
		||||
	$(GO) install $(GOMOCK_PACKAGE)
 | 
			
		||||
	$(GO) install $(GOPLS_PACKAGE)
 | 
			
		||||
 | 
			
		||||
node_modules: package-lock.json
 | 
			
		||||
	npm install --no-save
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -183,7 +183,7 @@ func newDownloader(ctx context.Context, ownerName string, opts base.MigrateOptio
 | 
			
		|||
// migrateRepository will download information and then upload it to Uploader, this is a simple
 | 
			
		||||
// process for small repository. For a big repository, save all the data to disk
 | 
			
		||||
// before upload is better
 | 
			
		||||
func migrateRepository(ctx context.Context, doer *user_model.User, downloader base.Downloader, uploader base.Uploader, opts base.MigrateOptions, messenger base.Messenger) error {
 | 
			
		||||
func migrateRepository(_ context.Context, doer *user_model.User, downloader base.Downloader, uploader base.Uploader, opts base.MigrateOptions, messenger base.Messenger) error {
 | 
			
		||||
	if messenger == nil {
 | 
			
		||||
		messenger = base.NilMessenger
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -285,7 +285,7 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName
 | 
			
		|||
}
 | 
			
		||||
 | 
			
		||||
// changeRepositoryName changes all corresponding setting from old repository name to new one.
 | 
			
		||||
func changeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, newRepoName string) (err error) {
 | 
			
		||||
func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newRepoName string) (err error) {
 | 
			
		||||
	oldRepoName := repo.Name
 | 
			
		||||
	newRepoName = strings.ToLower(newRepoName)
 | 
			
		||||
	if err = repo_model.IsUsableRepoName(newRepoName); err != nil {
 | 
			
		||||
| 
						 | 
				
			
			@ -347,7 +347,7 @@ func ChangeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo
 | 
			
		|||
	// local copy's origin accordingly.
 | 
			
		||||
 | 
			
		||||
	repoWorkingPool.CheckIn(fmt.Sprint(repo.ID))
 | 
			
		||||
	if err := changeRepositoryName(ctx, doer, repo, newRepoName); err != nil {
 | 
			
		||||
	if err := changeRepositoryName(ctx, repo, newRepoName); err != nil {
 | 
			
		||||
		repoWorkingPool.CheckOut(fmt.Sprint(repo.ID))
 | 
			
		||||
		return err
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -42,7 +42,7 @@ func TestAPIRepoTags(t *testing.T) {
 | 
			
		|||
	assert.Equal(t, setting.AppURL+"user2/repo1/archive/v1.1.zip", tags[0].ZipballURL)
 | 
			
		||||
	assert.Equal(t, setting.AppURL+"user2/repo1/archive/v1.1.tar.gz", tags[0].TarballURL)
 | 
			
		||||
 | 
			
		||||
	newTag := createNewTagUsingAPI(t, session, token, user.Name, repoName, "gitea/22", "", "nice!\nand some text")
 | 
			
		||||
	newTag := createNewTagUsingAPI(t, token, user.Name, repoName, "gitea/22", "", "nice!\nand some text")
 | 
			
		||||
	resp = MakeRequest(t, req, http.StatusOK)
 | 
			
		||||
	DecodeJSON(t, resp, &tags)
 | 
			
		||||
	assert.Len(t, tags, 2)
 | 
			
		||||
| 
						 | 
				
			
			@ -72,7 +72,7 @@ func TestAPIRepoTags(t *testing.T) {
 | 
			
		|||
	MakeRequest(t, req, http.StatusNotFound)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func createNewTagUsingAPI(t *testing.T, session *TestSession, token, ownerName, repoName, name, target, msg string) *api.Tag {
 | 
			
		||||
func createNewTagUsingAPI(t *testing.T, token, ownerName, repoName, name, target, msg string) *api.Tag {
 | 
			
		||||
	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/tags", ownerName, repoName)
 | 
			
		||||
	req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateTagOption{
 | 
			
		||||
		TagName: name,
 | 
			
		||||
| 
						 | 
				
			
			@ -96,7 +96,7 @@ func TestAPIGetTagArchiveDownloadCount(t *testing.T) {
 | 
			
		|||
	repoName := "repo1"
 | 
			
		||||
	tagName := "TagDownloadCount"
 | 
			
		||||
 | 
			
		||||
	createNewTagUsingAPI(t, session, token, user.Name, repoName, tagName, "", "")
 | 
			
		||||
	createNewTagUsingAPI(t, token, user.Name, repoName, tagName, "", "")
 | 
			
		||||
 | 
			
		||||
	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/tags/%s?token=%s", user.Name, repoName, tagName, token)
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -237,7 +237,7 @@ func (c *compareDump) assertLoadFiles(beforeFilename, afterFilename string, t re
 | 
			
		|||
		//
 | 
			
		||||
		// Given []Something{} create afterPtr, beforePtr []*Something{}
 | 
			
		||||
		//
 | 
			
		||||
		sliceType := reflect.SliceOf(reflect.PtrTo(t.Elem()))
 | 
			
		||||
		sliceType := reflect.SliceOf(reflect.PointerTo(t.Elem()))
 | 
			
		||||
		beforeSlice := reflect.MakeSlice(sliceType, 0, 10)
 | 
			
		||||
		beforePtr = reflect.New(beforeSlice.Type())
 | 
			
		||||
		beforePtr.Elem().Set(beforeSlice)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -36,7 +36,7 @@ func TestGPGGit(t *testing.T) {
 | 
			
		|||
	defer os.Setenv("GNUPGHOME", oldGNUPGHome)
 | 
			
		||||
 | 
			
		||||
	// Need to create a root key
 | 
			
		||||
	rootKeyPair, err := importTestingKey(tmpDir, "gitea", "gitea@fake.local")
 | 
			
		||||
	rootKeyPair, err := importTestingKey()
 | 
			
		||||
	if !assert.NoError(t, err, "importTestingKey") {
 | 
			
		||||
		return
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			@ -263,7 +263,7 @@ func TestGPGGit(t *testing.T) {
 | 
			
		|||
	})
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func crudActionCreateFile(t *testing.T, ctx APITestContext, user *user_model.User, from, to, path string, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
 | 
			
		||||
func crudActionCreateFile(_ *testing.T, ctx APITestContext, user *user_model.User, from, to, path string, callback ...func(*testing.T, api.FileResponse)) func(*testing.T) {
 | 
			
		||||
	return doAPICreateFile(ctx, path, &api.CreateFileOptions{
 | 
			
		||||
		FileOptions: api.FileOptions{
 | 
			
		||||
			BranchName:    from,
 | 
			
		||||
| 
						 | 
				
			
			@ -282,7 +282,7 @@ func crudActionCreateFile(t *testing.T, ctx APITestContext, user *user_model.Use
 | 
			
		|||
	}, callback...)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func importTestingKey(tmpDir, name, email string) (*openpgp.Entity, error) {
 | 
			
		||||
func importTestingKey() (*openpgp.Entity, error) {
 | 
			
		||||
	if _, _, err := process.GetManager().Exec("gpg --import tests/integration/private-testing.key", "gpg", "--import", "tests/integration/private-testing.key"); err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										23
									
								
								tools/lint-go-gopls.sh
									
										
									
									
									
										Executable file
									
								
							
							
						
						
									
										23
									
								
								tools/lint-go-gopls.sh
									
										
									
									
									
										Executable file
									
								
							| 
						 | 
				
			
			@ -0,0 +1,23 @@
 | 
			
		|||
#!/bin/bash
 | 
			
		||||
set -uo pipefail
 | 
			
		||||
 | 
			
		||||
cd "$(dirname -- "${BASH_SOURCE[0]}")" && cd ..
 | 
			
		||||
 | 
			
		||||
IGNORE_PATTERNS=(
 | 
			
		||||
  "is deprecated" # TODO: fix these
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
# lint all go files with 'gopls check' and look for lines starting with the
 | 
			
		||||
# current absolute path, indicating a error was found. This is neccessary
 | 
			
		||||
# because the tool does not set non-zero exit code when errors are found.
 | 
			
		||||
# ref: https://github.com/golang/go/issues/67078
 | 
			
		||||
ERROR_LINES=$("$GO" run "$GOPLS_PACKAGE" check "$@" 2>/dev/null | grep -E "^$PWD" | grep -vFf <(printf '%s\n' "${IGNORE_PATTERNS[@]}"));
 | 
			
		||||
NUM_ERRORS=$(echo -n "$ERROR_LINES" | wc -l)
 | 
			
		||||
 | 
			
		||||
if [ "$NUM_ERRORS" -eq "0" ]; then
 | 
			
		||||
  exit 0;
 | 
			
		||||
else
 | 
			
		||||
  echo "$ERROR_LINES"
 | 
			
		||||
  echo "Found $NUM_ERRORS 'gopls check' errors"
 | 
			
		||||
  exit 1;
 | 
			
		||||
fi
 | 
			
		||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue