From ca2a32b0876585ff7438be1cd37f696ebe1432f6 Mon Sep 17 00:00:00 2001 From: chavacava Date: Tue, 1 Oct 2024 12:14:02 +0200 Subject: [PATCH] code cleanup (#1054) --- rule/add-constant.go | 5 ++- rule/argument-limit.go | 61 +++++++++++++++++--------------- rule/blank-imports.go | 8 ++--- rule/bool-literal-in-expr.go | 1 - rule/cognitive-complexity.go | 22 ++++++------ rule/comment-spacings.go | 17 ++++----- rule/constant-logical-expr.go | 5 +-- rule/cyclomatic.go | 58 ++++++++++++++++-------------- rule/deep-exit.go | 5 +-- rule/defer.go | 8 +++-- rule/dot-imports.go | 7 ++-- rule/enforce-map-style.go | 4 +-- rule/enforce-slice-style.go | 16 ++++----- rule/file-header.go | 20 ++++++----- rule/function-length.go | 20 ++++++----- rule/function-result-limit.go | 37 +++++++++++-------- rule/get-return.go | 22 ++++++++---- rule/identical-branches.go | 15 ++++---- rule/line-length-limit.go | 23 ++++++------ rule/max-control-nesting.go | 2 +- rule/max-public-structs.go | 25 +++++++------ rule/struct-tag.go | 46 +++++++++++++----------- rule/time-equal.go | 19 +++++----- rule/time-naming.go | 5 +-- rule/unchecked-type-assertion.go | 26 +++++++------- rule/unconditional-recursion.go | 7 ++-- rule/unhandled-error.go | 41 +++++++++++---------- rule/var-declarations.go | 16 +++++---- rule/var-naming.go | 24 ++++++------- rule/waitgroup-by-value.go | 2 +- 30 files changed, 315 insertions(+), 252 deletions(-) diff --git a/rule/add-constant.go b/rule/add-constant.go index 73dfa932c..f7a2447be 100644 --- a/rule/add-constant.go +++ b/rule/add-constant.go @@ -160,12 +160,15 @@ func (w *lintAddConstantRule) isIgnoredFunc(fName string) bool { } func (w *lintAddConstantRule) checkStrLit(n *ast.BasicLit) { + const IgnoreMarker = -1 + if w.allowList[kindSTRING][n.Value] { return } count := w.strLits[n.Value] - if count >= 0 { + mustCheck := count > IgnoreMarker + if mustCheck { w.strLits[n.Value] = count + 1 if w.strLits[n.Value] > w.strLitLimit { w.onFailure(lint.Failure{ diff --git a/rule/argument-limit.go b/rule/argument-limit.go index 8120288fd..b6ce0e81a 100644 --- a/rule/argument-limit.go +++ b/rule/argument-limit.go @@ -10,7 +10,7 @@ import ( // ArgumentsLimitRule lints given else constructs. type ArgumentsLimitRule struct { - total int + max int sync.Mutex } @@ -19,18 +19,20 @@ const defaultArgumentsLimit = 8 func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.total == 0 { - if len(arguments) < 1 { - r.total = defaultArgumentsLimit - return - } + if r.max != 0 { + return + } - total, ok := arguments[0].(int64) // Alt. non panicking version - if !ok { - panic(`invalid value passed as argument number to the "argument-limit" rule`) - } - r.total = int(total) + if len(arguments) < 1 { + r.max = defaultArgumentsLimit + return } + + maxArguments, ok := arguments[0].(int64) // Alt. non panicking version + if !ok { + panic(`invalid value passed as argument number to the "argument-limit" rule`) + } + r.max = int(maxArguments) } // Apply applies the rule to given file. @@ -43,7 +45,7 @@ func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) [] } walker := lintArgsNum{ - total: r.total, + max: r.max, onFailure: onFailure, } @@ -58,27 +60,30 @@ func (*ArgumentsLimitRule) Name() string { } type lintArgsNum struct { - total int + max int onFailure func(lint.Failure) } func (w lintArgsNum) Visit(n ast.Node) ast.Visitor { node, ok := n.(*ast.FuncDecl) - if ok { - num := 0 - for _, l := range node.Type.Params.List { - for range l.Names { - num++ - } - } - if num > w.total { - w.onFailure(lint.Failure{ - Confidence: 1, - Failure: fmt.Sprintf("maximum number of arguments per function exceeded; max %d but got %d", w.total, num), - Node: node.Type, - }) - return w + if !ok { + return w + } + + num := 0 + for _, l := range node.Type.Params.List { + for range l.Names { + num++ } } - return w + + if num > w.max { + w.onFailure(lint.Failure{ + Confidence: 1, + Failure: fmt.Sprintf("maximum number of arguments per function exceeded; max %d but got %d", w.max, num), + Node: node.Type, + }) + } + + return nil // skip visiting the body of the function } diff --git a/rule/blank-imports.go b/rule/blank-imports.go index a3d50b4f7..0ddb4aad2 100644 --- a/rule/blank-imports.go +++ b/rule/blank-imports.go @@ -22,9 +22,8 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu } const ( - message = "a blank import should be only in a main or test package, or have a comment justifying it" - category = "imports" - + message = "a blank import should be only in a main or test package, or have a comment justifying it" + category = "imports" embedImportPath = `"embed"` ) @@ -39,7 +38,8 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu continue // Ignore non-blank imports. } - if i > 0 { + isNotFirstElement := i > 0 + if isNotFirstElement { prev := file.AST.Imports[i-1] prevPos := file.ToPosition(prev.Pos()) diff --git a/rule/bool-literal-in-expr.go b/rule/bool-literal-in-expr.go index d6150339b..71551e55a 100644 --- a/rule/bool-literal-in-expr.go +++ b/rule/bool-literal-in-expr.go @@ -45,7 +45,6 @@ func (w *lintBoolLiteral) Visit(node ast.Node) ast.Visitor { lexeme, ok := isExprABooleanLit(n.X) if !ok { lexeme, ok = isExprABooleanLit(n.Y) - if !ok { return w } diff --git a/rule/cognitive-complexity.go b/rule/cognitive-complexity.go index 1973faef8..83640fd3d 100644 --- a/rule/cognitive-complexity.go +++ b/rule/cognitive-complexity.go @@ -21,19 +21,21 @@ const defaultMaxCognitiveComplexity = 7 func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.maxComplexity == 0 { + if r.maxComplexity != 0 { + return // already configured + } - if len(arguments) < 1 { - r.maxComplexity = defaultMaxCognitiveComplexity - return - } + if len(arguments) < 1 { + r.maxComplexity = defaultMaxCognitiveComplexity + return + } - complexity, ok := arguments[0].(int64) - if !ok { - panic(fmt.Sprintf("invalid argument type for cognitive-complexity, expected int64, got %T", arguments[0])) - } - r.maxComplexity = int(complexity) + complexity, ok := arguments[0].(int64) + if !ok { + panic(fmt.Sprintf("invalid argument type for cognitive-complexity, expected int64, got %T", arguments[0])) } + + r.maxComplexity = int(complexity) } // Apply applies the rule to given file. diff --git a/rule/comment-spacings.go b/rule/comment-spacings.go index bfb7eaf23..f72151301 100644 --- a/rule/comment-spacings.go +++ b/rule/comment-spacings.go @@ -18,16 +18,17 @@ type CommentSpacingsRule struct { func (r *CommentSpacingsRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() + if r.allowList != nil { + return // already configured + } - if r.allowList == nil { - r.allowList = []string{} - for _, arg := range arguments { - allow, ok := arg.(string) // Alt. non panicking version - if !ok { - panic(fmt.Sprintf("invalid argument %v for %s; expected string but got %T", arg, r.Name(), arg)) - } - r.allowList = append(r.allowList, `//`+allow) + r.allowList = []string{} + for _, arg := range arguments { + allow, ok := arg.(string) // Alt. non panicking version + if !ok { + panic(fmt.Sprintf("invalid argument %v for %s; expected string but got %T", arg, r.Name(), arg)) } + r.allowList = append(r.allowList, `//`+allow) } } diff --git a/rule/constant-logical-expr.go b/rule/constant-logical-expr.go index 36cd641f7..9e34d3d16 100644 --- a/rule/constant-logical-expr.go +++ b/rule/constant-logical-expr.go @@ -41,8 +41,9 @@ func (w *lintConstantLogicalExpr) Visit(node ast.Node) ast.Visitor { return w } - if gofmt(n.X) != gofmt(n.Y) { // check if subexpressions are the same - return w + subExpressionsAreNotEqual := gofmt(n.X) != gofmt(n.Y) + if subExpressionsAreNotEqual { + return w // nothing to say } // Handles cases like: a <= a, a == a, a >= a diff --git a/rule/cyclomatic.go b/rule/cyclomatic.go index 9f6d50043..10413de24 100644 --- a/rule/cyclomatic.go +++ b/rule/cyclomatic.go @@ -22,18 +22,20 @@ const defaultMaxCyclomaticComplexity = 10 func (r *CyclomaticRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.maxComplexity == 0 { - if len(arguments) < 1 { - r.maxComplexity = defaultMaxCyclomaticComplexity - return - } + if r.maxComplexity != 0 { + return // already configured + } - complexity, ok := arguments[0].(int64) // Alt. non panicking version - if !ok { - panic(fmt.Sprintf("invalid argument for cyclomatic complexity; expected int but got %T", arguments[0])) - } - r.maxComplexity = int(complexity) + if len(arguments) < 1 { + r.maxComplexity = defaultMaxCyclomaticComplexity + return + } + + complexity, ok := arguments[0].(int64) // Alt. non panicking version + if !ok { + panic(fmt.Sprintf("invalid argument for cyclomatic complexity; expected int but got %T", arguments[0])) } + r.maxComplexity = int(complexity) } // Apply applies the rule to given file. @@ -70,31 +72,35 @@ type lintCyclomatic struct { func (w lintCyclomatic) Visit(_ ast.Node) ast.Visitor { f := w.file for _, decl := range f.AST.Decls { - if fn, ok := decl.(*ast.FuncDecl); ok { - c := complexity(fn) - if c > w.complexity { - w.onFailure(lint.Failure{ - Confidence: 1, - Category: "maintenance", - Failure: fmt.Sprintf("function %s has cyclomatic complexity %d (> max enabled %d)", - funcName(fn), c, w.complexity), - Node: fn, - }) - } + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + + c := complexity(fn) + if c > w.complexity { + w.onFailure(lint.Failure{ + Confidence: 1, + Category: "maintenance", + Failure: fmt.Sprintf("function %s has cyclomatic complexity %d (> max enabled %d)", + funcName(fn), c, w.complexity), + Node: fn, + }) } } + return nil } // funcName returns the name representation of a function or method: // "(Type).Name" for methods or simply "Name" for functions. func funcName(fn *ast.FuncDecl) string { - if fn.Recv != nil { - if fn.Recv.NumFields() > 0 { - typ := fn.Recv.List[0].Type - return fmt.Sprintf("(%s).%s", recvString(typ), fn.Name) - } + declarationHasReceiver := fn.Recv != nil && fn.Recv.NumFields() > 0 + if declarationHasReceiver { + typ := fn.Recv.List[0].Type + return fmt.Sprintf("(%s).%s", recvString(typ), fn.Name) } + return fn.Name.Name } diff --git a/rule/deep-exit.go b/rule/deep-exit.go index 918d4294a..7b3dd0f82 100644 --- a/rule/deep-exit.go +++ b/rule/deep-exit.go @@ -73,9 +73,10 @@ func (w lintDeepExit) Visit(node ast.Node) ast.Visitor { return w } - fn := fc.Sel.Name pkg := id.Name - if w.exitFunctions[pkg] != nil && w.exitFunctions[pkg][fn] { // it's a call to an exit function + fn := fc.Sel.Name + isACallToExitFunction := w.exitFunctions[pkg] != nil && w.exitFunctions[pkg][fn] + if isACallToExitFunction { w.onFailure(lint.Failure{ Confidence: 1, Node: ce, diff --git a/rule/defer.go b/rule/defer.go index 9a1972274..3c31d507b 100644 --- a/rule/defer.go +++ b/rule/defer.go @@ -16,10 +16,12 @@ type DeferRule struct { func (r *DeferRule) configure(arguments lint.Arguments) { r.Lock() - if r.allow == nil { - r.allow = r.allowFromArgs(arguments) + defer r.Unlock() + if r.allow != nil { + return // already configured } - r.Unlock() + + r.allow = r.allowFromArgs(arguments) } // Apply applies the rule to given file. diff --git a/rule/dot-imports.go b/rule/dot-imports.go index 6b877677d..523b48197 100644 --- a/rule/dot-imports.go +++ b/rule/dot-imports.go @@ -81,12 +81,13 @@ type lintImports struct { } func (w lintImports) Visit(_ ast.Node) ast.Visitor { - for _, is := range w.fileAst.Imports { - if is.Name != nil && is.Name.Name == "." && !w.allowPackages.isAllowedPackage(is.Path.Value) { + for _, importSpec := range w.fileAst.Imports { + isDotImport := importSpec.Name != nil && importSpec.Name.Name == "." + if isDotImport && !w.allowPackages.isAllowedPackage(importSpec.Path.Value) { w.onFailure(lint.Failure{ Confidence: 1, Failure: "should not use dot imports", - Node: is, + Node: importSpec, Category: "imports", }) } diff --git a/rule/enforce-map-style.go b/rule/enforce-map-style.go index 36ac2374c..1c4717c2c 100644 --- a/rule/enforce-map-style.go +++ b/rule/enforce-map-style.go @@ -94,8 +94,8 @@ func (r *EnforceMapStyleRule) Apply(file *lint.File, arguments lint.Arguments) [ return true } - if len(v.Elts) > 0 { - // not an empty map + isEmptyMap := len(v.Elts) > 0 + if isEmptyMap { return true } diff --git a/rule/enforce-slice-style.go b/rule/enforce-slice-style.go index 60d8ac066..df72c8cef 100644 --- a/rule/enforce-slice-style.go +++ b/rule/enforce-slice-style.go @@ -101,8 +101,8 @@ func (r *EnforceSliceStyleRule) Apply(file *lint.File, arguments lint.Arguments) return true } - if len(v.Elts) > 0 { - // not an empty slice + isNotEmptySlice := len(v.Elts) > 0 + if isNotEmptySlice { return true } @@ -132,8 +132,8 @@ func (r *EnforceSliceStyleRule) Apply(file *lint.File, arguments lint.Arguments) return true } - if len(v.Args) < 2 { - // skip invalid make declarations + isInvalidMakeDeclaration := len(v.Args) < 2 + if isInvalidMakeDeclaration { return true } @@ -148,8 +148,8 @@ func (r *EnforceSliceStyleRule) Apply(file *lint.File, arguments lint.Arguments) return true } - if arg.Value != "0" { - // skip slice with non-zero size + isSliceSizeNotZero := arg.Value != "0" + if isSliceSizeNotZero { return true } @@ -160,8 +160,8 @@ func (r *EnforceSliceStyleRule) Apply(file *lint.File, arguments lint.Arguments) return true } - if arg.Value != "0" { - // skip non-zero capacity slice + isNonZeroCapacitySlice := arg.Value != "0" + if isNonZeroCapacitySlice { return true } } diff --git a/rule/file-header.go b/rule/file-header.go index a7d69ff2b..0dcb57746 100644 --- a/rule/file-header.go +++ b/rule/file-header.go @@ -22,16 +22,18 @@ var ( func (r *FileHeaderRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.header == "" { - if len(arguments) < 1 { - return - } + if r.header != "" { + return // already configured + } - var ok bool - r.header, ok = arguments[0].(string) - if !ok { - panic(fmt.Sprintf("invalid argument for \"file-header\" rule: argument should be a string, got %T", arguments[0])) - } + if len(arguments) < 1 { + return + } + + var ok bool + r.header, ok = arguments[0].(string) + if !ok { + panic(fmt.Sprintf("invalid argument for \"file-header\" rule: argument should be a string, got %T", arguments[0])) } } diff --git a/rule/function-length.go b/rule/function-length.go index fd65884e9..30402313d 100644 --- a/rule/function-length.go +++ b/rule/function-length.go @@ -20,12 +20,14 @@ type FunctionLength struct { func (r *FunctionLength) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if !r.configured { - maxStmt, maxLines := r.parseArguments(arguments) - r.maxStmt = int(maxStmt) - r.maxLines = int(maxLines) - r.configured = true + if r.configured { + return } + + r.configured = true + maxStmt, maxLines := r.parseArguments(arguments) + r.maxStmt = int(maxStmt) + r.maxLines = int(maxLines) } // Apply applies the rule to given file. @@ -61,8 +63,9 @@ func (*FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxLin return defaultFuncStmtsLimit, defaultFuncLinesLimit } - if len(arguments) != 2 { - panic(fmt.Sprintf(`invalid configuration for "function-length" rule, expected 2 arguments but got %d`, len(arguments))) + const minArguments = 2 + if len(arguments) != minArguments { + panic(fmt.Sprintf(`invalid configuration for "function-length" rule, expected %d arguments but got %d`, minArguments, len(arguments))) } maxStmt, maxStmtOk := arguments[0].(int64) @@ -98,7 +101,8 @@ func (w lintFuncLength) Visit(n ast.Node) ast.Visitor { } body := node.Body - if body == nil || len(node.Body.List) == 0 { + emptyBody := body == nil || len(node.Body.List) == 0 + if emptyBody { return nil } diff --git a/rule/function-result-limit.go b/rule/function-result-limit.go index c125d0570..23474b5ee 100644 --- a/rule/function-result-limit.go +++ b/rule/function-result-limit.go @@ -19,20 +19,24 @@ const defaultResultsLimit = 3 func (r *FunctionResultsLimitRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.max == 0 { - if len(arguments) < 1 { - r.max = defaultResultsLimit - return - } - maxResults, ok := arguments[0].(int64) // Alt. non panicking version - if !ok { - panic(fmt.Sprintf(`invalid value passed as return results number to the "function-result-limit" rule; need int64 but got %T`, arguments[0])) - } - if maxResults < 0 { - panic(`the value passed as return results number to the "function-result-limit" rule cannot be negative`) - } - r.max = int(maxResults) + if r.max != 0 { + return // already configured } + + if len(arguments) < 1 { + r.max = defaultResultsLimit + return + } + + maxResults, ok := arguments[0].(int64) // Alt. non panicking version + if !ok { + panic(fmt.Sprintf(`invalid value passed as return results number to the "function-result-limit" rule; need int64 but got %T`, arguments[0])) + } + if maxResults < 0 { + panic(`the value passed as return results number to the "function-result-limit" rule cannot be negative`) + } + + r.max = int(maxResults) } // Apply applies the rule to given file. @@ -67,7 +71,8 @@ func (w lintFunctionResultsNum) Visit(n ast.Node) ast.Visitor { node, ok := n.(*ast.FuncDecl) if ok { num := 0 - if node.Type.Results != nil { + hasResults := node.Type.Results != nil + if hasResults { num = node.Type.Results.NumFields() } if num > w.max { @@ -76,8 +81,10 @@ func (w lintFunctionResultsNum) Visit(n ast.Node) ast.Visitor { Failure: fmt.Sprintf("maximum number of return results per function exceeded; max %d but got %d", w.max, num), Node: node.Type, }) - return w } + + return nil // skip visiting function's body } + return w } diff --git a/rule/get-return.go b/rule/get-return.go index 600a40fac..06323a087 100644 --- a/rule/get-return.go +++ b/rule/get-return.go @@ -33,15 +33,25 @@ type lintReturnRule struct { onFailure func(lint.Failure) } +const getterPrefix = "GET" + +var lenGetterPrefix = len(getterPrefix) + func isGetter(name string) bool { - if strings.HasPrefix(strings.ToUpper(name), "GET") { - if len(name) > 3 { - c := name[3] - return !(c >= 'a' && c <= 'z') - } + nameHasGetterPrefix := strings.HasPrefix(strings.ToUpper(name), getterPrefix) + if !nameHasGetterPrefix { + return false } - return false + isJustGet := len(name) == lenGetterPrefix + if isJustGet { + return false + } + + c := name[lenGetterPrefix] + lowerCaseAfterGetterPrefix := c >= 'a' && c <= 'z' + + return !lowerCaseAfterGetterPrefix } func hasResults(rs *ast.FieldList) bool { diff --git a/rule/identical-branches.go b/rule/identical-branches.go index 9222c8a9c..c6008925f 100644 --- a/rule/identical-branches.go +++ b/rule/identical-branches.go @@ -39,9 +39,11 @@ func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor { return w } - if n.Else == nil { + noElseBranch := n.Else == nil + if noElseBranch { return w } + branches := []*ast.BlockStmt{n.Body} elseBranch, ok := n.Else.(*ast.BlockStmt) @@ -59,14 +61,15 @@ func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor { func (lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool { if len(branches) < 2 { - return false + return false // only one branch to compare thus we return } - ref := gofmt(branches[0]) - refSize := len(branches[0].List) + referenceBranch := gofmt(branches[0]) + referenceBranchSize := len(branches[0].List) for i := 1; i < len(branches); i++ { - currentSize := len(branches[i].List) - if currentSize != refSize || gofmt(branches[i]) != ref { + currentBranch := branches[i] + currentBranchSize := len(currentBranch.List) + if currentBranchSize != referenceBranchSize || gofmt(currentBranch) != referenceBranch { return false } } diff --git a/rule/line-length-limit.go b/rule/line-length-limit.go index 6e55cbb38..1e91646b9 100644 --- a/rule/line-length-limit.go +++ b/rule/line-length-limit.go @@ -23,19 +23,22 @@ const defaultLineLengthLimit = 80 func (r *LineLengthLimitRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.max == 0 { - if len(arguments) < 1 { - r.max = defaultLineLengthLimit - return - } + if r.max != 0 { + return // already configured + } - maxLength, ok := arguments[0].(int64) // Alt. non panicking version - if !ok || maxLength < 0 { - panic(`invalid value passed as argument number to the "line-length-limit" rule`) - } + if len(arguments) < 1 { + r.max = defaultLineLengthLimit + return + } - r.max = int(maxLength) + maxLength, ok := arguments[0].(int64) // Alt. non panicking version + if !ok || maxLength < 0 { + panic(`invalid value passed as argument number to the "line-length-limit" rule`) } + + r.max = int(maxLength) + } // Apply applies the rule to given file. diff --git a/rule/max-control-nesting.go b/rule/max-control-nesting.go index e15d42e0b..5dbb1eefa 100644 --- a/rule/max-control-nesting.go +++ b/rule/max-control-nesting.go @@ -110,7 +110,7 @@ func (r *MaxControlNestingRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() if !(r.max < 1) { - return // max already set + return // max already configured } if len(arguments) < 1 { diff --git a/rule/max-public-structs.go b/rule/max-public-structs.go index b06e51f1b..ea93402a6 100644 --- a/rule/max-public-structs.go +++ b/rule/max-public-structs.go @@ -19,20 +19,23 @@ const defaultMaxPublicStructs = 5 func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.max < 1 { - if len(arguments) < 1 { - r.max = defaultMaxPublicStructs - return - } + if r.max == 0 { + return // already configured + } + + if len(arguments) < 1 { + r.max = defaultMaxPublicStructs + return + } - checkNumberOfArguments(1, arguments, r.Name()) + checkNumberOfArguments(1, arguments, r.Name()) - maxStructs, ok := arguments[0].(int64) // Alt. non panicking version - if !ok { - panic(`invalid value passed as argument number to the "max-public-structs" rule`) - } - r.max = maxStructs + maxStructs, ok := arguments[0].(int64) // Alt. non panicking version + if !ok { + panic(`invalid value passed as argument number to the "max-public-structs" rule`) } + r.max = maxStructs + } // Apply applies the rule to given file. diff --git a/rule/struct-tag.go b/rule/struct-tag.go index f6ee47a73..ec3f0c7cf 100644 --- a/rule/struct-tag.go +++ b/rule/struct-tag.go @@ -20,23 +20,27 @@ type StructTagRule struct { func (r *StructTagRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if r.userDefined == nil && len(arguments) > 0 { - checkNumberOfArguments(1, arguments, r.Name()) - r.userDefined = make(map[string][]string, len(arguments)) - for _, arg := range arguments { - item, ok := arg.(string) - if !ok { - panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string, got %v (of type %T)", r.Name(), arg, arg)) - } - parts := strings.Split(item, ",") - if len(parts) < 2 { - panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string of the form key[,option]+, got %s", r.Name(), item)) - } - key := strings.TrimSpace(parts[0]) - for i := 1; i < len(parts); i++ { - option := strings.TrimSpace(parts[i]) - r.userDefined[key] = append(r.userDefined[key], option) - } + + mustConfigure := r.userDefined == nil && len(arguments) > 0 + if !mustConfigure { + return + } + + checkNumberOfArguments(1, arguments, r.Name()) + r.userDefined = make(map[string][]string, len(arguments)) + for _, arg := range arguments { + item, ok := arg.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string, got %v (of type %T)", r.Name(), arg, arg)) + } + parts := strings.Split(item, ",") + if len(parts) < 2 { + panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string of the form key[,option]+, got %s", r.Name(), item)) + } + key := strings.TrimSpace(parts[0]) + for i := 1; i < len(parts); i++ { + option := strings.TrimSpace(parts[i]) + r.userDefined[key] = append(r.userDefined[key], option) } } } @@ -75,11 +79,13 @@ type lintStructTagRule struct { func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { switch n := node.(type) { case *ast.StructType: - if n.Fields == nil || n.Fields.NumFields() < 1 { + isEmptyStruct := n.Fields == nil || n.Fields.NumFields() < 1 + if isEmptyStruct { return nil // skip empty structs } - w.usedTagNbr = map[int]bool{} // init - w.usedTagName = map[string]bool{} // init + + w.usedTagNbr = map[int]bool{} + w.usedTagName = map[string]bool{} for _, f := range n.Fields.List { if f.Tag != nil { w.checkTaggedField(f) diff --git a/rule/time-equal.go b/rule/time-equal.go index 3b85e18a8..a4fab88b3 100644 --- a/rule/time-equal.go +++ b/rule/time-equal.go @@ -50,26 +50,23 @@ func (l *lintTimeEqual) Visit(node ast.Node) ast.Visitor { return l } - xtyp := l.file.Pkg.TypeOf(expr.X) - ytyp := l.file.Pkg.TypeOf(expr.Y) - - if !isNamedType(xtyp, "time", "Time") || !isNamedType(ytyp, "time", "Time") { + typeOfX := l.file.Pkg.TypeOf(expr.X) + typeOfY := l.file.Pkg.TypeOf(expr.Y) + bothAreOfTimeType := isNamedType(typeOfX, "time", "Time") && isNamedType(typeOfY, "time", "Time") + if !bothAreOfTimeType { return l } - var failure string - switch expr.Op { - case token.EQL: - failure = fmt.Sprintf("use %s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op) - case token.NEQ: - failure = fmt.Sprintf("use !%s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op) + negateStr := "" + if token.NEQ == expr.Op { + negateStr = "!" } l.onFailure(lint.Failure{ Category: "time", Confidence: 1, Node: node, - Failure: failure, + Failure: fmt.Sprintf("use %s%s.Equal(%s) instead of %q operator", negateStr, gofmt(expr.X), gofmt(expr.Y), expr.Op), }) return l diff --git a/rule/time-naming.go b/rule/time-naming.go index cea452e61..5bccf8a7a 100644 --- a/rule/time-naming.go +++ b/rule/time-naming.go @@ -90,6 +90,7 @@ func isNamedType(typ types.Type, importPath, name string) bool { if !ok { return false } - tn := n.Obj() - return tn != nil && tn.Pkg() != nil && tn.Pkg().Path() == importPath && tn.Name() == name + + typeName := n.Obj() + return typeName != nil && typeName.Pkg() != nil && typeName.Pkg().Path() == importPath && typeName.Name() == name } diff --git a/rule/unchecked-type-assertion.go b/rule/unchecked-type-assertion.go index ffc74f5fe..eea344060 100644 --- a/rule/unchecked-type-assertion.go +++ b/rule/unchecked-type-assertion.go @@ -54,7 +54,7 @@ func (u *UncheckedTypeAssertionRule) Apply(file *lint.File, args lint.Arguments) var failures []lint.Failure - walker := &lintUnchekedTypeAssertion{ + walker := &lintUncheckedTypeAssertion{ onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, @@ -71,7 +71,7 @@ func (*UncheckedTypeAssertionRule) Name() string { return "unchecked-type-assertion" } -type lintUnchekedTypeAssertion struct { +type lintUncheckedTypeAssertion struct { onFailure func(lint.Failure) acceptIgnoredTypeAssertionResult bool } @@ -89,14 +89,14 @@ func isTypeSwitch(e *ast.TypeAssertExpr) bool { return e.Type == nil } -func (w *lintUnchekedTypeAssertion) requireNoTypeAssert(expr ast.Expr) { +func (w *lintUncheckedTypeAssertion) requireNoTypeAssert(expr ast.Expr) { e, ok := expr.(*ast.TypeAssertExpr) if ok && !isTypeSwitch(e) { w.addFailure(e, ruleUTAMessagePanic) } } -func (w *lintUnchekedTypeAssertion) handleIfStmt(n *ast.IfStmt) { +func (w *lintUncheckedTypeAssertion) handleIfStmt(n *ast.IfStmt) { ifCondition, ok := n.Cond.(*ast.BinaryExpr) if ok { w.requireNoTypeAssert(ifCondition.X) @@ -104,7 +104,7 @@ func (w *lintUnchekedTypeAssertion) handleIfStmt(n *ast.IfStmt) { } } -func (w *lintUnchekedTypeAssertion) requireBinaryExpressionWithoutTypeAssertion(expr ast.Expr) { +func (w *lintUncheckedTypeAssertion) requireBinaryExpressionWithoutTypeAssertion(expr ast.Expr) { binaryExpr, ok := expr.(*ast.BinaryExpr) if ok { w.requireNoTypeAssert(binaryExpr.X) @@ -112,19 +112,19 @@ func (w *lintUnchekedTypeAssertion) requireBinaryExpressionWithoutTypeAssertion( } } -func (w *lintUnchekedTypeAssertion) handleCaseClause(n *ast.CaseClause) { +func (w *lintUncheckedTypeAssertion) handleCaseClause(n *ast.CaseClause) { for _, expr := range n.List { w.requireNoTypeAssert(expr) w.requireBinaryExpressionWithoutTypeAssertion(expr) } } -func (w *lintUnchekedTypeAssertion) handleSwitch(n *ast.SwitchStmt) { +func (w *lintUncheckedTypeAssertion) handleSwitch(n *ast.SwitchStmt) { w.requireNoTypeAssert(n.Tag) w.requireBinaryExpressionWithoutTypeAssertion(n.Tag) } -func (w *lintUnchekedTypeAssertion) handleAssignment(n *ast.AssignStmt) { +func (w *lintUncheckedTypeAssertion) handleAssignment(n *ast.AssignStmt) { if len(n.Rhs) == 0 { return } @@ -148,21 +148,21 @@ func (w *lintUnchekedTypeAssertion) handleAssignment(n *ast.AssignStmt) { } // handles "return foo(.*bar)" - one of them is enough to fail as golang does not forward the type cast tuples in return statements -func (w *lintUnchekedTypeAssertion) handleReturn(n *ast.ReturnStmt) { +func (w *lintUncheckedTypeAssertion) handleReturn(n *ast.ReturnStmt) { for _, r := range n.Results { w.requireNoTypeAssert(r) } } -func (w *lintUnchekedTypeAssertion) handleRange(n *ast.RangeStmt) { +func (w *lintUncheckedTypeAssertion) handleRange(n *ast.RangeStmt) { w.requireNoTypeAssert(n.X) } -func (w *lintUnchekedTypeAssertion) handleChannelSend(n *ast.SendStmt) { +func (w *lintUncheckedTypeAssertion) handleChannelSend(n *ast.SendStmt) { w.requireNoTypeAssert(n.Value) } -func (w *lintUnchekedTypeAssertion) Visit(node ast.Node) ast.Visitor { +func (w *lintUncheckedTypeAssertion) Visit(node ast.Node) ast.Visitor { switch n := node.(type) { case *ast.RangeStmt: w.handleRange(n) @@ -183,7 +183,7 @@ func (w *lintUnchekedTypeAssertion) Visit(node ast.Node) ast.Visitor { return w } -func (w *lintUnchekedTypeAssertion) addFailure(n *ast.TypeAssertExpr, why string) { +func (w *lintUncheckedTypeAssertion) addFailure(n *ast.TypeAssertExpr, why string) { s := fmt.Sprintf("type cast result is unchecked in %v - %s", gofmt(n), why) w.onFailure(lint.Failure{ Category: "bad practice", diff --git a/rule/unconditional-recursion.go b/rule/unconditional-recursion.go index 9ac2648cd..d806b6757 100644 --- a/rule/unconditional-recursion.go +++ b/rule/unconditional-recursion.go @@ -185,9 +185,10 @@ func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool { return false } - fn := se.Sel.Name - pkg := id.Name - if exitFunctions[pkg] != nil && exitFunctions[pkg][fn] { // it's a call to an exit function + functionName := se.Sel.Name + pkgName := id.Name + isCallToExitFunction := exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName] + if isCallToExitFunction { return true } } diff --git a/rule/unhandled-error.go b/rule/unhandled-error.go index ce6fa3864..95ba56180 100644 --- a/rule/unhandled-error.go +++ b/rule/unhandled-error.go @@ -19,27 +19,30 @@ type UnhandledErrorRule struct { func (r *UnhandledErrorRule) configure(arguments lint.Arguments) { r.Lock() - if r.ignoreList == nil { - for _, arg := range arguments { - argStr, ok := arg.(string) - if !ok { - panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg)) - } + defer r.Unlock() - argStr = strings.Trim(argStr, " ") - if argStr == "" { - panic("Invalid argument to the unhandled-error rule, expected regular expression must not be empty.") - } + if r.ignoreList != nil { + return // already configured + } - exp, err := regexp.Compile(argStr) - if err != nil { - panic(fmt.Sprintf("Invalid argument to the unhandled-error rule: regexp %q does not compile: %v", argStr, err)) - } + for _, arg := range arguments { + argStr, ok := arg.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg)) + } - r.ignoreList = append(r.ignoreList, exp) + argStr = strings.Trim(argStr, " ") + if argStr == "" { + panic("Invalid argument to the unhandled-error rule, expected regular expression must not be empty.") } + + exp, err := regexp.Compile(argStr) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the unhandled-error rule: regexp %q does not compile: %v", argStr, err)) + } + + r.ignoreList = append(r.ignoreList, exp) } - r.Unlock() } // Apply applies the rule to given file. @@ -130,9 +133,9 @@ func (w *lintUnhandledErrors) funcName(call *ast.CallExpr) string { } name := fn.FullName() - name = strings.Replace(name, "(", "", -1) - name = strings.Replace(name, ")", "", -1) - name = strings.Replace(name, "*", "", -1) + name = strings.ReplaceAll(name, "(", "") + name = strings.ReplaceAll(name, ")", "") + name = strings.ReplaceAll(name, "*", "") return name } diff --git a/rule/var-declarations.go b/rule/var-declarations.go index a15ff1eb4..3f9d7068a 100644 --- a/rule/var-declarations.go +++ b/rule/var-declarations.go @@ -46,13 +46,15 @@ type lintVarDeclarations struct { func (w *lintVarDeclarations) Visit(node ast.Node) ast.Visitor { switch v := node.(type) { case *ast.GenDecl: - if v.Tok != token.CONST && v.Tok != token.VAR { + isVarOrConstDeclaration := v.Tok == token.CONST || v.Tok == token.VAR + if !isVarOrConstDeclaration { return nil } w.lastGen = v return w case *ast.ValueSpec: - if w.lastGen.Tok == token.CONST { + isConstDeclaration := w.lastGen.Tok == token.CONST + if isConstDeclaration { return nil } if len(v.Names) > 1 || v.Type == nil || len(v.Values) == 0 { @@ -64,14 +66,14 @@ func (w *lintVarDeclarations) Visit(node ast.Node) ast.Visitor { if isIdent(v.Names[0], "_") { return nil } - // If the RHS is a zero value, suggest dropping it. - zero := false + // If the RHS is a isZero value, suggest dropping it. + isZero := false if lit, ok := rhs.(*ast.BasicLit); ok { - zero = zeroLiteral[lit.Value] + isZero = zeroLiteral[lit.Value] } else if isIdent(rhs, "nil") { - zero = true + isZero = true } - if zero { + if isZero { w.onFailure(lint.Failure{ Confidence: 0.9, Node: rhs, diff --git a/rule/var-naming.go b/rule/var-naming.go index e91c22dc2..e93159b20 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -19,9 +19,9 @@ var upperCaseConstRE = regexp.MustCompile(`^_?[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) // VarNamingRule lints given else constructs. type VarNamingRule struct { configured bool - allowlist []string - blocklist []string - upperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants + allowList []string + blockList []string + allowUpperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants skipPackageNameChecks bool sync.Mutex } @@ -35,11 +35,11 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) { r.configured = true if len(arguments) >= 1 { - r.allowlist = getList(arguments[0], "allowlist") + r.allowList = getList(arguments[0], "allowlist") } if len(arguments) >= 2 { - r.blocklist = getList(arguments[1], "blocklist") + r.blockList = getList(arguments[1], "blocklist") } if len(arguments) >= 3 { @@ -56,7 +56,7 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) { if !ok { panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0])) } - r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" + r.allowUpperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" r.skipPackageNameChecks = fmt.Sprint(args["skipPackageNameChecks"]) == "true" } } @@ -93,12 +93,12 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. walker := lintNames{ file: file, fileAst: fileAst, - allowlist: r.allowlist, - blocklist: r.blocklist, + allowList: r.allowList, + blockList: r.blockList, onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, - upperCaseConst: r.upperCaseConst, + upperCaseConst: r.allowUpperCaseConst, } if !r.skipPackageNameChecks { @@ -151,7 +151,7 @@ func (w *lintNames) check(id *ast.Ident, thing string) { return } - should := lint.Name(id.Name, w.allowlist, w.blocklist) + should := lint.Name(id.Name, w.allowList, w.blockList) if id.Name == should { return } @@ -177,8 +177,8 @@ type lintNames struct { file *lint.File fileAst *ast.File onFailure func(lint.Failure) - allowlist []string - blocklist []string + allowList []string + blockList []string upperCaseConst bool } diff --git a/rule/waitgroup-by-value.go b/rule/waitgroup-by-value.go index 98644f41c..a2d304ae5 100644 --- a/rule/waitgroup-by-value.go +++ b/rule/waitgroup-by-value.go @@ -51,7 +51,7 @@ func (w lintWaitGroupByValueRule) Visit(node ast.Node) ast.Visitor { }) } - return nil + return nil // skip visiting function body } func (lintWaitGroupByValueRule) isWaitGroup(ft ast.Expr) bool {