Skip to content

Commit 71e1d26

Browse files
authored
fix: fix generated syscall package description (#369)
* fix: validate doc string * feat: filter build-constraint files * fix: update tests
1 parent 0a6c543 commit 71e1d26

File tree

4 files changed

+130
-14
lines changed

4 files changed

+130
-14
lines changed

internal/pkgindex/parser.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,15 @@ func ParseImportCompletionItem(ctx context.Context, params PackageParseParams) (
5757
continue
5858
}
5959

60-
// Found a doc string, exit.
6160
result.Detail = src.Name.String()
62-
docComment := strings.TrimSpace(analyzer.FormatDocString(doc.Text()).Value)
61+
62+
// Package doc should start with "Package xxx", see issue #367.
63+
docComment := strings.TrimSpace(doc.Text())
64+
if !validatePackageDoc(docComment) {
65+
continue
66+
}
67+
68+
docComment = strings.TrimSpace(analyzer.FormatDocString(docComment).Value)
6369
docString = docComment + "\n\n" + docString
6470
break
6571
}
@@ -71,3 +77,8 @@ func ParseImportCompletionItem(ctx context.Context, params PackageParseParams) (
7177
})
7278
return result, nil
7379
}
80+
81+
func validatePackageDoc(str string) bool {
82+
normalizedStr := strings.ToLower(str)
83+
return strings.HasPrefix(normalizedStr, "package ")
84+
}

internal/pkgindex/parser_test.go

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"strings"
89
"testing"
910

1011
"github.com/stretchr/testify/require"
@@ -13,10 +14,13 @@ import (
1314

1415
func TestParseImportCompletionItem(t *testing.T) {
1516
cases := map[string]struct {
16-
pkgName string
17-
expectErr string
18-
getContext func() context.Context
19-
expect func(importPath string) monaco.CompletionItem
17+
pkgName string
18+
customGoRoot string
19+
expectErr string
20+
filterInputFiles bool
21+
getContext func() context.Context
22+
expect func(importPath string) monaco.CompletionItem
23+
compareFunc func(t *testing.T, got monaco.CompletionItem)
2024
}{
2125
"package with documentation": {
2226
pkgName: "foopkg/pkgbar",
@@ -67,6 +71,15 @@ func TestParseImportCompletionItem(t *testing.T) {
6771
return ctx
6872
},
6973
},
74+
"367-syscall-invalid-description": {
75+
customGoRoot: mustGetGoRoot(t) + "/src",
76+
pkgName: "syscall",
77+
filterInputFiles: true,
78+
compareFunc: func(t *testing.T, got monaco.CompletionItem) {
79+
expectPfx := "Package syscall contains an interface to the low-level"
80+
expectStartWith(t, got.Documentation.Value.Value, expectPfx)
81+
},
82+
},
7083
}
7184

7285
const rootDir = "testdata"
@@ -77,25 +90,35 @@ func TestParseImportCompletionItem(t *testing.T) {
7790
ctx = c.getContext()
7891
}
7992

93+
goRoot := rootDir
94+
if c.customGoRoot != "" {
95+
goRoot = c.customGoRoot
96+
}
97+
8098
result, err := ParseImportCompletionItem(ctx, PackageParseParams{
81-
RootDir: rootDir,
99+
RootDir: goRoot,
82100
ImportPath: c.pkgName,
83-
Files: findDirFiles(t, rootDir, c.pkgName),
101+
Files: findDirFiles(t, goRoot, c.pkgName, c.filterInputFiles),
84102
})
85103
if c.expectErr != "" {
86104
require.Error(t, err)
87105
require.Contains(t, err.Error(), c.expectErr)
88106
return
89107
}
90108

91-
expect := c.expect(c.pkgName)
92109
require.NoError(t, err)
110+
if c.compareFunc != nil {
111+
c.compareFunc(t, result)
112+
return
113+
}
114+
115+
expect := c.expect(c.pkgName)
93116
require.Equal(t, expect, result)
94117
})
95118
}
96119
}
97120

98-
func findDirFiles(t *testing.T, dir, pkgName string) []string {
121+
func findDirFiles(t *testing.T, dir, pkgName string, filterFiles bool) []string {
99122
t.Helper()
100123
entries, err := os.ReadDir(filepath.Join(dir, pkgName))
101124
require.NoError(t, err)
@@ -106,8 +129,26 @@ func findDirFiles(t *testing.T, dir, pkgName string) []string {
106129
continue
107130
}
108131

109-
files = append(files, entry.Name())
132+
name := entry.Name()
133+
if filterFiles {
134+
if strings.HasSuffix(name, "_test.go") || !strings.HasSuffix(name, ".go") {
135+
continue
136+
}
137+
}
138+
139+
files = append(files, name)
110140
}
111141

112142
return files
113143
}
144+
145+
func mustGetGoRoot(t *testing.T) string {
146+
t.Helper()
147+
v, err := ResolveGoRoot()
148+
require.NoError(t, err)
149+
return v
150+
}
151+
152+
func expectStartWith(t *testing.T, str string, pfx string) {
153+
require.Truef(t, strings.HasPrefix(str, pfx), "string %q should start with %q", str, pfx)
154+
}

internal/pkgindex/scanner.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,21 @@ import (
1010
"os"
1111
"path"
1212
"path/filepath"
13+
"regexp"
1314
"strings"
1415

1516
"github.com/x1unix/go-playground/pkg/monaco"
1617
)
1718

1819
const (
19-
resultsPreallocSize = 300
20+
queueInitSize = 300
21+
22+
// resultsInitSize value based on a number of packages in Go 1.22
23+
resultsInitSize = 180
2024
)
2125

26+
var buildContraintRegex = regexp.MustCompile(`(?m)^([\S]+)_(freebsd|darwin|plan9|windows|netbsd|arm|386|loong64|mips|ppc|riscv|s390x)`)
27+
2228
type scanResult struct {
2329
hasPkg bool
2430
item monaco.CompletionItem
@@ -60,7 +66,7 @@ func (s *GoRootScanner) start() ([]monaco.CompletionItem, error) {
6066
return nil, fmt.Errorf("cannot open Go SDK directory: %w", err)
6167
}
6268

63-
q := newQueue(resultsPreallocSize)
69+
q := newQueue(queueInitSize)
6470
for _, entry := range entries {
6571
if !entry.Type().IsDir() {
6672
continue
@@ -75,7 +81,7 @@ func (s *GoRootScanner) start() ([]monaco.CompletionItem, error) {
7581
q.add(pkgName)
7682
}
7783

78-
results := make([]monaco.CompletionItem, 0, resultsPreallocSize)
84+
results := make([]monaco.CompletionItem, 0, resultsInitSize)
7985
for q.occupied() {
8086
pkgName, ok := q.pop()
8187
if !ok {
@@ -169,5 +175,19 @@ func shouldIgnoreFileName(fname string) bool {
169175
return true
170176
}
171177

178+
if isBuildConstraintFile(fname) {
179+
return true
180+
}
181+
172182
return strings.HasSuffix(fname, "_test.go")
173183
}
184+
185+
// isBuildConstraintFile checks whether file name contains Go build constraint.
186+
//
187+
// Linux and "_unix.go" files are allowed to align with GoDoc behavior.
188+
// Wasm-target files are intentionally allowed to support `syscall/js` package.
189+
//
190+
// For example: `foo_amd64.go` or `bar_openbsd_ppc64.go`.
191+
func isBuildConstraintFile(fname string) bool {
192+
return buildContraintRegex.MatchString(fname)
193+
}

internal/pkgindex/scanner_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package pkgindex
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestShouldIgnoreFileName(t *testing.T) {
10+
ignoredFiles := []string{
11+
"internal",
12+
"testdata",
13+
"foo_test.go",
14+
"bar_windows.go",
15+
"foo_darwin.go",
16+
"foo_openbsd_arm64.go",
17+
"foo_netbsd_riscv64.go",
18+
"mmap_unix_test.go",
19+
"zerrors_linux_mips64.go",
20+
}
21+
22+
allowFiles := []string{
23+
"foo.go",
24+
"foo_bar.go",
25+
"foo_js.go",
26+
"foo_wasm.go",
27+
"foo_js_wasm.go",
28+
"env_unix.go",
29+
"exec_linux.go",
30+
"zerrors_linux_amd64.go",
31+
}
32+
33+
t.Run("ignore", func(t *testing.T) {
34+
for _, n := range ignoredFiles {
35+
require.True(t, shouldIgnoreFileName(n), n)
36+
}
37+
})
38+
39+
t.Run("allow", func(t *testing.T) {
40+
for _, n := range allowFiles {
41+
require.False(t, shouldIgnoreFileName(n), n)
42+
}
43+
})
44+
}

0 commit comments

Comments
 (0)