From f48a41dc0d1d3f23f3afa0bfe751fce882f1d7d3 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sun, 8 Dec 2024 10:14:11 +0100 Subject: [PATCH 1/3] initial working version --- lint/failure.go | 15 +++++++++++++++ lint/file.go | 7 ++++++- lint/linter.go | 4 +--- lint/package.go | 25 +++++++++++++++++++++---- rule/add_constant.go | 27 +++++++++++++++++---------- 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/lint/failure.go b/lint/failure.go index 479b0cb48..17302a706 100644 --- a/lint/failure.go +++ b/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, + } +} diff --git a/lint/file.go b/lint/file.go index c695d63f5..4da078906 100644 --- a/lint/file.go +++ b/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,9 @@ 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 +126,7 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) { } } } + return nil } type enableDisableConfig struct { diff --git a/lint/linter.go b/lint/linter.go index 10f5cac36..0da195bb0 100644 --- a/lint/linter.go +++ b/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) { diff --git a/lint/package.go b/lint/package.go index 41575f480..78c8a802b 100644 --- a/lint/package.go +++ b/lint/package.go @@ -181,17 +181,34 @@ 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 + errChan := make(chan error) + doneChan := make(chan struct{}) + + wg.Add(len(p.files)) + go func() { // This goroutine will signal when all files where linted + wg.Wait() + doneChan <- struct{}{} + }() + for _, file := range p.files { - wg.Add(1) go (func(file *File) { - file.lint(rules, config, failures) + err := file.lint(rules, config, failures) + if err != nil { + errChan <- err // signal the error + } wg.Done() })(file) } - wg.Wait() + + select { // We block until... + case <-doneChan: //...all files were linted + return nil + case err := <-errChan: //...or there is an error + return err + } } // IsAtLeastGo121 returns true if the Go version for this package is 1.21 or higher, false otherwise diff --git a/rule/add_constant.go b/rule/add_constant.go index 5d7cd6095..4575cb4d8 100644 --- a/rule/add_constant.go +++ b/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,16 @@ 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 { + println(">>>> configuring") 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 +233,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 } From b4af81c174b8883a285b81d4329fbc1f4bebdb77 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sun, 8 Dec 2024 15:36:40 +0100 Subject: [PATCH 2/3] uses errgroup, removes debug println --- lint/file.go | 1 + lint/package.go | 29 ++++++----------------------- rule/add_constant.go | 1 - 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/lint/file.go b/lint/file.go index 4da078906..2498328d7 100644 --- a/lint/file.go +++ b/lint/file.go @@ -111,6 +111,7 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) error { if failure.IsInternal() { return errors.New(failure.Failure) } + if failure.RuleName == "" { failure.RuleName = currentRule.Name() } diff --git a/lint/package.go b/lint/package.go index 78c8a802b..db5be7600 100644 --- a/lint/package.go +++ b/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" @@ -183,32 +184,14 @@ func (p *Package) scanSortable() { func (p *Package) lint(rules []Rule, config Config, failures chan Failure) error { p.scanSortable() - var wg sync.WaitGroup - errChan := make(chan error) - doneChan := make(chan struct{}) - - wg.Add(len(p.files)) - go func() { // This goroutine will signal when all files where linted - wg.Wait() - doneChan <- struct{}{} - }() - + var eg errgroup.Group for _, file := range p.files { - go (func(file *File) { - err := file.lint(rules, config, failures) - if err != nil { - errChan <- err // signal the error - } - wg.Done() - })(file) + eg.Go(func() error { + return file.lint(rules, config, failures) + }) } - select { // We block until... - case <-doneChan: //...all files were linted - return nil - case err := <-errChan: //...or there is an error - return err - } + return eg.Wait() } // IsAtLeastGo121 returns true if the Go version for this package is 1.21 or higher, false otherwise diff --git a/rule/add_constant.go b/rule/add_constant.go index 4575cb4d8..b8fd10a96 100644 --- a/rule/add_constant.go +++ b/rule/add_constant.go @@ -206,7 +206,6 @@ func (w *lintAddConstantRule) isStructTag(n *ast.BasicLit) bool { } func (r *AddConstantRule) configure(arguments lint.Arguments) error { - println(">>>> configuring") r.strLitLimit = defaultStrLitLimit r.allowList = newAllowList() if len(arguments) == 0 { From fc661c3f22a920b62cc8b153b861428289c46809 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sun, 8 Dec 2024 15:37:30 +0100 Subject: [PATCH 3/3] update dependencies --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index 2c48d4b92..ca23cba6c 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 1cd6350b3..696de223e 100644 --- a/go.sum +++ b/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=