Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more linters to improve code readability #19989

Merged
merged 12 commits into from
Jun 20, 2022
9 changes: 9 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ linters:
- revive
- gofumpt
- depguard
- nakedret
42wim marked this conversation as resolved.
Show resolved Hide resolved
- unconvert
- wastedassign
42wim marked this conversation as resolved.
Show resolved Hide resolved
- nolintlint
- stylecheck
enable-all: false
disable-all: true
fast: false
Expand All @@ -32,6 +37,10 @@ run:
- web_src

linters-settings:
stylecheck:
checks: ["all", "-ST1005", "-ST1003"]
nakedret:
max-func-lines: 0
6543 marked this conversation as resolved.
Show resolved Hide resolved
gocritic:
disabled-checks:
- ifElseChain
Expand Down
2 changes: 1 addition & 1 deletion cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ func writeDataPktLine(out io.Writer, data []byte) error {
if err != nil {
return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
}
if 4 != lr {
if lr != 4 {
return fail("Internal Server Error", "Pkt-Line response failed: %v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion integrations/api_issue_stopwatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestAPIListStopWatches(t *testing.T) {
assert.EqualValues(t, issue.Title, apiWatches[0].IssueTitle)
assert.EqualValues(t, repo.Name, apiWatches[0].RepoName)
assert.EqualValues(t, repo.OwnerName, apiWatches[0].RepoOwnerName)
assert.Greater(t, int64(apiWatches[0].Seconds), int64(0))
assert.Greater(t, apiWatches[0].Seconds, int64(0))
}
}

Expand Down
4 changes: 2 additions & 2 deletions integrations/api_packages_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestPackageContainer(t *testing.T) {

req = NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
addTokenAuthHeader(req, anonymousToken)
resp = MakeRequest(t, req, http.StatusOK)
MakeRequest(t, req, http.StatusOK)
})

t.Run("User", func(t *testing.T) {
Expand All @@ -112,7 +112,7 @@ func TestPackageContainer(t *testing.T) {

req = NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
addTokenAuthHeader(req, userToken)
resp = MakeRequest(t, req, http.StatusOK)
MakeRequest(t, req, http.StatusOK)
})
})

Expand Down
6 changes: 3 additions & 3 deletions integrations/editor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
"_csrf": csrf,
"protected": "off",
})
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)
// Check if master branch has been locked successfully
flashCookie = session.GetCookie("macaron_flash")
assert.NotNil(t, flashCookie)
Expand All @@ -109,7 +109,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
"commit_choice": "direct",
},
)
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)

// Verify the change
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", branch, filePath))
Expand Down Expand Up @@ -139,7 +139,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
"new_branch_name": targetBranch,
},
)
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)

// Verify the change
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", targetBranch, filePath))
Expand Down
6 changes: 3 additions & 3 deletions integrations/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string
defer PrintCurrentTest(t)()
little, big = commitAndPushTest(t, dstPath, "data-file-")
})
return
return little, big
}

func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) {
Expand Down Expand Up @@ -191,7 +191,7 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin
lockTest(t, dstPath)
})
})
return
return littleLFS, bigLFS
}

func commitAndPushTest(t *testing.T, dstPath, prefix string) (little, big string) {
Expand All @@ -210,7 +210,7 @@ func commitAndPushTest(t *testing.T, dstPath, prefix string) (little, big string
big = doCommitAndPush(t, bigSize, dstPath, prefix)
})
})
return
return little, big
}

func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS string) {
Expand Down
2 changes: 1 addition & 1 deletion integrations/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ func getTokenForLoggedInUser(t testing.TB, session *TestSession) string {
"_csrf": doc.GetCSRF(),
"name": fmt.Sprintf("api-testing-token-%d", tokenCounter),
})
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)
req = NewRequest(t, "GET", "/user/settings/applications")
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
Expand Down
2 changes: 1 addition & 1 deletion integrations/nonascii_branches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func testSrcRouteRedirect(t *testing.T, session *TestSession, user, repo, route,

// Perform redirect
req = NewRequest(t, "GET", location)
resp = session.MakeRequest(t, req, expectedStatus)
session.MakeRequest(t, req, expectedStatus)
}

func setDefaultBranch(t *testing.T, session *TestSession, user, repo, branch string) {
Expand Down
4 changes: 2 additions & 2 deletions integrations/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
})
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OmJsYWJsYQ==")
resp = MakeRequest(t, req, http.StatusBadRequest)
MakeRequest(t, req, http.StatusBadRequest)

// missing header
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
Expand All @@ -206,7 +206,7 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
"code": "authcode",
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
})
resp = MakeRequest(t, req, http.StatusBadRequest)
MakeRequest(t, req, http.StatusBadRequest)
}

func TestRefreshTokenInvalidation(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion integrations/repo_fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO
"uid": fmt.Sprintf("%d", forkOwner.ID),
"repo_name": forkRepoName,
})
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)

// Step4: check the existence of the forked repo
req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
Expand Down
2 changes: 1 addition & 1 deletion integrations/repo_generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func testRepoGenerate(t *testing.T, session *TestSession, templateOwnerName, tem
"repo_name": generateRepoName,
"git_content": "true",
})
resp = session.MakeRequest(t, req, http.StatusSeeOther)
session.MakeRequest(t, req, http.StatusSeeOther)

// Step4: check the existence of the generated repo
req = NewRequestf(t, "GET", "/%s/%s", generateOwnerName, generateRepoName)
Expand Down
2 changes: 1 addition & 1 deletion integrations/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,6 @@ func TestListStopWatches(t *testing.T) {
assert.EqualValues(t, issue.Title, apiWatches[0].IssueTitle)
assert.EqualValues(t, repo.Name, apiWatches[0].RepoName)
assert.EqualValues(t, repo.OwnerName, apiWatches[0].RepoOwnerName)
assert.Greater(t, int64(apiWatches[0].Seconds), int64(0))
assert.Greater(t, apiWatches[0].Seconds, int64(0))
}
}
2 changes: 1 addition & 1 deletion models/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ func DeleteOldActions(olderThan time.Duration) (err error) {
}

_, err = db.GetEngine(db.DefaultContext).Where("created_unix < ?", time.Now().Add(-olderThan).Unix()).Delete(&Action{})
return
return err
}

func notifyWatchers(ctx context.Context, actions ...*Action) error {
Expand Down
2 changes: 1 addition & 1 deletion models/admin/notice.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,5 @@ func DeleteOldSystemNotices(olderThan time.Duration) (err error) {
}

_, err = db.GetEngine(db.DefaultContext).Where("created_unix < ?", time.Now().Add(-olderThan).Unix()).Delete(&Notice{})
return
return err
}
2 changes: 1 addition & 1 deletion models/asymkey/gpg_key_commit_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,5 +520,5 @@ func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_
}
}

return
return err
}
6 changes: 3 additions & 3 deletions models/asymkey/ssh_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func TestFromOpenSSH(t *testing.T) {
td := t.TempDir()

data := []byte("hello, ssh world")
dataPath := write(t, []byte(data), td, "data")
dataPath := write(t, data, td, "data")

privPath := write(t, []byte(tt.priv), td, "id")
write(t, []byte(tt.pub), td, "id.pub")
Expand Down Expand Up @@ -372,14 +372,14 @@ func TestToOpenSSH(t *testing.T) {
td := t.TempDir()

data := []byte("hello, ssh world")
write(t, []byte(data), td, "data")
write(t, data, td, "data")

armored, err := sshsig.Sign([]byte(tt.priv), bytes.NewReader(data), "file")
if err != nil {
t.Fatal(err)
}

sigPath := write(t, []byte(armored), td, "oursig")
sigPath := write(t, armored, td, "oursig")

// Create an allowed_signers file with two keys to check against.
allowedSigner := "test@rekor.dev " + tt.pub + "\n"
Expand Down
8 changes: 4 additions & 4 deletions models/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func GetOAuth2ApplicationByClientID(ctx context.Context, clientID string) (app *
if !has {
return nil, ErrOAuthClientIDInvalid{ClientID: clientID}
}
return
return app, err
}

// GetOAuth2ApplicationByID returns the oauth2 application with the given id. Returns an error if not found.
Expand All @@ -143,7 +143,7 @@ func GetOAuth2ApplicationByID(ctx context.Context, id int64) (app *OAuth2Applica
func GetOAuth2ApplicationsByUserID(ctx context.Context, userID int64) (apps []*OAuth2Application, err error) {
apps = make([]*OAuth2Application, 0)
err = db.GetEngine(ctx).Where("uid = ?", userID).Find(&apps)
return
return apps, err
}

// CreateOAuth2ApplicationOptions holds options to create an oauth2 application
Expand Down Expand Up @@ -300,7 +300,7 @@ func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (redirect
}
q.Set("code", code.Code)
redirect.RawQuery = q.Encode()
return
return redirect, err
}

// Invalidate deletes the auth code from the database to invalidate this code
Expand Down Expand Up @@ -430,7 +430,7 @@ func GetOAuth2GrantByID(ctx context.Context, id int64) (grant *OAuth2Grant, err
} else if !has {
return nil, nil
}
return
return grant, err
}

// GetOAuth2GrantsByUserID lists all grants of a certain user
Expand Down
2 changes: 1 addition & 1 deletion models/db/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,5 +285,5 @@ func DeleteAllRecords(tableName string) error {
// GetMaxID will return max id of the table
func GetMaxID(beanOrTableName interface{}) (maxID int64, err error) {
_, err = x.Select("MAX(id)").Table(beanOrTableName).Get(&maxID)
return
return maxID, err
}
2 changes: 1 addition & 1 deletion models/db/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func UpsertResourceIndex(ctx context.Context, tableName string, groupID int64) (
default:
return fmt.Errorf("database type not supported")
}
return
return err
}

var (
Expand Down
2 changes: 1 addition & 1 deletion models/db/list_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (opts *ListOptions) GetSkipTake() (skip, take int) {
func (opts *ListOptions) GetStartEnd() (start, end int) {
start, take := opts.GetSkipTake()
end = start + take
return
return start, end
}

// SetDefaultValues sets default values
Expand Down
2 changes: 1 addition & 1 deletion models/db/sql_postgres_with_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) {
_, err := execer.Exec(`SELECT set_config(
'search_path',
$1 || ',' || current_setting('search_path'),
false)`, []driver.Value{schemaValue}) //nolint
false)`, []driver.Value{schemaValue})
if err != nil {
_ = conn.Close()
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions models/git/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, c
whitelist = append(whitelist, userID)
}

return
return whitelist, err
}

// updateUserWhitelist checks whether the user whitelist changed and returns a whitelist with
Expand Down Expand Up @@ -392,7 +392,7 @@ func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, curre
whitelist = append(whitelist, userID)
}

return
return whitelist, err
}

// updateTeamWhitelist checks whether the team whitelist changed and returns a whitelist with
Expand All @@ -415,7 +415,7 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre
}
}

return
return whitelist, err
}

// DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
Expand Down Expand Up @@ -539,7 +539,7 @@ func FindRenamedBranch(repoID int64, from string) (branch *RenamedBranch, exist
}
exist, err = db.GetEngine(db.DefaultContext).Get(branch)

return
return branch, exist, err
}

// RenameBranch rename a branch
Expand Down
2 changes: 1 addition & 1 deletion models/git/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func upsertCommitStatusIndex(ctx context.Context, repoID int64, sha string) (err
default:
return fmt.Errorf("database type not supported")
}
return
return err
}

// GetNextCommitStatusIndex retried 3 times to generate a resource index
Expand Down
4 changes: 2 additions & 2 deletions models/issues/assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (issue *Issue) LoadAssignees(ctx context.Context) (err error) {
if len(issue.Assignees) > 0 {
issue.Assignee = issue.Assignees[0]
}
return
return err
}

// GetAssigneeIDsByIssue returns the IDs of users assigned to an issue
Expand Down Expand Up @@ -167,5 +167,5 @@ func MakeIDsFromAPIAssigneesToAdd(oneAssignee string, multipleAssignees []string
// Get the IDs of all assignees
assigneeIDs, err = user_model.GetUserIDsByNames(requestAssignees, false)

return
return assigneeIDs, err
}
Loading