refactor: modifies linting machinery to use Failure as a mean to signal erros in rules application (#1178)

Author: chavacava

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/spf13/afero v1.11.0
 	golang.org/x/mod v0.22.0
+	golang.org/x/sync v0.10.0
 	golang.org/x/tools v0.28.0
 )
 

go.sum

@@ -42,6 +42,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
 golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=

lint/failure.go

@@ -37,3 +37,18 @@ type Failure struct {
 func (f *Failure) GetFilename() string {
 	return f.Position.Start.Filename
 }
+
+const internalFailure = "REVIVE_INTERNAL"
+
+// IsInternal returns true if this failure is internal, false otherwise.
+func (f *Failure) IsInternal() bool {
+	return f.Category == internalFailure
+}
+
+// NewInternalFailure yields an internal failure with the given message as failure message.
+func NewInternalFailure(message string) Failure {
+	return Failure{
+		Category: internalFailure,
+		Failure:  message,
+	}
+}

lint/file.go

@@ -2,6 +2,7 @@ package lint
 
 import (
 	"bytes"
+	"errors"
 	"go/ast"
 	"go/parser"
 	"go/printer"
@@ -96,7 +97,7 @@ func (f *File) isMain() bool {
 
 const directiveSpecifyDisableReason = "specify-disable-reason"
 
-func (f *File) lint(rules []Rule, config Config, failures chan Failure) {
+func (f *File) lint(rules []Rule, config Config, failures chan Failure) error {
 	rulesConfig := config.Rules
 	_, mustSpecifyDisableReason := config.Directives[directiveSpecifyDisableReason]
 	disabledIntervals := f.disabledIntervals(rules, mustSpecifyDisableReason, failures)
@@ -107,6 +108,10 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) {
 		}
 		currentFailures := currentRule.Apply(f, ruleConfig.Arguments)
 		for idx, failure := range currentFailures {
+			if failure.IsInternal() {
+				return errors.New(failure.Failure)
+			}
+
 			if failure.RuleName == "" {
 				failure.RuleName = currentRule.Name()
 			}
@@ -122,6 +127,7 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) {
 			}
 		}
 	}
+	return nil
 }
 
 type enableDisableConfig struct {

lint/linter.go

@@ -152,9 +152,7 @@ func (l *Linter) lintPackage(filenames []string, gover *goversion.Version, ruleS
 		return nil
 	}
 
-	pkg.lint(ruleSet, config, failures)
-
-	return nil
+	return pkg.lint(ruleSet, config, failures)
 }
 
 func detectGoMod(dir string) (rootDir string, ver *goversion.Version, err error) {

lint/package.go

@@ -9,6 +9,7 @@ import (
 	"sync"
 
 	goversion "github.com/hashicorp/go-version"
+	"golang.org/x/sync/errgroup"
 
 	"github.com/mgechev/revive/internal/astutils"
 	"github.com/mgechev/revive/internal/typeparams"
@@ -181,17 +182,16 @@ func (p *Package) scanSortable() {
 	}
 }
 
-func (p *Package) lint(rules []Rule, config Config, failures chan Failure) {
+func (p *Package) lint(rules []Rule, config Config, failures chan Failure) error {
 	p.scanSortable()
-	var wg sync.WaitGroup
+	var eg errgroup.Group
 	for _, file := range p.files {
-		wg.Add(1)
-		go (func(file *File) {
-			file.lint(rules, config, failures)
-			wg.Done()
-		})(file)
+		eg.Go(func() error {
+			return file.lint(rules, config, failures)
+		})
 	}
-	wg.Wait()
+
+	return eg.Wait()
 }
 
 // IsAtLeastGo121 returns true if the Go version for this package is 1.21 or higher, false otherwise

rule/add_constant.go

@@ -42,7 +42,11 @@ type AddConstantRule struct {
 
 // Apply applies the rule to given file.
 func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
-	r.configureOnce.Do(func() { r.configure(arguments) })
+	var configureErr error
+	r.configureOnce.Do(func() { configureErr = r.configure(arguments) })
+	if configureErr != nil {
+		return []lint.Failure{lint.NewInternalFailure(configureErr.Error())}
+	}
 
 	var failures []lint.Failure
 
@@ -201,15 +205,15 @@ func (w *lintAddConstantRule) isStructTag(n *ast.BasicLit) bool {
 	return ok
 }
 
-func (r *AddConstantRule) configure(arguments lint.Arguments) {
+func (r *AddConstantRule) configure(arguments lint.Arguments) error {
 	r.strLitLimit = defaultStrLitLimit
 	r.allowList = newAllowList()
 	if len(arguments) == 0 {
-		return
+		return nil
 	}
 	args, ok := arguments[0].(map[string]any)
 	if !ok {
-		panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting a k,v map. Got %T", arguments[0]))
+		return fmt.Errorf("invalid argument to the add-constant rule, expecting a k,v map. Got %T", arguments[0])
 	}
 	for k, v := range args {
 		kind := ""
@@ -228,39 +232,41 @@ func (r *AddConstantRule) configure(arguments lint.Arguments) {
 			}
 			list, ok := v.(string)
 			if !ok {
-				panic(fmt.Sprintf("Invalid argument to the add-constant rule, string expected. Got '%v' (%T)", v, v))
+				fmt.Errorf("invalid argument to the add-constant rule, string expected. Got '%v' (%T)", v, v)
 			}
 			r.allowList.add(kind, list)
 		case "maxLitCount":
 			sl, ok := v.(string)
 			if !ok {
-				panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v' (%T)", v, v))
+				fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v' (%T)", v, v)
 			}
 
 			limit, err := strconv.Atoi(sl)
 			if err != nil {
-				panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v))
+				fmt.Errorf("invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)
 			}
 			r.strLitLimit = limit
 		case "ignoreFuncs":
 			excludes, ok := v.(string)
 			if !ok {
-				panic(fmt.Sprintf("Invalid argument to the ignoreFuncs parameter of add-constant rule, string expected. Got '%v' (%T)", v, v))
+				fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule, string expected. Got '%v' (%T)", v, v)
 			}
 
 			for _, exclude := range strings.Split(excludes, ",") {
 				exclude = strings.Trim(exclude, " ")
 				if exclude == "" {
-					panic("Invalid argument to the ignoreFuncs parameter of add-constant rule, expected regular expression must not be empty.")
+					fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule, expected regular expression must not be empty.")
 				}
 
 				exp, err := regexp.Compile(exclude)
 				if err != nil {
-					panic(fmt.Sprintf("Invalid argument to the ignoreFuncs parameter of add-constant rule: regexp %q does not compile: %v", exclude, err))
+					fmt.Errorf("invalid argument to the ignoreFuncs parameter of add-constant rule: regexp %q does not compile: %v", exclude, err)
 				}
 
 				r.ignoreFunctions = append(r.ignoreFunctions, exp)
 			}
 		}
 	}
+
+	return nil
 }