code cleanup (#1054)

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{

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
 }

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

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
 			}

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.

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

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

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
 }
 

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,

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.

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",
 			})
 		}

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
 			}
 

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