Skip to content

Commit f71d206

Browse files
mdbmdb
authored
fix: Some NOOP cleanup and a buggy test fix!
* correct contraction "it's" to "its" Signed-off-by: Mike Ball <mikedball@gmail.com> * place period inside quote Signed-off-by: Mike Ball <mikedball@gmail.com> * run 'gofmt -w ./..' to consistently format files Signed-off-by: Mike Ball <mikedball@gmail.com> * build trigger * declare tc variable before its use This declares the `tc` variable in a consistent location, and also ensures its declaration before its use. * fix: ensure package comments appear in consistent order This fixes the flaky test failures (https://github.com/yardbirdsax/aster/actions/runs/3832511354) that were previously vulnerable to inconsistent ordering of the package comments. The inconsistent ordering itself was the product of the fact that ranging over a map in Go does not guarantee consistent order. Therefore, the tests' assertion that `aster:*` comments appear before `aster_test:*` comments was not guarnateed. This fixes the test vulnerability by sorting the package comments within the `aster#PackageComment` method before returning them. Signed-off-by: Mike Ball <mikedball@gmail.com>
1 parent 1ad0ea0 commit f71d206

File tree

6 files changed

+104
-98
lines changed

6 files changed

+104
-98
lines changed

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ with [code markers](https://book.kubebuilder.io/reference/markers.html). I knew
1616
but found the interface very complex. What was a [`Decl`](https://pkg.go.dev/go/ast#Decl) and when
1717
is something a `GenDecl` vs `FuncDecl`? How do I figure out what the type of an object is (i.e.
1818
`struct` vs `func` vs something else)? And once I actually retrieve the right `Decl`, how do I get
19-
it's name or what its members / fields are?
19+
its name or what its members / fields are?
2020

2121
The goal of `aster` was to be able to say "here's a pattern, find me all objects that have
22-
comments matching it attached, and tell me what they are and, if applicable, what their fields are".
22+
comments matching it attached, and tell me what they are and, if applicable, what their fields are."
2323
For a simple example, see the [`astersample`](cmd/astersample/main.go) program.

‎aster.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"go/parser"
1010
"go/token"
1111
"regexp"
12+
"sort"
1213
"strings"
1314
)
1415

@@ -64,14 +65,19 @@ func (a *Aster) PackageComment() string {
6465
packageComments := []string{}
6566

6667
for pkgName, pkg := range a.packages {
68+
var b strings.Builder
6769
if len(a.packages) > 1 {
68-
packageComments = append(packageComments, pkgName+":\n")
70+
b.WriteString(pkgName + ":\n")
6971
}
7072
for _, f := range pkg.Files {
71-
packageComments = append(packageComments, f.Doc.Text())
73+
b.WriteString(f.Doc.Text())
7274
}
75+
76+
packageComments = append(packageComments, b.String())
7377
}
7478

79+
sort.Strings(packageComments)
80+
7581
return strings.Join(packageComments, "")
7682
}
7783

‎aster_test.go‎

Lines changed: 72 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -9,85 +9,85 @@ import (
99
)
1010

1111
func TestGetPackageComment(t *testing.T) {
12-
tests := []struct{
13-
name string
14-
directory string
15-
packages []string
16-
expectedPackageComment string
17-
}{
18-
{
19-
name: "one package",
20-
directory: ".",
21-
packages: []string{"aster"},
22-
expectedPackageComment: "Package aster provides a high level interface for parsing Go code using the stdlib's\n" +
23-
"[`go/ast`](https://pkg.go.dev/go/ast) module.\n",
24-
},
25-
{
26-
name: "multi package",
27-
directory: ".",
28-
packages: []string{"aster", "aster_test"},
29-
expectedPackageComment: "aster:\nPackage aster provides a high level interface for parsing Go code using the stdlib's\n" +
30-
"[`go/ast`](https://pkg.go.dev/go/ast) module.\n" +
31-
"aster_test:\nPackage aster_test contains all the tests for the `aster` module.\n",
32-
},
33-
}
12+
tests := []struct {
13+
name string
14+
directory string
15+
packages []string
16+
expectedPackageComment string
17+
}{
18+
{
19+
name: "one package",
20+
directory: ".",
21+
packages: []string{"aster"},
22+
expectedPackageComment: "Package aster provides a high level interface for parsing Go code using the stdlib's\n" +
23+
"[`go/ast`](https://pkg.go.dev/go/ast) module.\n",
24+
},
25+
{
26+
name: "multi package",
27+
directory: ".",
28+
packages: []string{"aster", "aster_test"},
29+
expectedPackageComment: "aster:\nPackage aster provides a high level interface for parsing Go code using the stdlib's\n" +
30+
"[`go/ast`](https://pkg.go.dev/go/ast) module.\n" +
31+
"aster_test:\nPackage aster_test contains all the tests for the `aster` module.\n",
32+
},
33+
}
3434

35-
for _, tc := range tests {
36-
t.Run(tc.name, func(t *testing.T) {
37-
tc := tc
38-
t.Parallel()
35+
for _, tc := range tests {
36+
tc := tc
37+
t.Run(tc.name, func(t *testing.T) {
38+
t.Parallel()
3939

40-
actualPackageComment := aster.FromDirectory(tc.directory).Packages(tc.packages).PackageComment()
40+
actualPackageComment := aster.FromDirectory(tc.directory).Packages(tc.packages).PackageComment()
4141

42-
assert.Equal(t, tc.expectedPackageComment, actualPackageComment)
43-
})
44-
}
42+
assert.Equal(t, tc.expectedPackageComment, actualPackageComment)
43+
})
44+
}
4545
}
4646

4747
func TestMatchComment(t *testing.T) {
48-
tests := []struct{
49-
name string
50-
directory string
51-
comment string
52-
expectedResults []aster.Result
53-
}{
54-
{
55-
name: "with comment",
56-
directory: "sample",
57-
comment: "aster:",
58-
expectedResults: []aster.Result{
59-
{
60-
Name: "Sample",
61-
Type: "struct",
62-
Comments: "aster: hello\n",
63-
Fields: []aster.Field{
64-
{
65-
Name: "Text",
66-
Type: "string",
67-
Comments: "Text is some sample text\n",
68-
},
69-
},
70-
},
71-
{
72-
Name: "Sampler",
73-
Type: "func",
74-
Comments: "aster: hello again\n",
75-
},
76-
},
77-
},
78-
}
48+
tests := []struct {
49+
name string
50+
directory string
51+
comment string
52+
expectedResults []aster.Result
53+
}{
54+
{
55+
name: "with comment",
56+
directory: "sample",
57+
comment: "aster:",
58+
expectedResults: []aster.Result{
59+
{
60+
Name: "Sample",
61+
Type: "struct",
62+
Comments: "aster: hello\n",
63+
Fields: []aster.Field{
64+
{
65+
Name: "Text",
66+
Type: "string",
67+
Comments: "Text is some sample text\n",
68+
},
69+
},
70+
},
71+
{
72+
Name: "Sampler",
73+
Type: "func",
74+
Comments: "aster: hello again\n",
75+
},
76+
},
77+
},
78+
}
7979

80-
for _, tc := range tests {
81-
tc := tc
82-
t.Run(tc.name, func(t *testing.T) {
83-
t.Parallel()
80+
for _, tc := range tests {
81+
tc := tc
82+
t.Run(tc.name, func(t *testing.T) {
83+
t.Parallel()
8484

85-
a := aster.FromDirectory(tc.directory)
86-
actualResults, err := a.MatchComment("aster:")
85+
a := aster.FromDirectory(tc.directory)
86+
actualResults, err := a.MatchComment("aster:")
8787

88-
assert.Equal(t, tc.expectedResults, actualResults)
89-
assert.NoError(t, a.Error)
90-
assert.NoError(t, err)
91-
})
92-
}
88+
assert.Equal(t, tc.expectedResults, actualResults)
89+
assert.NoError(t, a.Error)
90+
assert.NoError(t, err)
91+
})
92+
}
9393
}

‎cmd/astersample/main.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@ import (
88
)
99

1010
func main() {
11-
dir := flag.String("d", "", "The directory to parse")
12-
flag.Parse()
11+
dir := flag.String("d", "", "The directory to parse")
12+
flag.Parse()
1313

14-
if dir == nil || *dir == "" {
15-
log.Fatal("must provide a value for the directory to be parsed")
16-
}
14+
if dir == nil || *dir == "" {
15+
log.Fatal("must provide a value for the directory to be parsed")
16+
}
1717

18-
results, err := aster.FromDirectory(*dir).MatchComment("aster:")
19-
if err != nil {
20-
log.Fatal("could not retrieve results: %w", err)
21-
}
18+
results, err := aster.FromDirectory(*dir).MatchComment("aster:")
19+
if err != nil {
20+
log.Fatal("could not retrieve results: %w", err)
21+
}
2222

23-
for _, r := range results {
24-
log.Println("result found:")
25-
log.Printf("\tname: %s, type: %s", r.Name, r.Type)
26-
log.Printf("\tComments:\n\t\t\t%s", r.Comments)
27-
log.Println("------------------------------------")
28-
}
29-
}
23+
for _, r := range results {
24+
log.Println("result found:")
25+
log.Printf("\tname: %s, type: %s", r.Name, r.Type)
26+
log.Printf("\tComments:\n\t\t\t%s", r.Comments)
27+
log.Println("------------------------------------")
28+
}
29+
}

‎sample/doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
// Package sample is, well, a sample.
22
// This is another line to ensure the entire comment set is captured.
3-
package sample
3+
package sample

‎sample/sample.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package sample
22

3-
//aster: hello
3+
// aster: hello
44
type Sample struct {
5-
// Text is some sample text
6-
Text string
5+
// Text is some sample text
6+
Text string
77
}
88

9-
//aster: hello again
10-
func (s Sample) Sampler() {}
9+
// aster: hello again
10+
func (s Sample) Sampler() {}

0 commit comments

Comments
 (0)