Skip to content

Commit

Permalink
Add more linters to improve code readability (go-gitea#19989)
Browse files Browse the repository at this point in the history
Add nakedret, unconvert, wastedassign, stylecheck and nolintlint linters to improve code readability

- nakedret - https://github.com/alexkohler/nakedret - nakedret is a Go static analysis tool to find naked returns in functions greater than a specified function length.
- unconvert - https://github.com/mdempsky/unconvert - Remove unnecessary type conversions
- wastedassign - https://github.com/sanposhiho/wastedassign -  wastedassign finds wasted assignment statements.
- notlintlint -  Reports ill-formed or insufficient nolint directives
- stylecheck - https://staticcheck.io/docs/checks/#ST - keep style consistent
  - excluded: [ST1003 - Poorly chosen identifier](https://staticcheck.io/docs/checks/#ST1003) and [ST1005 - Incorrectly formatted error string](https://staticcheck.io/docs/checks/#ST1005)
  • Loading branch information
42wim authored and AbdulrhmnGhanem committed Aug 23, 2022
1 parent d52322a commit c7015c3
Show file tree
Hide file tree
Showing 147 changed files with 402 additions and 397 deletions.
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
- unconvert
- wastedassign
- 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
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

0 comments on commit c7015c3

Please sign in to comment.