Skip to content

Commit

Permalink
hclwrite: fix data race in formatSpaces()
Browse files Browse the repository at this point in the history
* Fix data race in `formatSpaces()` by inlining shared state.
* Run tests with the race detector enabled.

Signed-off-by: Ryan Cragun <me@ryan.ec>
  • Loading branch information
ryancragun committed Feb 22, 2022
1 parent 3454437 commit a26ee4f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
uses: actions/checkout@v2
- name: Go test
run: |
go test ./...
go test ./... -race
fmt_and_vet:
name: "fmt and lint"
Expand Down
22 changes: 9 additions & 13 deletions hclwrite/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,6 @@ import (
"github.com/hashicorp/hcl/v2/hclsyntax"
)

var inKeyword = hclsyntax.Keyword([]byte{'i', 'n'})

// placeholder token used when we don't have a token but we don't want
// to pass a real "nil" and complicate things with nil pointer checks
var nilToken = &Token{
Type: hclsyntax.TokenNil,
Bytes: []byte{},
SpacesBefore: 0,
}

// format rewrites tokens within the given sequence, in-place, to adjust the
// whitespace around their content to achieve canonical formatting.
func format(tokens Tokens) {
Expand Down Expand Up @@ -108,6 +98,14 @@ func formatIndent(lines []formatLine) {
}

func formatSpaces(lines []formatLine) {
// placeholder token used when we don't have a token but we don't want
// to pass a real "nil" and complicate things with nil pointer checks
nilToken := &Token{
Type: hclsyntax.TokenNil,
Bytes: []byte{},
SpacesBefore: 0,
}

for _, line := range lines {
for i, token := range line.lead {
var before, after *Token
Expand Down Expand Up @@ -156,7 +154,6 @@ func formatSpaces(lines []formatLine) {
}

func formatCells(lines []formatLine) {

chainStart := -1
maxColumns := 0

Expand Down Expand Up @@ -218,7 +215,6 @@ func formatCells(lines []formatLine) {
if chainStart != -1 {
closeCommentChain(len(lines))
}

}

// spaceAfterToken decides whether a particular subject token should have a
Expand Down Expand Up @@ -251,7 +247,7 @@ func spaceAfterToken(subject, before, after *Token) bool {
// No extra spaces within templates
return false

case inKeyword.TokenMatches(subject.asHCLSyntax()) && before.Type == hclsyntax.TokenIdent:
case hclsyntax.Keyword([]byte{'i', 'n'}).TokenMatches(subject.asHCLSyntax()) && before.Type == hclsyntax.TokenIdent:
// This is a special case for inside for expressions where a user
// might want to use a literal tuple constructor:
// [for x in [foo]: x]
Expand Down
14 changes: 12 additions & 2 deletions hclwrite/round_trip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ block {
if !bytes.Equal(result, src) {
dmp := diffmatchpatch.New()
diffs := dmp.DiffMain(string(src), string(result), false)
//t.Errorf("wrong result\nresult:\n%s\ninput:\n%s", result, src)
// t.Errorf("wrong result\nresult:\n%s\ninput:\n%s", result, src)
t.Errorf("wrong result\ndiff: (red indicates missing lines, and green indicates unexpected lines)\n%s", dmp.DiffPrettyText(diffs))
}
})
Expand Down Expand Up @@ -124,7 +124,6 @@ func TestRoundTripFormat(t *testing.T) {

for _, test := range tests {
t.Run(test, func(t *testing.T) {

attrsAsObj := func(src []byte, phase string) cty.Value {
t.Logf("source %s:\n%s", phase, src)
f, diags := hclsyntax.ParseConfig(src, "", hcl.Pos{Line: 1, Column: 1})
Expand Down Expand Up @@ -168,5 +167,16 @@ func TestRoundTripFormat(t *testing.T) {
}
})
}
}

// TestRoundTripSafeConcurrent concurrently generates a new file. When run with
// the race detector it will fail if a data race is detected.
func TestRoundTripSafeConcurrent(t *testing.T) {
for i := 0; i < 2; i++ {
go func() {
f := NewEmptyFile()
b := f.Body()
b.SetAttributeValue("foo", cty.StringVal("bar"))
}()
}
}

0 comments on commit a26ee4f

Please sign in to comment.