Add more linters to improve code readability (#19989)

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)

models/issues/assignees.go

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

models/issues/comment.go

@@ -315,7 +315,7 @@ func (c *Comment) LoadIssueCtx(ctx context.Context) (err error) {
 		return nil
 	}
 	c.Issue, err = GetIssueByID(ctx, c.IssueID)
-	return
+	return err
 }
 
 // BeforeInsert will be invoked by XORM before inserting a record
@@ -627,7 +627,7 @@ func (c *Comment) LoadResolveDoer() (err error) {
 			err = nil
 		}
 	}
-	return
+	return err
 }
 
 // IsResolved check if an code comment is resolved
@@ -955,7 +955,7 @@ func createIssueDependencyComment(ctx context.Context, doer *user_model.User, is
 		DependentIssueID: issue.ID,
 	}
 	_, err = CreateCommentCtx(ctx, opts)
-	return
+	return err
 }
 
 // CreateCommentOptions defines options for creating comment
@@ -1350,7 +1350,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *Pul
 
 	comment, err = CreateComment(ops)
 
-	return
+	return comment, err
 }
 
 // CreateAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes
@@ -1372,7 +1372,7 @@ func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullReques
 		Repo:  pr.BaseRepo,
 		Issue: pr.Issue,
 	})
-	return
+	return comment, err
 }
 
 // getCommitsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
@@ -1434,7 +1434,7 @@ func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldC
 		}
 	}
 
-	return
+	return commitIDs, isForcePush, err
 }
 
 type commitBranchCheckItem struct {

models/issues/issue.go

@@ -223,7 +223,7 @@ func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
 		return nil, err
 	}
 	pr.Issue = issue
-	return
+	return pr, err
 }
 
 // LoadLabels loads labels
@@ -255,7 +255,7 @@ func (issue *Issue) loadPoster(ctx context.Context) (err error) {
 			return
 		}
 	}
-	return
+	return err
 }
 
 func (issue *Issue) loadPullRequest(ctx context.Context) (err error) {
@@ -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
 	}
 
@@ -2110,7 +2110,7 @@ func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) {
 	} else {
 		err = repo_model.StatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues")
 	}
-	return
+	return err
 }
 
 // FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
@@ -2123,7 +2123,7 @@ func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_mo
 	if err = UpdateIssueMentions(ctx, issue.ID, mentions); err != nil {
 		return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
 	}
-	return
+	return mentions, err
 }
 
 // ResolveIssueMentionsByVisibility returns the users mentioned in an issue, removing those that
@@ -2257,7 +2257,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
 		users = append(users, user)
 	}
 
-	return
+	return users, err
 }
 
 // UpdateIssuesMigrationsByType updates all migrated repositories' issues from gitServiceType to replace originalAuthorID to posterID
@@ -2380,7 +2380,7 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
 		return
 	}
 
-	return
+	return attachmentPaths, err
 }
 
 // RemapExternalUser ExternalUserRemappable interface

models/issues/issue_project.go

@@ -14,32 +14,32 @@ import (
 )
 
 // LoadProject load the project the issue was assigned to
-func (i *Issue) LoadProject() (err error) {
-	return i.loadProject(db.DefaultContext)
+func (issue *Issue) LoadProject() (err error) {
+	return issue.loadProject(db.DefaultContext)
 }
 
-func (i *Issue) loadProject(ctx context.Context) (err error) {
-	if i.Project == nil {
+func (issue *Issue) loadProject(ctx context.Context) (err error) {
+	if issue.Project == nil {
 		var p project_model.Project
 		if _, err = db.GetEngine(ctx).Table("project").
 			Join("INNER", "project_issue", "project.id=project_issue.project_id").
-			Where("project_issue.issue_id = ?", i.ID).
+			Where("project_issue.issue_id = ?", issue.ID).
 			Get(&p); err != nil {
 			return err
 		}
-		i.Project = &p
+		issue.Project = &p
 	}
-	return
+	return err
 }
 
 // ProjectID return project id if issue was assigned to one
-func (i *Issue) ProjectID() int64 {
-	return i.projectID(db.DefaultContext)
+func (issue *Issue) ProjectID() int64 {
+	return issue.projectID(db.DefaultContext)
 }
 
-func (i *Issue) projectID(ctx context.Context) int64 {
+func (issue *Issue) projectID(ctx context.Context) int64 {
 	var ip project_model.ProjectIssue
-	has, err := db.GetEngine(ctx).Where("issue_id=?", i.ID).Get(&ip)
+	has, err := db.GetEngine(ctx).Where("issue_id=?", issue.ID).Get(&ip)
 	if err != nil || !has {
 		return 0
 	}
@@ -47,13 +47,13 @@ func (i *Issue) projectID(ctx context.Context) int64 {
 }
 
 // ProjectBoardID return project board id if issue was assigned to one
-func (i *Issue) ProjectBoardID() int64 {
-	return i.projectBoardID(db.DefaultContext)
+func (issue *Issue) ProjectBoardID() int64 {
+	return issue.projectBoardID(db.DefaultContext)
 }
 
-func (i *Issue) projectBoardID(ctx context.Context) int64 {
+func (issue *Issue) projectBoardID(ctx context.Context) int64 {
 	var ip project_model.ProjectIssue
-	has, err := db.GetEngine(ctx).Where("issue_id=?", i.ID).Get(&ip)
+	has, err := db.GetEngine(ctx).Where("issue_id=?", issue.ID).Get(&ip)
 	if err != nil || !has {
 		return 0
 	}

models/issues/issue_watch.go

@@ -65,7 +65,7 @@ func GetIssueWatch(ctx context.Context, userID, issueID int64) (iw *IssueWatch,
 		Where("user_id = ?", userID).
 		And("issue_id = ?", issueID).
 		Get(iw)
-	return
+	return iw, exists, err
 }
 
 // CheckIssueWatch check if an user is watching an issue

models/issues/issue_xref.go

@@ -231,46 +231,46 @@ func (issue *Issue) verifyReferencedIssue(stdCtx context.Context, ctx *crossRefe
 }
 
 // AddCrossReferences add cross references
-func (comment *Comment) AddCrossReferences(stdCtx context.Context, doer *user_model.User, removeOld bool) error {
-	if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment {
+func (c *Comment) AddCrossReferences(stdCtx context.Context, doer *user_model.User, removeOld bool) error {
+	if c.Type != CommentTypeCode && c.Type != CommentTypeComment {
 		return nil
 	}
-	if err := comment.LoadIssueCtx(stdCtx); err != nil {
+	if err := c.LoadIssueCtx(stdCtx); err != nil {
 		return err
 	}
 	ctx := &crossReferencesContext{
 		Type:        CommentTypeCommentRef,
 		Doer:        doer,
-		OrigIssue:   comment.Issue,
-		OrigComment: comment,
+		OrigIssue:   c.Issue,
+		OrigComment: c,
 		RemoveOld:   removeOld,
 	}
-	return comment.Issue.createCrossReferences(stdCtx, ctx, "", comment.Content)
+	return c.Issue.createCrossReferences(stdCtx, ctx, "", c.Content)
 }
 
-func (comment *Comment) neuterCrossReferences(ctx context.Context) error {
-	return neuterCrossReferences(ctx, comment.IssueID, comment.ID)
+func (c *Comment) neuterCrossReferences(ctx context.Context) error {
+	return neuterCrossReferences(ctx, c.IssueID, c.ID)
 }
 
 // LoadRefComment loads comment that created this reference from database
-func (comment *Comment) LoadRefComment() (err error) {
-	if comment.RefComment != nil {
+func (c *Comment) LoadRefComment() (err error) {
+	if c.RefComment != nil {
 		return nil
 	}
-	comment.RefComment, err = GetCommentByID(db.DefaultContext, comment.RefCommentID)
-	return
+	c.RefComment, err = GetCommentByID(db.DefaultContext, c.RefCommentID)
+	return err
 }
 
 // LoadRefIssue loads comment that created this reference from database
-func (comment *Comment) LoadRefIssue() (err error) {
-	if comment.RefIssue != nil {
+func (c *Comment) LoadRefIssue() (err error) {
+	if c.RefIssue != nil {
 		return nil
 	}
-	comment.RefIssue, err = GetIssueByID(db.DefaultContext, comment.RefIssueID)
+	c.RefIssue, err = GetIssueByID(db.DefaultContext, c.RefIssueID)
 	if err == nil {
-		err = comment.RefIssue.LoadRepo(db.DefaultContext)
+		err = c.RefIssue.LoadRepo(db.DefaultContext)
 	}
-	return
+	return err
 }
 
 // CommentTypeIsRef returns true if CommentType is a reference from another issue
@@ -279,44 +279,44 @@ func CommentTypeIsRef(t CommentType) bool {
 }
 
 // RefCommentHTMLURL returns the HTML URL for the comment that created this reference
-func (comment *Comment) RefCommentHTMLURL() string {
+func (c *Comment) RefCommentHTMLURL() string {
 	// Edge case for when the reference is inside the title or the description of the referring issue
-	if comment.RefCommentID == 0 {
-		return comment.RefIssueHTMLURL()
+	if c.RefCommentID == 0 {
+		return c.RefIssueHTMLURL()
 	}
-	if err := comment.LoadRefComment(); err != nil { // Silently dropping errors :unamused:
-		log.Error("LoadRefComment(%d): %v", comment.RefCommentID, err)
+	if err := c.LoadRefComment(); err != nil { // Silently dropping errors :unamused:
+		log.Error("LoadRefComment(%d): %v", c.RefCommentID, err)
 		return ""
 	}
-	return comment.RefComment.HTMLURL()
+	return c.RefComment.HTMLURL()
 }
 
 // RefIssueHTMLURL returns the HTML URL of the issue where this reference was created
-func (comment *Comment) RefIssueHTMLURL() string {
-	if err := comment.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
-		log.Error("LoadRefIssue(%d): %v", comment.RefCommentID, err)
+func (c *Comment) RefIssueHTMLURL() string {
+	if err := c.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
+		log.Error("LoadRefIssue(%d): %v", c.RefCommentID, err)
 		return ""
 	}
-	return comment.RefIssue.HTMLURL()
+	return c.RefIssue.HTMLURL()
 }
 
 // RefIssueTitle returns the title of the issue where this reference was created
-func (comment *Comment) RefIssueTitle() string {
-	if err := comment.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
-		log.Error("LoadRefIssue(%d): %v", comment.RefCommentID, err)
+func (c *Comment) RefIssueTitle() string {
+	if err := c.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
+		log.Error("LoadRefIssue(%d): %v", c.RefCommentID, err)
 		return ""
 	}
-	return comment.RefIssue.Title
+	return c.RefIssue.Title
 }
 
 // RefIssueIdent returns the user friendly identity (e.g. "#1234") of the issue where this reference was created
-func (comment *Comment) RefIssueIdent() string {
-	if err := comment.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
-		log.Error("LoadRefIssue(%d): %v", comment.RefCommentID, err)
+func (c *Comment) RefIssueIdent() string {
+	if err := c.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
+		log.Error("LoadRefIssue(%d): %v", c.RefCommentID, err)
 		return ""
 	}
 	// FIXME: check this name for cross-repository references (#7901 if it gets merged)
-	return fmt.Sprintf("#%d", comment.RefIssue.Index)
+	return fmt.Sprintf("#%d", c.RefIssue.Index)
 }
 
 // __________      .__  .__ __________                                     __

models/issues/pull.go

@@ -323,7 +323,7 @@ func (pr *PullRequest) LoadProtectedBranchCtx(ctx context.Context) (err error) {
 		}
 		pr.ProtectedBranch, err = git_model.GetProtectedBranchBy(ctx, pr.BaseRepo.ID, pr.BaseBranch)
 	}
-	return
+	return err
 }
 
 // ReviewCount represents a count of Reviews

models/issues/review.go

@@ -134,7 +134,7 @@ func (r *Review) LoadCodeComments(ctx context.Context) (err error) {
 		return
 	}
 	r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r)
-	return
+	return err
 }
 
 func (r *Review) loadIssue(ctx context.Context) (err error) {
@@ -142,7 +142,7 @@ func (r *Review) loadIssue(ctx context.Context) (err error) {
 		return
 	}
 	r.Issue, err = GetIssueByID(ctx, r.IssueID)
-	return
+	return err
 }
 
 func (r *Review) loadReviewer(ctx context.Context) (err error) {
@@ -150,7 +150,7 @@ func (r *Review) loadReviewer(ctx context.Context) (err error) {
 		return
 	}
 	r.Reviewer, err = user_model.GetUserByIDCtx(ctx, r.ReviewerID)
-	return
+	return err
 }
 
 func (r *Review) loadReviewerTeam(ctx context.Context) (err error) {
@@ -159,7 +159,7 @@ func (r *Review) loadReviewerTeam(ctx context.Context) (err error) {
 	}
 
 	r.ReviewerTeam, err = organization.GetTeamByID(ctx, r.ReviewerTeamID)
-	return
+	return err
 }
 
 // LoadReviewer loads reviewer
@@ -186,7 +186,7 @@ func (r *Review) LoadAttributes(ctx context.Context) (err error) {
 	if err = r.loadReviewerTeam(ctx); err != nil {
 		return
 	}
-	return
+	return err
 }
 
 // GetReviewByID returns the review by the given ID
@@ -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 {
@@ -548,21 +548,21 @@ func GetTeamReviewerByIssueIDAndTeamID(ctx context.Context, issueID, teamID int6
 		return nil, ErrReviewNotExist{0}
 	}
 
-	return
+	return review, err
 }
 
 // MarkReviewsAsStale marks existing reviews as stale
 func MarkReviewsAsStale(issueID int64) (err error) {
 	_, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `review` SET stale=? WHERE issue_id=?", true, issueID)
 
-	return
+	return err
 }
 
 // MarkReviewsAsNotStale marks existing reviews as not stale for a giving commit SHA
 func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) {
 	_, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `review` SET stale=? WHERE issue_id=? AND commit_id=?", false, issueID, commitID)
 
-	return
+	return err
 }
 
 // DismissReview change the dismiss status of a review
@@ -579,7 +579,7 @@ func DismissReview(review *Review, isDismiss bool) (err error) {
 
 	_, err = db.GetEngine(db.DefaultContext).ID(review.ID).Cols("dismissed").Update(review)
 
-	return
+	return err
 }
 
 // InsertReviews inserts review and review comments

models/issues/stopwatch.go

@@ -63,7 +63,7 @@ func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, ex
 		Where("user_id = ?", userID).
 		And("issue_id = ?", issueID).
 		Get(sw)
-	return
+	return sw, exists, err
 }
 
 // UserIDCount is a simple coalition of UserID and Count
@@ -130,7 +130,7 @@ func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopw
 	exists, err = db.GetEngine(ctx).
 		Where("user_id = ?", userID).
 		Get(sw)
-	return
+	return exists, sw, err
 }
 
 // FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore

models/issues/tracked_time.go

@@ -63,7 +63,7 @@ func (t *TrackedTime) loadAttributes(ctx context.Context) (err error) {
 			return
 		}
 	}
-	return
+	return err
 }
 
 // LoadAttributes load Issue, User
@@ -73,7 +73,7 @@ func (tl TrackedTimeList) LoadAttributes() (err error) {
 			return err
 		}
 	}
-	return
+	return err
 }
 
 // FindTrackedTimesOptions represent the filters for tracked times. If an ID is 0 it will be ignored.
@@ -130,7 +130,7 @@ func (opts *FindTrackedTimesOptions) toSession(e db.Engine) db.Engine {
 // GetTrackedTimes returns all tracked times that fit to the given options.
 func GetTrackedTimes(ctx context.Context, options *FindTrackedTimesOptions) (trackedTimes TrackedTimeList, err error) {
 	err = options.toSession(db.GetEngine(ctx)).Find(&trackedTimes)
-	return
+	return trackedTimes, err
 }
 
 // CountTrackedTimes returns count of tracked times that fit to the given options.
@@ -291,7 +291,7 @@ func deleteTimes(ctx context.Context, opts FindTrackedTimesOptions) (removedTime
 	}
 
 	_, err = opts.toSession(db.GetEngine(ctx)).Table("tracked_time").Cols("deleted").Update(&TrackedTime{Deleted: true})
-	return
+	return removedTime, err
 }
 
 func deleteTime(ctx context.Context, t *TrackedTime) error {