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

Enable more revive linter rules #30608

Merged
merged 11 commits into from
Apr 22, 2024
55 changes: 32 additions & 23 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
linters:
enable-all: false
disable-all: true
fast: false
enable:
- bidichk
# - deadcode # deprecated - https://github.com/golangci/golangci-lint/issues/1841
- depguard
- dupl
- errcheck
- forbidigo
- gocritic
# - gocyclo # The cyclomatic complexety of a lot of functions is too high, we should refactor those another time.
- gofmt
- gofumpt
- gosimple
Expand All @@ -17,20 +18,18 @@ linters:
- nolintlint
- revive
- staticcheck
# - structcheck # deprecated - https://github.com/golangci/golangci-lint/issues/1841
- stylecheck
- typecheck
- unconvert
- unused
# - varcheck # deprecated - https://github.com/golangci/golangci-lint/issues/1841
- wastedassign
enable-all: false
disable-all: true
fast: false

run:
timeout: 10m

output:
sort-results: true

linters-settings:
stylecheck:
checks: ["all", "-ST1005", "-ST1003"]
Expand All @@ -47,27 +46,37 @@ linters-settings:
errorCode: 1
warningCode: 1
rules:
- name: atomic
- name: bare-return
- name: blank-imports
- name: constant-logical-expr
- name: context-as-argument
- name: context-keys-type
- name: dot-imports
- name: duplicated-imports
- name: empty-lines
- name: error-naming
- name: error-return
- name: error-strings
- name: error-naming
- name: errorf
- name: exported
- name: identical-branches
- name: if-return
- name: increment-decrement
- name: var-naming
- name: var-declaration
- name: indent-error-flow
- name: modifies-value-receiver
- name: package-comments
- name: range
- name: receiver-naming
- name: redefines-builtin-id
- name: string-of-int
- name: superfluous-else
- name: time-naming
- name: unconditional-recursion
- name: unexported-return
- name: indent-error-flow
- name: errorf
- name: duplicated-imports
- name: modifies-value-receiver
- name: unreachable-code
- name: var-declaration
- name: var-naming
gofumpt:
extra-rules: true
depguard:
Expand All @@ -93,8 +102,8 @@ issues:
max-issues-per-linter: 0
max-same-issues: 0
exclude-dirs: [node_modules, public, web_src]
exclude-case-sensitive: true
exclude-rules:
# Exclude some linters from running on tests files.
- path: _test\.go
linters:
- gocyclo
Expand All @@ -112,19 +121,19 @@ issues:
- path: cmd
linters:
- forbidigo
- linters:
- text: "webhook"
linters:
- dupl
text: "webhook"
- linters:
- text: "`ID' should not be capitalized"
linters:
- gocritic
text: "`ID' should not be capitalized"
- linters:
- text: "swagger"
linters:
- unused
- deadcode
text: "swagger"
- linters:
- text: "argument x is overwritten before first use"
linters:
- staticcheck
text: "argument x is overwritten before first use"
- text: "commentFormatting: put a space between `//` and comment text"
linters:
- gocritic
Expand Down
2 changes: 1 addition & 1 deletion cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ func hookPrintResult(output, isCreate bool, branch, url string) {
fmt.Fprintf(os.Stderr, " %s\n", url)
}
fmt.Fprintln(os.Stderr, "")
os.Stderr.Sync()
_ = os.Stderr.Sync()
}

func pushOptions() map[string]string {
Expand Down
1 change: 0 additions & 1 deletion models/asymkey/gpg_key_commit_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *CommitVerific
Reason: "gpg.error.no_committer_account",
}
}

}
}

Expand Down
1 change: 0 additions & 1 deletion models/db/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ func NamesToBean(names ...string) ([]any, error) {
// Need to map provided names to beans...
beanMap := make(map[string]any)
for _, bean := range tables {

beanMap[strings.ToLower(reflect.Indirect(reflect.ValueOf(bean)).Type().Name())] = bean
beanMap[strings.ToLower(x.TableName(bean))] = bean
beanMap[strings.ToLower(x.TableName(bean, true))] = bean
Expand Down
2 changes: 0 additions & 2 deletions models/issues/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,9 @@ func CreateReview(ctx context.Context, opts CreateReviewOptions) (*Review, error
return nil, err
}
}

} else if opts.ReviewerTeam != nil {
review.Type = ReviewTypeRequest
review.ReviewerTeamID = opts.ReviewerTeam.ID

} else {
return nil, fmt.Errorf("provide either reviewer or reviewer team")
}
Expand Down
5 changes: 0 additions & 5 deletions models/migrations/base/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func RecreateTable(sess *xorm.Session, bean any) error {
log.Error("Unable to recreate uniques on table %s. Error: %v", tableName, err)
return err
}

case setting.Database.Type.IsMySQL():
// MySQL will drop all the constraints on the old table
if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil {
Expand Down Expand Up @@ -228,7 +227,6 @@ func RecreateTable(sess *xorm.Session, bean any) error {
return err
}
sequenceMap[sequence] = sequenceData

}

// CASCADE causes postgres to drop all the constraints on the old table
Expand Down Expand Up @@ -293,9 +291,7 @@ func RecreateTable(sess *xorm.Session, bean any) error {
return err
}
}

}

case setting.Database.Type.IsMSSQL():
// MSSQL will drop all the constraints on the old table
if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil {
Expand All @@ -308,7 +304,6 @@ func RecreateTable(sess *xorm.Session, bean any) error {
log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err)
return err
}

default:
log.Fatal("Unrecognized DB")
}
Expand Down
2 changes: 0 additions & 2 deletions models/migrations/v1_11/v111.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ func AddBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
for _, u := range units {
var found bool
for _, team := range teams {

var teamU []*TeamUnit
var unitEnabled bool
err = sess.Where("team_id = ?", team.ID).Find(&teamU)
Expand Down Expand Up @@ -331,7 +330,6 @@ func AddBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
}

if !protectedBranch.EnableApprovalsWhitelist {

perm, err := getUserRepoPermission(sess, baseRepo, reviewer)
if err != nil {
return false, err
Expand Down
4 changes: 2 additions & 2 deletions models/migrations/v1_20/v250.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func ChangeContainerMetadataMultiArch(x *xorm.Engine) error {

// Convert to new metadata format

new := &MetadataNew{
newMetadata := &MetadataNew{
Type: old.Type,
IsTagged: old.IsTagged,
Platform: old.Platform,
Expand All @@ -119,7 +119,7 @@ func ChangeContainerMetadataMultiArch(x *xorm.Engine) error {
Manifests: manifests,
}

metadataJSON, err := json.Marshal(new)
metadataJSON, err := json.Marshal(newMetadata)
if err != nil {
return err
}
Expand Down
1 change: 0 additions & 1 deletion models/migrations/v1_6/v71.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func AddScratchHash(x *xorm.Engine) error {
if _, err := sess.ID(tfa.ID).Cols("scratch_salt, scratch_hash").Update(tfa); err != nil {
return fmt.Errorf("couldn't add in scratch_hash and scratch_salt: %w", err)
}

}
}

Expand Down
1 change: 0 additions & 1 deletion models/migrations/v1_9/v85.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func HashAppToken(x *xorm.Engine) error {
if _, err := sess.ID(token.ID).Cols("token_hash, token_salt, token_last_eight, sha1").Update(token); err != nil {
return fmt.Errorf("couldn't add in sha1, token_hash, token_salt and token_last_eight: %w", err)
}

}
}

Expand Down
3 changes: 1 addition & 2 deletions models/organization/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,8 @@ func GetTeamIDsByNames(ctx context.Context, orgID int64, names []string, ignoreN
if err != nil {
if ignoreNonExistent {
continue
} else {
return nil, err
}
return nil, err
}
ids = append(ids, u.ID)
}
Expand Down
2 changes: 0 additions & 2 deletions models/project/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,11 @@ func createBoardsForProjectsType(ctx context.Context, project *Project) error {
var items []string

switch project.BoardType {

case BoardTypeBugTriage:
items = setting.Project.ProjectBoardBugTriageType

case BoardTypeBasicKanban:
items = setting.Project.ProjectBoardBasicKanbanType

case BoardTypeNone:
fallthrough
default:
Expand Down
1 change: 0 additions & 1 deletion models/repo/user_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64)
// the owner of a private repo needs to be explicitly added.
cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID})
}

} else {
// This is a "public" repository:
// Any user that has read access, is a watcher or organization member can be requested to review
Expand Down
3 changes: 1 addition & 2 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,9 +988,8 @@ func GetUserIDsByNames(ctx context.Context, names []string, ignoreNonExistent bo
if err != nil {
if ignoreNonExistent {
continue
} else {
return nil, err
}
return nil, err
}
ids = append(ids, u.ID)
}
Expand Down
12 changes: 6 additions & 6 deletions modules/auth/password/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ func NewComplexity() {
func setupComplexity(values []string) {
if len(values) != 1 || values[0] != "off" {
for _, val := range values {
if complex, ok := charComplexities[val]; ok {
validChars += complex.ValidChars
requiredList = append(requiredList, complex)
if complexity, ok := charComplexities[val]; ok {
validChars += complexity.ValidChars
requiredList = append(requiredList, complexity)
}
}
if len(requiredList) == 0 {
// No valid character classes found; use all classes as default
for _, complex := range charComplexities {
validChars += complex.ValidChars
requiredList = append(requiredList, complex)
for _, complexity := range charComplexities {
validChars += complexity.ValidChars
requiredList = append(requiredList, complexity)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions modules/git/batch_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,10 @@ func ParseTreeLine(objectFormat ObjectFormat, rd *bufio.Reader, modeBuf, fnameBu

// Deal with the binary hash
idx = 0
len := objectFormat.FullLength() / 2
for idx < len {
length := objectFormat.FullLength() / 2
for idx < length {
var read int
read, err = rd.Read(shaBuf[idx:len])
read, err = rd.Read(shaBuf[idx:length])
n += read
if err != nil {
return mode, fname, sha, n, err
Expand Down
3 changes: 1 addition & 2 deletions modules/git/commit_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ readLoop:
if len(line) > 0 && line[0] == ' ' {
_, _ = signatureSB.Write(line[1:])
continue
} else {
pgpsig = false
}
pgpsig = false
}

if !message {
Expand Down
1 change: 0 additions & 1 deletion modules/git/pipeline/lfs_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err
errChan <- err
break
}

}
}()

Expand Down
8 changes: 4 additions & 4 deletions modules/git/repo_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,18 +251,18 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions)
return nil, err
}

len := objectFormat.FullLength()
length := objectFormat.FullLength()
commits := []*Commit{}
shaline := make([]byte, len+1)
shaline := make([]byte, length+1)
for {
n, err := io.ReadFull(stdoutReader, shaline)
if err != nil || n < len {
if err != nil || n < length {
if err == io.EOF {
err = nil
}
return commits, err
}
objectID, err := NewIDFromString(string(shaline[0:len]))
objectID, err := NewIDFromString(string(shaline[0:length]))
if err != nil {
return nil, err
}
Expand Down
1 change: 0 additions & 1 deletion modules/git/submodule.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string {
// ex: git@try.gitea.io:go-gitea/gitea
match := scpSyntax.FindAllStringSubmatch(refURI, -1)
if len(match) > 0 {

m := match[0]
refHostname := m[2]
pth := m[3]
Expand Down
2 changes: 0 additions & 2 deletions modules/indexer/code/bleve/bleve.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ func (b *Indexer) addDelete(filename string, repo *repo_model.Repository, batch
func (b *Indexer) Index(ctx context.Context, repo *repo_model.Repository, sha string, changes *internal.RepoChanges) error {
batch := inner_bleve.NewFlushingBatch(b.inner.Indexer, maxBatchSize)
if len(changes.Updates) > 0 {

// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
if err := git.EnsureValidGitRepository(ctx, repo.RepoPath()); err != nil {
log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
Expand Down Expand Up @@ -335,7 +334,6 @@ func (b *Indexer) Search(ctx context.Context, opts *internal.SearchOptions) (int
if result, err = b.inner.Indexer.Search(facetRequest); err != nil {
return 0, nil, nil, err
}

}
languagesFacet := result.Facets["languages"]
for _, term := range languagesFacet.Terms.Terms() {
Expand Down
1 change: 0 additions & 1 deletion modules/indexer/issues/elasticsearch/elasticsearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
query := elastic.NewBoolQuery()

if options.Keyword != "" {

searchType := esMultiMatchTypePhrasePrefix
if options.IsFuzzyKeyword {
searchType = esMultiMatchTypeBestFields
Expand Down