refactor: rule configuration and error management (#1185)

* refactor: avoid running rule once configuration failed

* refactor: remove deep-exit in lint/linter.go

Author: ccoVeille

config/config.go

@@ -149,6 +149,12 @@ func GetLintingRules(config *lint.Config, extraRules []lint.Rule) ([]lint.Rule,
 			continue // skip disabled rules
 		}
 
+		if r, ok := r.(lint.ConfigurableRule); ok {
+			if err := r.Configure(ruleConfig.Arguments); err != nil {
+				return nil, fmt.Errorf("cannot configure rule: %s", name)
+			}
+		}
+
 		lintingRules = append(lintingRules, r)
 	}
 

lint/linter.go

@@ -10,10 +10,10 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-	"sync"
 
 	goversion "github.com/hashicorp/go-version"
 	"golang.org/x/mod/modfile"
+	"golang.org/x/sync/errgroup"
 )
 
 // ReadFile defines an abstraction for reading files.
@@ -101,20 +101,23 @@ func (l *Linter) Lint(packages [][]string, ruleSet []Rule, config Config) (<-cha
 		perPkgVersions[n] = v
 	}
 
-	var wg sync.WaitGroup
-	wg.Add(len(packages))
+	var wg errgroup.Group
 	for n := range packages {
-		go func(pkg []string, gover *goversion.Version) {
+		wg.Go(func() error {
+			pkg := packages[n]
+			gover := perPkgVersions[n]
 			if err := l.lintPackage(pkg, gover, ruleSet, config, failures); err != nil {
-				fmt.Fprintln(os.Stderr, "error during linting: "+err.Error())
-				os.Exit(1)
+				return fmt.Errorf("error during linting: %w", err)
 			}
-			wg.Done()
-		}(packages[n], perPkgVersions[n])
+			return nil
+		})
 	}
 
 	go func() {
-		wg.Wait()
+		err := wg.Wait()
+		if err != nil {
+			failures <- NewInternalFailure(err.Error())
+		}
 		close(failures)
 	}()
 

lint/rule.go

@@ -17,6 +17,11 @@ type Rule interface {
 	Apply(*File, Arguments) []Failure
 }
 
+// ConfigurableRule defines an abstract configurable rule interface.
+type ConfigurableRule interface {
+	Configure(Arguments) error
+}
+
 // ToFailurePosition returns the failure position.
 func ToFailurePosition(start, end token.Pos, file *File) FailurePosition {
 	return FailurePosition{

rule/add_constant.go

@@ -7,7 +7,6 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -37,18 +36,10 @@ type AddConstantRule struct {
 	allowList       allowList
 	ignoreFunctions []*regexp.Regexp
 	strLitLimit     int
-
-	configureOnce sync.Once
 }
 
 // Apply applies the rule to given file.
-func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *AddConstantRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	onFailure := func(failure lint.Failure) {
@@ -206,7 +197,10 @@ func (w *lintAddConstantRule) isStructTag(n *ast.BasicLit) bool {
 	return ok
 }
 
-func (r *AddConstantRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *AddConstantRule) Configure(arguments lint.Arguments) error {
 	r.strLitLimit = defaultStrLitLimit
 	r.allowList = newAllowList()
 	if len(arguments) == 0 {

rule/argument_limit.go

@@ -4,21 +4,21 @@ import (
 	"errors"
 	"fmt"
 	"go/ast"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
 
 // ArgumentsLimitRule lints the number of arguments a function can receive.
 type ArgumentsLimitRule struct {
 	max int
-
-	configureOnce sync.Once
 }
 
 const defaultArgumentsLimit = 8
 
-func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *ArgumentsLimitRule) Configure(arguments lint.Arguments) error {
 	if len(arguments) < 1 {
 		r.max = defaultArgumentsLimit
 		return nil
@@ -33,14 +33,7 @@ func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply applies the rule to given file.
-func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *ArgumentsLimitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	for _, decl := range file.AST.Decls {

rule/banned_characters.go

@@ -4,21 +4,21 @@ import (
 	"fmt"
 	"go/ast"
 	"strings"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
 
 // BannedCharsRule checks if a file contains banned characters.
 type BannedCharsRule struct {
 	bannedCharList []string
-
-	configureOnce sync.Once
 }
 
 const bannedCharsRuleName = "banned-characters"
 
-func (r *BannedCharsRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *BannedCharsRule) Configure(arguments lint.Arguments) error {
 	if len(arguments) > 0 {
 		err := checkNumberOfArguments(1, arguments, bannedCharsRuleName)
 		if err != nil {
@@ -35,14 +35,7 @@ func (r *BannedCharsRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply applied the rule to the given file.
-func (r *BannedCharsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *BannedCharsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 	onFailure := func(failure lint.Failure) {
 		failures = append(failures, failure)

rule/cognitive_complexity.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 	"golang.org/x/tools/go/ast/astutil"
@@ -13,13 +12,14 @@ import (
 // CognitiveComplexityRule sets restriction for maximum cognitive complexity.
 type CognitiveComplexityRule struct {
 	maxComplexity int
-
-	configureOnce sync.Once
 }
 
 const defaultMaxCognitiveComplexity = 7
 
-func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *CognitiveComplexityRule) Configure(arguments lint.Arguments) error {
 	if len(arguments) < 1 {
 		r.maxComplexity = defaultMaxCognitiveComplexity
 		return nil
@@ -35,14 +35,7 @@ func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply applies the rule to given file.
-func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *CognitiveComplexityRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	linter := cognitiveComplexityLinter{

rule/comment_spacings.go

@@ -3,7 +3,6 @@ package rule
 import (
 	"fmt"
 	"strings"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -12,11 +11,12 @@ import (
 // the comment symbol( // ) and the start of the comment text
 type CommentSpacingsRule struct {
 	allowList []string
-
-	configureOnce sync.Once
 }
 
-func (r *CommentSpacingsRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *CommentSpacingsRule) Configure(arguments lint.Arguments) error {
 	r.allowList = []string{}
 	for _, arg := range arguments {
 		allow, ok := arg.(string) // Alt. non panicking version
@@ -29,14 +29,7 @@ func (r *CommentSpacingsRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply the rule.
-func (r *CommentSpacingsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *CommentSpacingsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	for _, cg := range file.AST.Comments {

rule/comments_density.go

@@ -4,21 +4,21 @@ import (
 	"fmt"
 	"go/ast"
 	"strings"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
 
 // CommentsDensityRule enforces a minimum comment / code relation.
 type CommentsDensityRule struct {
 	minimumCommentsDensity int64
-
-	configureOnce sync.Once
 }
 
 const defaultMinimumCommentsPercentage = 0
 
-func (r *CommentsDensityRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *CommentsDensityRule) Configure(arguments lint.Arguments) error {
 	if len(arguments) < 1 {
 		r.minimumCommentsDensity = defaultMinimumCommentsPercentage
 		return nil
@@ -33,14 +33,7 @@ func (r *CommentsDensityRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply applies the rule to given file.
-func (r *CommentsDensityRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-	
+func (r *CommentsDensityRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	commentsLines := countDocLines(file.AST.Comments)
 	statementsCount := countStatements(file.AST)
 	density := (float32(commentsLines) / float32(statementsCount+commentsLines)) * 100

rule/context_as_argument.go

@@ -4,27 +4,17 @@ import (
 	"fmt"
 	"go/ast"
 	"strings"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
 
 // ContextAsArgumentRule suggests that `context.Context` should be the first argument of a function.
 type ContextAsArgumentRule struct {
 	allowTypes map[string]struct{}
-
-	configureOnce sync.Once
 }
 
 // Apply applies the rule to given file.
-func (r *ContextAsArgumentRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *ContextAsArgumentRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 	for _, decl := range file.AST.Decls {
 		fn, ok := decl.(*ast.FuncDecl)
@@ -64,7 +54,10 @@ func (*ContextAsArgumentRule) Name() string {
 	return "context-as-argument"
 }
 
-func (r *ContextAsArgumentRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *ContextAsArgumentRule) Configure(arguments lint.Arguments) error {
 	types, err := r.getAllowTypesFromArguments(arguments)
 	if err != nil {
 		return err

rule/cyclomatic.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"sync"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -14,13 +13,14 @@ import (
 // CyclomaticRule sets restriction for maximum cyclomatic complexity.
 type CyclomaticRule struct {
 	maxComplexity int
-
-	configureOnce sync.Once
 }
 
 const defaultMaxCyclomaticComplexity = 10
 
-func (r *CyclomaticRule) configure(arguments lint.Arguments) error {
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *CyclomaticRule) Configure(arguments lint.Arguments) error {
 	if len(arguments) < 1 {
 		r.maxComplexity = defaultMaxCyclomaticComplexity
 		return nil
@@ -35,14 +35,7 @@ func (r *CyclomaticRule) configure(arguments lint.Arguments) error {
 }
 
 // Apply applies the rule to given file.
-func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	var configureErr error
-	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
-
-	if configureErr != nil {
-		return newInternalFailureError(configureErr)
-	}
-
+func (r *CyclomaticRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 	for _, decl := range file.AST.Decls {
 		fn, ok := decl.(*ast.FuncDecl)