Skip to content

Commit

Permalink
fix race condition when linting multiple files with -format option (f…
Browse files Browse the repository at this point in the history
…ix #370)
  • Loading branch information
rhysd committed Feb 16, 2024
1 parent e6a867d commit d80a2a3
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 15 deletions.
16 changes: 12 additions & 4 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"sort"
"strings"
"sync"
"text/template"

"github.com/fatih/color"
Expand Down Expand Up @@ -251,8 +252,10 @@ func (by byRuleNameField) Swap(i, j int) {
// ErrorFormatter is a formatter to format a slice of ErrorTemplateFields. It is used for
// formatting error messages with -format option.
type ErrorFormatter struct {
temp *template.Template
rules map[string]*ruleTemplateFields
// Note: Evaluating a template is thread-safe. // > // https://pkg.go.dev/text/template
temp *template.Template
rules map[string]*ruleTemplateFields
rulesMu sync.Mutex
}

// NewErrorFormatter creates new ErrorFormatter instance. Given format must contain at least one
Expand Down Expand Up @@ -294,7 +297,7 @@ func NewErrorFormatter(format string) (*ErrorFormatter, error) {
return nil, fmt.Errorf("template %q to format error messages could not be parsed: %w", format, err)
}

return &ErrorFormatter{t, r}, nil
return &ErrorFormatter{t, r, sync.Mutex{}}, nil
}

// Print formats the slice of template fields and prints it with given writer.
Expand All @@ -315,8 +318,13 @@ func (f *ErrorFormatter) PrintErrors(out io.Writer, errs []*Error, src []byte) e
}

// RegisterRule registers the rule. Registered rules are used to get description and index of error
// kinds when you use `kindDescription` or `kindIndex` functions in an error format template.
// kinds when you use `kindDescription` or `kindIndex` functions in an error format template. This
// method can be called multiple times safely in parallel.
func (f *ErrorFormatter) RegisterRule(r Rule) {
// Synchronize access to f.rules (#370)
f.rulesMu.Lock()
defer f.rulesMu.Unlock()

n := r.Name()
if _, ok := f.rules[n]; !ok {
f.rules[n] = &ruleTemplateFields{n, r.Description()}
Expand Down
61 changes: 50 additions & 11 deletions error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"math"
"sort"
Expand Down Expand Up @@ -362,7 +363,7 @@ func TestErrorGetTemplateFieldsOK(t *testing.T) {
}
}

// Regression for #128
// Regression test for #128
func TestErrorGetTemplateFieldsColumnIsOutOfBounds(t *testing.T) {
err := errorAt(&Pos{1, 9999}, "kind", "this is message")
err.Filepath = "filename.yaml"
Expand All @@ -372,6 +373,20 @@ func TestErrorGetTemplateFieldsColumnIsOutOfBounds(t *testing.T) {
}
}

func TestErrorErrorToString(t *testing.T) {
err := &Error{
Message: "this is message",
Line: 1,
Column: 2,
Kind: "test",
}
want := err.Error()
have := err.String()
if want != have {
t.Fatalf("wanted %q but have %q", want, have)
}
}

var testErrorTemplateFields = []*ErrorTemplateFields{
{
Message: "message 1",
Expand Down Expand Up @@ -634,16 +649,40 @@ func TestErrorFormatterPrintGetVersion(t *testing.T) {
}
}

func TestErrorString(t *testing.T) {
err := &Error{
Message: "this is message",
Line: 1,
Column: 2,
Kind: "test",
// Regression test for #370
func TestErrorFormatterRegisterRuleInParallel(t *testing.T) {
f, err := NewErrorFormatter("{{json .}}")
if err != nil {
t.Fatal(err)
}
want := err.Error()
have := err.String()
if want != have {
t.Fatalf("wanted %q but have %q", want, have)

rules := []Rule{}
for i := 0; i < 100; i++ {
rules = append(rules,
&RuleBase{
name: fmt.Sprintf("rule%d", i),
desc: fmt.Sprintf("description for rule%d", i),
},
)
}

done := make(chan struct{})

for i := 0; i < 100; i++ {
go func() {
for _, r := range rules {
f.RegisterRule(r)
}
done <- struct{}{}
}()
}

for i := 0; i < 100; i++ {
<-done
}

// Note: `syntax-check` rule is registered by NewErrorFormatter
if len(f.rules) != 101 {
t.Fatalf("not all rules were registered. %d rules were registered", len(f.rules))
}
}

0 comments on commit d80a2a3

Please sign in to comment.