chore: cleanup code in rules (#1197)

rule/bool_literal_in_expr.go

@@ -88,5 +88,5 @@ func isExprABooleanLit(n ast.Node) (lexeme string, ok bool) {
 		return "", false
 	}
 
-	return oper.Name, (oper.Name == "true" || oper.Name == "false")
+	return oper.Name, oper.Name == "true" || oper.Name == "false"
 }

rule/cognitive_complexity.go

@@ -89,8 +89,8 @@ type cognitiveComplexityVisitor struct {
 }
 
 // subTreeComplexity calculates the cognitive complexity of an AST-subtree.
-func (v cognitiveComplexityVisitor) subTreeComplexity(n ast.Node) int {
+func (v *cognitiveComplexityVisitor) subTreeComplexity(n ast.Node) int {
-	ast.Walk(&v, n)
+	ast.Walk(v, n)
 	return v.complexity
 }
 
@@ -122,7 +122,7 @@ func (v *cognitiveComplexityVisitor) Visit(n ast.Node) ast.Visitor {
 		return nil
 	case *ast.BinaryExpr:
 		v.complexity += v.binExpComplexity(n)
-		return nil // skip visiting binexp sub-tree (already visited by binExpComplexity)
+		return nil // skip visiting binexp subtree (already visited by binExpComplexity)
 	case *ast.BranchStmt:
 		if n.Label != nil {
 			v.complexity++
@@ -156,7 +156,7 @@ func (v *cognitiveComplexityVisitor) walk(complexityIncrement int, targets ...as
 	v.nestingLevel = nesting
 }
 
-func (cognitiveComplexityVisitor) binExpComplexity(n *ast.BinaryExpr) int {
+func (*cognitiveComplexityVisitor) binExpComplexity(n *ast.BinaryExpr) int {
 	calculator := binExprComplexityCalculator{opsStack: []token.Token{}}
 
 	astutil.Apply(n, calculator.pre, calculator.post)

rule/comment_spacings.go

@@ -7,7 +7,7 @@ import (
 	"github.com/mgechev/revive/lint"
 )
 
-// CommentSpacingsRule check the whether there is a space between
+// CommentSpacingsRule check whether there is a space between
 // the comment symbol( // ) and the start of the comment text
 type CommentSpacingsRule struct {
 	allowList []string

rule/constant_logical_expr.go

@@ -91,7 +91,7 @@ func (*lintConstantLogicalExpr) isInequalityOperator(t token.Token) bool {
 	return false
 }
 
-func (w lintConstantLogicalExpr) newFailure(node ast.Node, msg string) {
+func (w *lintConstantLogicalExpr) newFailure(node ast.Node, msg string) {
 	w.onFailure(lint.Failure{
 		Confidence: 1,
 		Node:       node,

rule/datarace.go

@@ -22,6 +22,7 @@ func (r *DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 
 		funcResults := funcDecl.Type.Results
 
+		// TODO: ast.Object is deprecated
 		returnIDs := map[*ast.Object]struct{}{}
 		if funcResults != nil {
 			returnIDs = r.extractReturnIDs(funcResults.List)
@@ -34,7 +35,7 @@ func (r *DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 		fl := &lintFunctionForDataRaces{
 			onFailure: onFailure,
 			returnIDs: returnIDs,
-			rangeIDs:  map[*ast.Object]struct{}{},
+			rangeIDs:  map[*ast.Object]struct{}{}, // TODO: ast.Object is deprecated
 			go122for:  isGo122,
 		}
 
@@ -49,6 +50,7 @@ func (*DataRaceRule) Name() string {
 	return "datarace"
 }
 
+// TODO: ast.Object is deprecated
 func (*DataRaceRule) extractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} {
 	r := map[*ast.Object]struct{}{}
 	for _, f := range fields {
@@ -63,9 +65,10 @@ func (*DataRaceRule) extractReturnIDs(fields []*ast.Field) map[*ast.Object]struc
 type lintFunctionForDataRaces struct {
 	_         struct{}
 	onFailure func(failure lint.Failure)
-	returnIDs map[*ast.Object]struct{}
+	returnIDs map[*ast.Object]struct{} // TODO: ast.Object is deprecated
-	rangeIDs  map[*ast.Object]struct{}
+	rangeIDs  map[*ast.Object]struct{} // TODO: ast.Object is deprecated
-	go122for  bool
+
+	go122for bool
 }
 
 func (w lintFunctionForDataRaces) Visit(node ast.Node) ast.Visitor {

rule/identical_branches.go

@@ -59,7 +59,7 @@ func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor {
 	return w
 }
 
-func (lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool {
+func (*lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool {
 	if len(branches) < 2 {
 		return false // only one branch to compare thus we return
 	}
@@ -77,7 +77,7 @@ func (lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool {
 	return true
 }
 
-func (w lintIdenticalBranches) newFailure(node ast.Node, msg string) {
+func (w *lintIdenticalBranches) newFailure(node ast.Node, msg string) {
 	w.onFailure(lint.Failure{
 		Confidence: 1,
 		Node:       node,

rule/import_shadowing.go

@@ -28,7 +28,7 @@ func (*ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail
 		onFailure: func(failure lint.Failure) {
 			failures = append(failures, failure)
 		},
-		alreadySeen: map[*ast.Object]struct{}{},
+		alreadySeen: map[*ast.Object]struct{}{}, // TODO: ast.Object is deprecated
 		skipIdents:  map[*ast.Ident]struct{}{},
 	}
 
@@ -62,7 +62,7 @@ type importShadowing struct {
 	packageNameIdent *ast.Ident
 	importNames      map[string]struct{}
 	onFailure        func(lint.Failure)
-	alreadySeen      map[*ast.Object]struct{}
+	alreadySeen      map[*ast.Object]struct{} // TODO: ast.Object is deprecated
 	skipIdents       map[*ast.Ident]struct{}
 }
 

rule/range_val_address.go

@@ -70,7 +70,7 @@ func (w rangeValAddress) Visit(node ast.Node) ast.Visitor {
 
 type rangeBodyVisitor struct {
 	valueIsStarExpr bool
-	valueID         *ast.Object
+	valueID         *ast.Object // TODO: ast.Object is deprecated
 	onFailure       func(lint.Failure)
 }
 
@@ -140,7 +140,7 @@ func (bw rangeBodyVisitor) isAccessingRangeValueAddress(exp ast.Expr) bool {
 	v, ok := u.X.(*ast.Ident)
 	if !ok {
 		var s *ast.SelectorExpr
-		s, ok = u.X.(*ast.SelectorExpr)
+		s, ok = u.X.(*ast.SelectorExpr) // TODO: possible BUG: if it's `=` and not `:=`, it means that in the last return `ok` is always true
 		if !ok {
 			return false
 		}
@@ -154,7 +154,7 @@ func (bw rangeBodyVisitor) isAccessingRangeValueAddress(exp ast.Expr) bool {
 		}
 	}
 
-	return ok && v.Obj == bw.valueID
+	return ok && v.Obj == bw.valueID // TODO: ok is always true due to the previous TODO remark
 }
 
 func (bw rangeBodyVisitor) newFailure(node ast.Node) lint.Failure {

rule/redefines_builtin_id.go

@@ -198,7 +198,7 @@ func (w *lintRedefinesBuiltinID) Visit(node ast.Node) ast.Visitor {
 	return w
 }
 
-func (w lintRedefinesBuiltinID) addFailure(node ast.Node, msg string) {
+func (w *lintRedefinesBuiltinID) addFailure(node ast.Node, msg string) {
 	w.onFailure(lint.Failure{
 		Confidence: 1,
 		Node:       node,

rule/string_format.go

@@ -22,7 +22,7 @@ func (*StringFormatRule) Apply(file *lint.File, arguments lint.Arguments) []lint
 		failures = append(failures, failure)
 	}
 
-	w := lintStringFormatRule{onFailure: onFailure}
+	w := &lintStringFormatRule{onFailure: onFailure}
 	err := w.parseArguments(arguments)
 	if err != nil {
 		return newInternalFailureError(err)
@@ -39,7 +39,7 @@ func (*StringFormatRule) Name() string {
 }
 
 // ParseArgumentsTest is a public wrapper around w.parseArguments used for testing. Returns the error message provided to panic, or nil if no error was encountered
-func (StringFormatRule) ParseArgumentsTest(arguments lint.Arguments) *string {
+func (*StringFormatRule) ParseArgumentsTest(arguments lint.Arguments) *string {
 	w := lintStringFormatRule{}
 	c := make(chan any)
 	// Parse the arguments in a goroutine, defer a recover() call, return the error encountered (or nil if there was no error)
@@ -101,7 +101,7 @@ func (w *lintStringFormatRule) parseArguments(arguments lint.Arguments) error {
 	return nil
 }
 
-func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scopes stringFormatSubruleScopes, regex *regexp.Regexp, negated bool, errorMessage string, err error) {
+func (w *lintStringFormatRule) parseArgument(argument any, ruleNum int) (scopes stringFormatSubruleScopes, regex *regexp.Regexp, negated bool, errorMessage string, err error) {
 	g, ok := argument.([]any) // Cast to generic slice first
 	if !ok {
 		return stringFormatSubruleScopes{}, regex, false, "", w.configError("argument is not a slice", ruleNum, 0)
@@ -179,21 +179,21 @@ func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scopes s
 }
 
 // Report an invalid config, this is specifically the user's fault
-func (lintStringFormatRule) configError(msg string, ruleNum, option int) error {
+func (*lintStringFormatRule) configError(msg string, ruleNum, option int) error {
 	return fmt.Errorf("invalid configuration for string-format: %s [argument %d, option %d]", msg, ruleNum, option)
 }
 
 // Report a general config parsing failure, this may be the user's fault, but it isn't known for certain
-func (lintStringFormatRule) parseError(msg string, ruleNum, option int) error {
+func (*lintStringFormatRule) parseError(msg string, ruleNum, option int) error {
 	return fmt.Errorf("failed to parse configuration for string-format: %s [argument %d, option %d]", msg, ruleNum, option)
 }
 
 // Report a general scope config parsing failure, this may be the user's fault, but it isn't known for certain
-func (lintStringFormatRule) parseScopeError(msg string, ruleNum, option, scopeNum int) error {
+func (*lintStringFormatRule) parseScopeError(msg string, ruleNum, option, scopeNum int) error {
 	return fmt.Errorf("failed to parse configuration for string-format: %s [argument %d, option %d, scope index %d]", msg, ruleNum, option, scopeNum)
 }
 
-func (w lintStringFormatRule) Visit(node ast.Node) ast.Visitor {
+func (w *lintStringFormatRule) Visit(node ast.Node) ast.Visitor {
 	// First, check if node is a call expression
 	call, ok := node.(*ast.CallExpr)
 	if !ok {
@@ -218,7 +218,7 @@ func (w lintStringFormatRule) Visit(node ast.Node) ast.Visitor {
 }
 
 // Return the name of a call expression in the form of package.Func or Func
-func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok bool) {
+func (*lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok bool) {
 	if ident, ok := call.Fun.(*ast.Ident); ok {
 		// Local function call
 		return ident.Name, true
@@ -278,6 +278,12 @@ func (r *stringFormatSubrule) apply(call *ast.CallExpr, scope *stringFormatSubru
 			return
 		}
 	}
+
+	// extra safety check
+	if lit == nil {
+		return
+	}
+
 	// Unquote the string literal before linting
 	unquoted := lit.Value[1 : len(lit.Value)-1]
 	if r.stringIsOK(unquoted) {

rule/unconditional_recursion.go

@@ -17,7 +17,7 @@ func (*UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) []li
 		failures = append(failures, failure)
 	}
 
-	w := lintUnconditionalRecursionRule{onFailure: onFailure}
+	w := &lintUnconditionalRecursionRule{onFailure: onFailure}
 	ast.Walk(w, file.AST)
 	return failures
 }
@@ -57,7 +57,7 @@ type lintUnconditionalRecursionRule struct {
 // If we find conditional control exits, it means the function is NOT unconditionally-recursive
 // If we find a recursive call before finding any conditional exit, a failure is generated
 // In resume: if we found a recursive call control-dependent from the entry point of the function then we raise a failure.
-func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
+func (w *lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
 	switch n := node.(type) {
 	case *ast.FuncDecl:
 		var rec *ast.Ident
@@ -152,7 +152,7 @@ func (w *lintUnconditionalRecursionRule) updateFuncStatus(node ast.Node) {
 	w.currentFunc.seenConditionalExit = w.hasControlExit(node)
 }
 
-func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
+func (*lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
 	// isExit returns true if the given node makes control exit the function
 	isExit := func(node ast.Node) bool {
 		switch n := node.(type) {

rule/unused_param.go

@@ -21,8 +21,8 @@ type UnusedParamRule struct {
 //
 // Configuration implements the [lint.ConfigurableRule] interface.
 func (r *UnusedParamRule) Configure(args lint.Arguments) error {
-	// while by default args is an array, i think it's good to provide structures inside it by default, not arrays or primitives
+	// while by default args is an array, it could be good to provide structures inside it by default, not arrays or primitives
-	// it's more compatible to JSON nature of configurations
+	// as it's more compatible to JSON nature of configurations
 	r.allowRegex = allowBlankIdentifierRegex
 	r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it as _"
 	if len(args) == 0 {
@@ -139,6 +139,7 @@ func (w lintUnusedParamRule) Visit(node ast.Node) ast.Visitor {
 	return w // full method body was inspected
 }
 
+// TODO: ast.Object is deprecated
 func retrieveNamedParams(params *ast.FieldList) map[*ast.Object]bool {
 	result := map[*ast.Object]bool{}
 	if params.List == nil {

rule/unused_receiver.go

@@ -19,8 +19,8 @@ type UnusedReceiverRule struct {
 //
 // Configuration implements the [lint.ConfigurableRule] interface.
 func (r *UnusedReceiverRule) Configure(args lint.Arguments) error {
-	// while by default args is an array, i think it's good to provide structures inside it by default, not arrays or primitives
+	// while by default args is an array, it could be good to provide structures inside it by default, not arrays or primitives
-	// it's more compatible to JSON nature of configurations
+	// as it's more compatible to JSON nature of configurations
 	r.allowRegex = allowBlankIdentifierRegex
 	r.failureMsg = "method receiver '%s' is not referenced in method's body, consider removing or renaming it as _"
 	if len(args) == 0 {

rule/utils.go

@@ -123,7 +123,7 @@ func isCallToExitFunction(pkgName, functionName string) bool {
 	return exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName]
 }
 
-// newInternalFailureError returns an slice of Failure with a single internal failure in it
+// newInternalFailureError returns a slice of Failure with a single internal failure in it
 func newInternalFailureError(e error) []lint.Failure {
 	return []lint.Failure{lint.NewInternalFailure(e.Error())}
 }

rule/waitgroup_by_value.go

@@ -38,7 +38,7 @@ func (w lintWaitGroupByValueRule) Visit(node ast.Node) ast.Visitor {
 		return w
 	}
 
-	// Check all function's parameters
+	// Check all function parameters
 	for _, field := range fd.Type.Params.List {
 		if !w.isWaitGroup(field.Type) {
 			continue