Add more linters to improve code readability

.golangci.yml

@@ -19,6 +19,10 @@ linters:
     - revive
     - gofumpt
     - depguard
+    - nakedret
+    - unconvert
+    - wastedassign
+    - nolintlint
   enable-all: false
   disable-all: true
   fast: false
@@ -32,6 +36,8 @@ run:
     - web_src
 
 linters-settings:
+  nakedret:
+    max-func-lines: 5 
   gocritic:
     disabled-checks:
       - ifElseChain

integrations/api_issue_stopwatch_test.go

@@ -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, 0)
 	}
 }
 

integrations/api_packages_container_test.go

@@ -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) {
@@ -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)
 		})
 	})
 

integrations/editor_test.go

@@ -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)
@@ -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))
@@ -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))

integrations/integration_test.go

@@ -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)

integrations/nonascii_branches_test.go

@@ -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) {

integrations/oauth_test.go

@@ -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{
@@ -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) {

integrations/repo_fork_test.go

@@ -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)

integrations/repo_generate_test.go

@@ -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)

integrations/user_test.go

@@ -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))
 	}
 }

models/asymkey/ssh_key_test.go

@@ -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")
@@ -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 := "[email protected] " + tt.pub + "\n"

models/db/sql_postgres_with_schema.go

@@ -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

models/issues/issue.go

@@ -311,7 +311,7 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {
 		return err
 	}
 	// Load reaction user data
-	if _, err := ReactionList(reactions).LoadUsers(ctx, issue.Repo); err != nil {
+	if _, err := reactions.LoadUsers(ctx, issue.Repo); err != nil {
 		return err
 	}
 

models/issues/review.go

@@ -537,7 +537,7 @@ func GetReviewByIssueIDAndUserID(ctx context.Context, issueID, userID int64) (*R
 func GetTeamReviewerByIssueIDAndTeamID(ctx context.Context, issueID, teamID int64) (review *Review, err error) {
 	review = new(Review)
 
-	has := false
+	var has bool
 	if has, err = db.GetEngine(ctx).SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id = ?)",
 		issueID, teamID).
 		Get(review); err != nil {

models/migrations/v189.go

@@ -81,7 +81,7 @@ func unwrapLDAPSourceCfg(x *xorm.Engine) error {
 			}
 			err := jsonUnmarshalHandleDoubleEncode([]byte(source.Cfg), &wrapped)
 			if err != nil {
-				return fmt.Errorf("failed to unmarshal %s: %w", string(source.Cfg), err)
+				return fmt.Errorf("failed to unmarshal %s: %w", source.Cfg, err)
 			}
 			if wrapped.Source != nil && len(wrapped.Source) > 0 {
 				bs, err := json.Marshal(wrapped.Source)

models/repo/repo.go

@@ -546,7 +546,7 @@ func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
 		log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err)
 		return template.HTML(markup.Sanitize(repo.Description))
 	}
-	return template.HTML(markup.Sanitize(string(desc)))
+	return template.HTML(markup.Sanitize(desc))
 }
 
 // CloneLink represents different types of clone URLs of repository.

models/webhook/hooktask.go

@@ -286,7 +286,7 @@ func deleteDeliveredHookTasksByWebhook(hookID int64, numberDeliveriesToKeep int)
 		Cols("hook_task.delivered").
 		Join("INNER", "webhook", "hook_task.hook_id = webhook.id").
 		OrderBy("hook_task.delivered desc").
-		Limit(1, int(numberDeliveriesToKeep)).
+		Limit(1, numberDeliveriesToKeep).
 		Find(&deliveryDates)
 	if err != nil {
 		return err

modules/charset/charset_test.go

@@ -296,11 +296,11 @@ func TestDetectEncoding(t *testing.T) {
 }
 
 func stringMustStartWith(t *testing.T, expected, value string) {
-	assert.Equal(t, expected, string(value[:len(expected)]))
+	assert.Equal(t, expected, value[:len(expected)])
 }
 
 func stringMustEndWith(t *testing.T, expected, value string) {
-	assert.Equal(t, expected, string(value[len(value)-len(expected):]))
+	assert.Equal(t, expected, value[len(value)-len(expected):])
 }
 
 func bytesMustStartWith(t *testing.T, expected, value []byte) {

modules/convert/convert.go

@@ -257,7 +257,7 @@ func ToHook(repoLink string, w *webhook.Webhook) *api.Hook {
 
 	return &api.Hook{
 		ID:      w.ID,
-		Type:    string(w.Type),
+		Type:    w.Type,
 		URL:     fmt.Sprintf("%s/settings/hooks/%d", repoLink, w.ID),
 		Active:  w.IsActive,
 		Config:  config,

modules/doctor/fix16961.go

@@ -216,7 +216,7 @@ func fixBrokenRepoUnit16961(repoUnit *repo_model.RepoUnit, bs []byte) (fixed boo
 		return false, nil
 	}
 
-	switch unit.Type(repoUnit.Type) {
+	switch repoUnit.Type {
 	case unit.TypeCode, unit.TypeReleases, unit.TypeWiki, unit.TypeProjects:
 		cfg := &repo_model.UnitConfig{}
 		repoUnit.Config = cfg

modules/eventsource/event.go

@@ -18,7 +18,7 @@ func wrapNewlines(w io.Writer, prefix, value []byte) (sum int64, err error) {
 	if len(value) == 0 {
 		return
 	}
-	n := 0
+	var n int
 	last := 0
 	for j := bytes.IndexByte(value, '\n'); j > -1; j = bytes.IndexByte(value[last:], '\n') {
 		n, err = w.Write(prefix)
@@ -64,7 +64,7 @@ type Event struct {
 // The return value n is the number of bytes written. Any error encountered during the write is also returned.
 func (e *Event) WriteTo(w io.Writer) (int64, error) {
 	sum := int64(0)
-	nint := 0
+	var nint int
 	n, err := wrapNewlines(w, []byte("event: "), []byte(e.Name))
 	sum += n
 	if err != nil {

modules/git/batch_reader.go

@@ -181,7 +181,7 @@ func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err er
 
 // ReadTagObjectID reads a tag object ID hash from a cat-file --batch stream, throwing away the rest of the stream.
 func ReadTagObjectID(rd *bufio.Reader, size int64) (string, error) {
-	id := ""
+	var id string
 	var n int64
 headerLoop:
 	for {
@@ -216,7 +216,7 @@ headerLoop:
 
 // ReadTreeID reads a tree ID from a cat-file --batch stream, throwing away the rest of the stream.
 func ReadTreeID(rd *bufio.Reader, size int64) (string, error) {
-	id := ""
+	var id string
 	var n int64
 headerLoop:
 	for {
@@ -328,7 +328,7 @@ func ParseTreeLine(rd *bufio.Reader, modeBuf, fnameBuf, shaBuf []byte) (mode, fn
 	// Deal with the 20-byte SHA
 	idx = 0
 	for idx < 20 {
-		read := 0
+		var read int
 		read, err = rd.Read(shaBuf[idx:20])
 		n += read
 		if err != nil {

modules/git/commit_info_nogogit.go

@@ -157,7 +157,7 @@ func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, commit *
 		if typ != "commit" {
 			return nil, fmt.Errorf("unexpected type: %s for commit id: %s", typ, commitID)
 		}
-		c, err = CommitFromReader(commit.repo, MustIDFromString(string(commitID)), io.LimitReader(batchReader, int64(size)))
+		c, err = CommitFromReader(commit.repo, MustIDFromString(commitID), io.LimitReader(batchReader, size))
 		if err != nil {
 			return nil, err
 		}

modules/git/pipeline/lfs_nogogit.go

@@ -116,7 +116,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 				continue
 			case "commit":
 				// Read in the commit to get its tree and in case this is one of the last used commits
-				curCommit, err = git.CommitFromReader(repo, git.MustIDFromString(string(commitID)), io.LimitReader(batchReader, int64(size)))
+				curCommit, err = git.CommitFromReader(repo, git.MustIDFromString(string(commitID)), io.LimitReader(batchReader, size))
 				if err != nil {
 					return nil, err
 				}

modules/git/repo_attribute.go

@@ -334,7 +334,7 @@ func (wr *lineSeparatedAttributeWriter) Write(p []byte) (n int, err error) {
 						wr.tmp = []byte(remaining[3:])
 						break
 					}
-					return l, fmt.Errorf("unexpected tail %s", string(remaining))
+					return l, fmt.Errorf("unexpected tail %s", remaining)
 				}
 				_, _ = sb.WriteRune(rn)
 				remaining = tail

modules/git/repo_compare_test.go

@@ -117,8 +117,8 @@ func TestReadWritePullHead(t *testing.T) {
 		return
 	}
 
-	assert.Len(t, string(headContents), 40)
-	assert.True(t, string(headContents) == newCommit)
+	assert.Len(t, headContents, 40)
+	assert.True(t, headContents == newCommit)
 
 	// Remove file after the test
 	err = repo.RemoveReference(PullPrefix + "1/head")

modules/gitgraph/graph_test.go

@@ -53,7 +53,7 @@ func BenchmarkParseGlyphs(b *testing.B) {
 	parser := &Parser{}
 	parser.Reset()
 	tgBytes := []byte(testglyphs)
-	tg := tgBytes
+	var tg []byte
 	for i := 0; i < b.N; i++ {
 		parser.Reset()
 		tg = tgBytes

modules/highlight/highlight.go

@@ -114,7 +114,7 @@ func CodeFromLexer(lexer chroma.Lexer, code string) string {
 	htmlbuf := bytes.Buffer{}
 	htmlw := bufio.NewWriter(&htmlbuf)
 
-	iterator, err := lexer.Tokenise(nil, string(code))
+	iterator, err := lexer.Tokenise(nil, code)
 	if err != nil {
 		log.Error("Can't tokenize code: %v", err)
 		return code
@@ -197,7 +197,7 @@ func File(numLines int, fileName, language string, code []byte) []string {
 
 	m := make([]string, 0, numLines)
 	for _, v := range strings.SplitN(htmlbuf.String(), "\n", numLines) {
-		content := string(v)
+		content := v
 		// need to keep lines that are only \n so copy/paste works properly in browser
 		if content == "" {
 			content = "\n"
@@ -220,8 +220,8 @@ func File(numLines int, fileName, language string, code []byte) []string {
 // return unhiglighted map
 func plainText(code string, numLines int) []string {
 	m := make([]string, 0, numLines)
-	for _, v := range strings.SplitN(string(code), "\n", numLines) {
-		content := string(v)
+	for _, v := range strings.SplitN(code, "\n", numLines) {
+		content := v
 		// need to keep lines that are only \n so copy/paste works properly in browser
 		if content == "" {
 			content = "\n"

modules/indexer/code/elastic_search.go

@@ -348,7 +348,7 @@ func convertResult(searchResult *elastic.SearchResult, kw string, pageSize int)
 		// FIXME: There is no way to get the position the keyword on the content currently on the same request.
 		// So we get it from content, this may made the query slower. See
 		// https://discuss.elastic.co/t/fetching-position-of-keyword-in-matched-document/94291
-		var startIndex, endIndex int = -1, -1
+		var startIndex, endIndex int
 		c, ok := hit.Highlight["content"]
 		if ok && len(c) > 0 {
 			// FIXME: Since the highlighting content will include <em> and </em> for the keywords,

modules/markup/common/footnote.go

@@ -203,9 +203,8 @@ func (b *footnoteBlockParser) Open(parent ast.Node, reader text.Reader, pc parse
 		return nil, parser.NoChildren
 	}
 	open := pos + 1
-	closes := 0
 	closure := util.FindClosure(line[pos+1:], '[', ']', false, false) //nolint
-	closes = pos + 1 + closure
+	closes := pos + 1 + closure
 	next := closes + 1
 	if closure > -1 {
 		if next >= len(line) || line[next] != ':' {