Skip to content

Commit

Permalink
refactor: modifies linting machinery to use Failure as a mean to sign…
Browse files Browse the repository at this point in the history
…al erros in rules application (#1178)
  • Loading branch information
chavacava authored Dec 8, 2024
1 parent f6a3820 commit e23fcdb
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 22 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
15 changes: 15 additions & 0 deletions lint/failure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
8 changes: 7 additions & 1 deletion lint/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package lint

import (
"bytes"
"errors"
"go/ast"
"go/parser"
"go/printer"
Expand Down Expand Up @@ -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)
Expand All @@ -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()
}
Expand All @@ -122,6 +127,7 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) {
}
}
}
return nil
}

type enableDisableConfig struct {
Expand Down
4 changes: 1 addition & 3 deletions lint/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 8 additions & 8 deletions lint/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
26 changes: 16 additions & 10 deletions rule/add_constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 := ""
Expand All @@ -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
}

0 comments on commit e23fcdb

Please sign in to comment.