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

Update tool dependencies, lock govulncheck and actionlint #25655

Merged
merged 16 commits into from
Jul 9, 2023
Merged
25 changes: 15 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,21 @@ linters-settings:
extra-rules: true
lang-version: "1.20"
depguard:
list-type: denylist
# Check the list against standard lib.
include-go-root: true
packages-with-error-message:
- encoding/json: "use gitea's modules/json instead of encoding/json"
- github.com/unknwon/com: "use gitea's util and replacements"
- io/ioutil: "use os or io instead"
- golang.org/x/exp: "it's experimental and unreliable."
- code.gitea.io/gitea/modules/git/internal: "do not use the internal package, use AddXxx function instead"
- gopkg.in/ini.v1: "do not use the ini package, use gitea's config system instead"
rules:
main:
deny:
- pkg: encoding/json
desc: use gitea's modules/json instead of encoding/json
- pkg: github.com/unknwon/com
desc: use gitea's util and replacements
- pkg: io/ioutil
desc: use os or io instead
- pkg: golang.org/x/exp
desc: it's experimental and unreliable
- pkg: code.gitea.io/gitea/modules/git/internal
desc: do not use the internal package, use AddXxx function instead
- pkg: gopkg.in/ini.v1
desc: do not use the ini package, use gitea's config system instead

issues:
max-issues-per-linter: 0
Expand Down
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ COMMA := ,

XGO_VERSION := go-1.20.x

AIR_PACKAGE ?= github.com/cosmtrek/air@v1.43.0
AIR_PACKAGE ?= github.com/cosmtrek/air@v1.44.0
EDITORCONFIG_CHECKER_PACKAGE ?= github.com/editorconfig-checker/editorconfig-checker/cmd/editorconfig-checker@2.7.0
GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.5.0
GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2
GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
GXZ_PAGAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.11
MISSPELL_PACKAGE ?= github.com/client9/misspell/cmd/misspell@v0.3.4
SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.30.4
SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.30.5
XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest
GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0
GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@latest
ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@latest
GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v0.2.0
ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@v1.6.25

DOCKER_IMAGE ?= gitea/gitea
DOCKER_TAG ?= latest
Expand Down
10 changes: 5 additions & 5 deletions models/activities/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func getIssueNotification(ctx context.Context, userID, issueID int64) (*Notifica
// NotificationsForUser returns notifications for a given user and status
func NotificationsForUser(ctx context.Context, user *user_model.User, statuses []NotificationStatus, page, perPage int) (notifications NotificationList, err error) {
if len(statuses) == 0 {
return
return nil, nil
}

sess := db.GetEngine(ctx).
Expand Down Expand Up @@ -372,16 +372,16 @@ func CountUnread(ctx context.Context, userID int64) int64 {
// LoadAttributes load Repo Issue User and Comment if not loaded
func (n *Notification) LoadAttributes(ctx context.Context) (err error) {
if err = n.loadRepo(ctx); err != nil {
return
return err
}
if err = n.loadIssue(ctx); err != nil {
return
return err
}
if err = n.loadUser(ctx); err != nil {
return
return err
}
if err = n.loadComment(ctx); err != nil {
return
return err
}
return err
Copy link
Member

@puni9869 puni9869 Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker.

https://github.com/go-gitea/gitea/pull/25655/files#diff-a9a3a131e7fcfb1641580afd9381afe56f44022d9a5e4ba651361f9230fa1694L386
what is the need of return err
we can put return nil

Sometime it hard to recall the Named return values in golang.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we do report the error back so we need to retur err and not nil here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also a similar case of #25655 (comment)

Some code like return mode, fname, sha, n, err and return sha, typ, size, err are written as is intentionally, to avoid breaking.

The purpose of it is to avoid changing old code too much (which might break something or cause difficulty for reviewing).

In short: In Golang, return err is always right for named return values, but if you want to change it, the developers should always spend more time on thinking about "what value should be returned".

}
Expand Down
2 changes: 1 addition & 1 deletion models/asymkey/gpg_key_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func populateHash(hashFunc crypto.Hash, msg []byte) (hash.Hash, error) {
func readArmoredSign(r io.Reader) (body io.Reader, err error) {
block, err := armor.Decode(r)
if err != nil {
return
return nil, err
}
if block.Type != openpgp.SignatureType {
return nil, fmt.Errorf("expected '" + openpgp.SignatureType + "', got: " + block.Type)
Expand Down
10 changes: 5 additions & 5 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) {

err = json.Unmarshal([]byte(c.Content), &data)
if err != nil {
return
return err
}

c.IsForcePush = data.IsForcePush
Expand Down Expand Up @@ -925,7 +925,7 @@ func createIssueDependencyComment(ctx context.Context, doer *user_model.User, is
cType = CommentTypeRemoveDependency
}
if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

// Make two comments, one in each issue
Expand All @@ -937,7 +937,7 @@ func createIssueDependencyComment(ctx context.Context, doer *user_model.User, is
DependentIssueID: dependentIssue.ID,
}
if _, err = CreateComment(ctx, opts); err != nil {
return
return err
}

opts = &CreateCommentOptions{
Expand Down Expand Up @@ -1170,11 +1170,11 @@ func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullReques
return nil, fmt.Errorf("comment type %d cannot be used to create an auto merge comment", typ)
}
if err = pr.LoadIssue(ctx); err != nil {
return
return nil, err
}

if err = pr.LoadBaseRepo(ctx); err != nil {
return
return nil, err
}

comment, err = CreateComment(ctx, &CreateCommentOptions{
Expand Down
22 changes: 9 additions & 13 deletions models/issues/comment_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,42 +468,38 @@ func (comments CommentList) loadReviews(ctx context.Context) error {
// loadAttributes loads all attributes
func (comments CommentList) loadAttributes(ctx context.Context) (err error) {
if err = comments.LoadPosters(ctx); err != nil {
return
return err
}

if err = comments.loadLabels(ctx); err != nil {
return
return err
}

if err = comments.loadMilestones(ctx); err != nil {
return
return err
}

if err = comments.loadOldMilestones(ctx); err != nil {
return
return err
}

if err = comments.loadAssignees(ctx); err != nil {
return
return err
}

if err = comments.LoadAttachments(ctx); err != nil {
return
return err
}

if err = comments.loadReviews(ctx); err != nil {
return
return err
}

if err = comments.LoadIssues(ctx); err != nil {
return
}

if err = comments.loadDependentIssues(ctx); err != nil {
return
return err
}

return nil
return comments.loadDependentIssues(ctx)
}

// LoadAttributes loads attributes of the comments, except for attachments and
Expand Down
15 changes: 7 additions & 8 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ func (issue *Issue) LoadPoster(ctx context.Context) (err error) {
if !user_model.IsErrUserNotExist(err) {
return fmt.Errorf("getUserByID.(poster) [%d]: %w", issue.PosterID, err)
}
err = nil
return
return nil
}
}
return err
Expand Down Expand Up @@ -316,27 +315,27 @@ func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
// LoadAttributes loads the attribute of this issue.
func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

if err = issue.LoadPoster(ctx); err != nil {
return
return err
}

if err = issue.LoadLabels(ctx); err != nil {
return
return err
}

if err = issue.LoadMilestone(ctx); err != nil {
return
return err
}

if err = issue.LoadProject(ctx); err != nil {
return
return err
}

if err = issue.LoadAssignees(ctx); err != nil {
return
return err
}

if err = issue.LoadPullRequest(ctx); err != nil && !IsErrPullRequestNotExist(err) {
Expand Down
4 changes: 2 additions & 2 deletions models/issues/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func newIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
}

if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

opts := &CreateCommentOptions{
Expand Down Expand Up @@ -168,7 +168,7 @@ func deleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *use
}

if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

opts := &CreateCommentOptions{
Expand Down
34 changes: 17 additions & 17 deletions models/issues/issue_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,10 @@ func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_mo
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
if len(mentions) == 0 {
return
return nil, nil
}
if err = issue.LoadRepo(ctx); err != nil {
return
return nil, err
}

resolved := make(map[string]bool, 10)
Expand Down Expand Up @@ -635,7 +635,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
}
}
if len(mentionUsers) == 0 {
return
return users, err
}

if users == nil {
Expand Down Expand Up @@ -702,66 +702,66 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
// Delete content histories
if _, err = sess.In("issue_id", deleteCond).
Delete(&ContentHistory{}); err != nil {
return
return nil, err
}

// Delete comments and attachments
if _, err = sess.In("issue_id", deleteCond).
Delete(&Comment{}); err != nil {
return
return nil, err
}

// Dependencies for issues in this repository
if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueDependency{}); err != nil {
return
return nil, err
}

// Delete dependencies for issues in other repositories
if _, err = sess.In("dependency_id", deleteCond).
Delete(&IssueDependency{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueUser{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&Reaction{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueWatch{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&Stopwatch{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&TrackedTime{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&project_model.ProjectIssue{}); err != nil {
return
return nil, err
}

if _, err = sess.In("dependent_issue_id", deleteCond).
Delete(&Comment{}); err != nil {
return
return nil, err
}

var attachments []*repo_model.Attachment
if err = sess.In("issue_id", deleteCond).
Find(&attachments); err != nil {
return
return nil, err
}

for j := range attachments {
Expand All @@ -770,11 +770,11 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []

if _, err = sess.In("issue_id", deleteCond).
Delete(&repo_model.Attachment{}); err != nil {
return
return nil, err
}

if _, err = db.DeleteByBean(ctx, &Issue{RepoID: repoID}); err != nil {
return
return nil, err
}

return attachmentPaths, err
Expand Down
Loading