Skip to content

Commit cd55b7e

Browse files
n0izn0izthehowl
authored andcommitted
feat(gnovm/pkg/packages): categorize imports (#3323)
This makes the imports utils split imports by file kinds, allowing to make explicit decisions about what imports to use at the various callsites - Create `FileKind` enum to categorize gno files, with variants `PackageSource`, `Test`, `XTest` and `Filetest` - Create `GetFileKind` util to derive the `FileKind` from a file name and body - Create `ImportsMap` type that maps `FileKind`s to lists of imports. It has a single method `Merge` to select and merge various imports from multiple `FileKind`s - Modify the`packages.Imports` helper to return an `ImportsMap` instead of a `[]string` and adapt callsites by using`ImportMap.Merge` to preserve existing behavior This is something I need for #3304 and #2932 but to help reviews I made an atomic PR here instead --------- Signed-off-by: Norman Meier <[email protected]> Co-authored-by: Morgan <[email protected]>
1 parent a5b4439 commit cd55b7e

File tree

9 files changed

+224
-33
lines changed

9 files changed

+224
-33
lines changed

gno.land/pkg/integration/pkgloader.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,11 @@ func (pl *PkgsLoader) LoadPackage(modroot string, path, name string) error {
131131
if err != nil {
132132
return fmt.Errorf("unable to read package at %q: %w", currentPkg.Dir, err)
133133
}
134-
imports, err := packages.Imports(pkg, nil)
134+
importsMap, err := packages.Imports(pkg, nil)
135135
if err != nil {
136136
return fmt.Errorf("unable to load package imports in %q: %w", currentPkg.Dir, err)
137137
}
138+
imports := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindFiletest)
138139
for _, imp := range imports {
139140
if imp.PkgPath == currentPkg.Name || gnolang.IsStdlib(imp.PkgPath) {
140141
continue

gnovm/cmd/gno/download_deps.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ func downloadDeps(io commands.IO, pkgDir string, gnoMod *gnomod.File, fetcher pk
2525
if err != nil {
2626
return fmt.Errorf("read package at %q: %w", pkgDir, err)
2727
}
28-
imports, err := packages.Imports(pkg, nil)
28+
importsMap, err := packages.Imports(pkg, nil)
2929
if err != nil {
3030
return fmt.Errorf("read imports at %q: %w", pkgDir, err)
3131
}
32+
imports := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindXTest)
3233

3334
for _, pkgPath := range imports {
3435
resolved := gnoMod.Resolve(module.Version{Path: pkgPath.PkgPath})

gnovm/pkg/doc/dirs.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,12 @@ func packageImportsRecursive(root string, pkgPath string) []string {
105105
pkg = &gnovm.MemPackage{}
106106
}
107107

108-
resRaw, err := packages.Imports(pkg, nil)
108+
importsMap, err := packages.Imports(pkg, nil)
109109
if err != nil {
110110
// ignore packages with invalid imports
111-
resRaw = nil
111+
importsMap = nil
112112
}
113+
resRaw := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindXTest)
113114
res := make([]string, len(resRaw))
114115
for idx, imp := range resRaw {
115116
res[idx] = imp.PkgPath

gnovm/pkg/gnomod/pkg.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,13 @@ func ListPkgs(root string) (PkgList, error) {
121121
pkg = &gnovm.MemPackage{}
122122
}
123123

124-
importsRaw, err := packages.Imports(pkg, nil)
124+
importsMap, err := packages.Imports(pkg, nil)
125125
if err != nil {
126126
// ignore imports on error
127-
importsRaw = nil
127+
importsMap = nil
128128
}
129+
importsRaw := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindXTest)
130+
129131
imports := make([]string, 0, len(importsRaw))
130132
for _, imp := range importsRaw {
131133
// remove self and standard libraries from imports

gnovm/pkg/packages/filekind.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package packages
2+
3+
import (
4+
"fmt"
5+
"go/parser"
6+
"go/token"
7+
"strings"
8+
)
9+
10+
// FileKind represent the category a gno source file falls in, can be one of:
11+
//
12+
// - [FileKindPackageSource] -> A *.gno file that will be included in the gnovm package
13+
//
14+
// - [FileKindTest] -> A *_test.gno file that will be used for testing
15+
//
16+
// - [FileKindXTest] -> A *_test.gno file with a package name ending in _test that will be used for blackbox testing
17+
//
18+
// - [FileKindFiletest] -> A *_filetest.gno file that will be used for snapshot testing
19+
type FileKind uint
20+
21+
const (
22+
FileKindUnknown FileKind = iota
23+
FileKindPackageSource
24+
FileKindTest
25+
FileKindXTest
26+
FileKindFiletest
27+
)
28+
29+
// GetFileKind analyzes a file's name and body to get it's [FileKind], fset is optional
30+
func GetFileKind(filename string, body string, fset *token.FileSet) (FileKind, error) {
31+
if !strings.HasSuffix(filename, ".gno") {
32+
return FileKindUnknown, fmt.Errorf("%s:1:1: not a gno file", filename)
33+
}
34+
35+
if strings.HasSuffix(filename, "_filetest.gno") {
36+
return FileKindFiletest, nil
37+
}
38+
39+
if !strings.HasSuffix(filename, "_test.gno") {
40+
return FileKindPackageSource, nil
41+
}
42+
43+
if fset == nil {
44+
fset = token.NewFileSet()
45+
}
46+
ast, err := parser.ParseFile(fset, filename, body, parser.PackageClauseOnly)
47+
if err != nil {
48+
return FileKindUnknown, err
49+
}
50+
packageName := ast.Name.Name
51+
52+
if strings.HasSuffix(packageName, "_test") {
53+
return FileKindXTest, nil
54+
}
55+
return FileKindTest, nil
56+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package packages
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestGetFileKind(t *testing.T) {
10+
tcs := []struct {
11+
name string
12+
filename string
13+
body string
14+
fileKind FileKind
15+
errorContains string
16+
}{
17+
{
18+
name: "compiled",
19+
filename: "foo.gno",
20+
fileKind: FileKindPackageSource,
21+
},
22+
{
23+
name: "test",
24+
filename: "foo_test.gno",
25+
body: "package foo",
26+
fileKind: FileKindTest,
27+
},
28+
{
29+
name: "xtest",
30+
filename: "foo_test.gno",
31+
body: "package foo_test",
32+
fileKind: FileKindXTest,
33+
},
34+
{
35+
name: "filetest",
36+
filename: "foo_filetest.gno",
37+
fileKind: FileKindFiletest,
38+
},
39+
{
40+
name: "err_badpkgclause",
41+
filename: "foo_test.gno",
42+
body: "pakage foo",
43+
errorContains: "foo_test.gno:1:1: expected 'package', found pakage",
44+
},
45+
{
46+
name: "err_notgnofile",
47+
filename: "foo.gno.bck",
48+
errorContains: `foo.gno.bck:1:1: not a gno file`,
49+
},
50+
}
51+
52+
for _, tc := range tcs {
53+
t.Run(tc.name, func(t *testing.T) {
54+
out, err := GetFileKind(tc.filename, tc.body, nil)
55+
if len(tc.errorContains) != 0 {
56+
require.ErrorContains(t, err, tc.errorContains)
57+
} else {
58+
require.NoError(t, err)
59+
}
60+
require.Equal(t, tc.fileKind, out)
61+
})
62+
}
63+
}

gnovm/pkg/packages/imports.go

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,33 +14,40 @@ import (
1414

1515
// Imports returns the list of gno imports from a [gnovm.MemPackage].
1616
// fset is optional.
17-
func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) ([]FileImport, error) {
18-
allImports := make([]FileImport, 0, 16)
19-
seen := make(map[string]struct{}, 16)
17+
func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) {
18+
res := make(ImportsMap, 16)
19+
seen := make(map[FileKind]map[string]struct{}, 16)
20+
2021
for _, file := range pkg.Files {
2122
if !strings.HasSuffix(file.Name, ".gno") {
2223
continue
2324
}
24-
if strings.HasSuffix(file.Name, "_filetest.gno") {
25-
continue
25+
26+
fileKind, err := GetFileKind(file.Name, file.Body, fset)
27+
if err != nil {
28+
return nil, err
2629
}
2730
imports, err := FileImports(file.Name, file.Body, fset)
2831
if err != nil {
2932
return nil, err
3033
}
3134
for _, im := range imports {
32-
if _, ok := seen[im.PkgPath]; ok {
35+
if _, ok := seen[fileKind][im.PkgPath]; ok {
3336
continue
3437
}
35-
allImports = append(allImports, im)
36-
seen[im.PkgPath] = struct{}{}
38+
res[fileKind] = append(res[fileKind], im)
39+
if _, ok := seen[fileKind]; !ok {
40+
seen[fileKind] = make(map[string]struct{}, 16)
41+
}
42+
seen[fileKind][im.PkgPath] = struct{}{}
3743
}
3844
}
39-
sort.Slice(allImports, func(i, j int) bool {
40-
return allImports[i].PkgPath < allImports[j].PkgPath
41-
})
4245

43-
return allImports, nil
46+
for _, imports := range res {
47+
sortImports(imports)
48+
}
49+
50+
return res, nil
4451
}
4552

4653
// FileImport represents an import
@@ -75,3 +82,31 @@ func FileImports(filename string, src string, fset *token.FileSet) ([]FileImport
7582
}
7683
return res, nil
7784
}
85+
86+
type ImportsMap map[FileKind][]FileImport
87+
88+
// Merge merges imports, it removes duplicates and sorts the result
89+
func (imap ImportsMap) Merge(kinds ...FileKind) []FileImport {
90+
res := make([]FileImport, 0, 16)
91+
seen := make(map[string]struct{}, 16)
92+
93+
for _, kind := range kinds {
94+
for _, im := range imap[kind] {
95+
if _, ok := seen[im.PkgPath]; ok {
96+
continue
97+
}
98+
seen[im.PkgPath] = struct{}{}
99+
100+
res = append(res, im)
101+
}
102+
}
103+
104+
sortImports(res)
105+
return res
106+
}
107+
108+
func sortImports(imports []FileImport) {
109+
sort.Slice(imports, func(i, j int) bool {
110+
return imports[i].PkgPath < imports[j].PkgPath
111+
})
112+
}

gnovm/pkg/packages/imports_test.go

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,26 @@ func TestImports(t *testing.T) {
5858
)
5959
`,
6060
},
61+
{
62+
name: "file2_test.gno",
63+
data: `
64+
package tmp_test
65+
66+
import (
67+
"testing"
68+
69+
"gno.land/p/demo/testpkg"
70+
"gno.land/p/demo/xtestdep"
71+
)
72+
`,
73+
},
6174
{
6275
name: "z_0_filetest.gno",
6376
data: `
6477
package main
6578
6679
import (
67-
"gno.land/p/demo/filetestpkg"
80+
"gno.land/p/demo/filetestdep"
6881
)
6982
`,
7083
},
@@ -95,17 +108,28 @@ func TestImports(t *testing.T) {
95108
},
96109
}
97110

98-
// Expected list of imports
111+
// Expected lists of imports
99112
// - ignore subdirs
100113
// - ignore duplicate
101-
// - ignore *_filetest.gno
102114
// - should be sorted
103-
expected := []string{
104-
"gno.land/p/demo/pkg1",
105-
"gno.land/p/demo/pkg2",
106-
"gno.land/p/demo/testpkg",
107-
"std",
108-
"testing",
115+
expected := map[FileKind][]string{
116+
FileKindPackageSource: {
117+
"gno.land/p/demo/pkg1",
118+
"gno.land/p/demo/pkg2",
119+
"std",
120+
},
121+
FileKindTest: {
122+
"gno.land/p/demo/testpkg",
123+
"testing",
124+
},
125+
FileKindXTest: {
126+
"gno.land/p/demo/testpkg",
127+
"gno.land/p/demo/xtestdep",
128+
"testing",
129+
},
130+
FileKindFiletest: {
131+
"gno.land/p/demo/filetestdep",
132+
},
109133
}
110134

111135
// Create subpkg dir
@@ -120,12 +144,19 @@ func TestImports(t *testing.T) {
120144

121145
pkg, err := gnolang.ReadMemPackage(tmpDir, "test")
122146
require.NoError(t, err)
123-
imports, err := Imports(pkg, nil)
147+
148+
importsMap, err := Imports(pkg, nil)
124149
require.NoError(t, err)
125-
importsStrings := make([]string, len(imports))
126-
for idx, imp := range imports {
127-
importsStrings[idx] = imp.PkgPath
150+
151+
// ignore specs
152+
got := map[FileKind][]string{}
153+
for key, vals := range importsMap {
154+
stringVals := make([]string, len(vals))
155+
for i, val := range vals {
156+
stringVals[i] = val.PkgPath
157+
}
158+
got[key] = stringVals
128159
}
129160

130-
require.Equal(t, expected, importsStrings)
161+
require.Equal(t, expected, got)
131162
}

gnovm/pkg/test/imports.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,11 @@ func LoadImports(store gno.Store, memPkg *gnovm.MemPackage) (err error) {
242242
}()
243243

244244
fset := token.NewFileSet()
245-
imports, err := packages.Imports(memPkg, fset)
245+
importsMap, err := packages.Imports(memPkg, fset)
246246
if err != nil {
247247
return err
248248
}
249+
imports := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindXTest)
249250
for _, imp := range imports {
250251
if gno.IsRealmPath(imp.PkgPath) {
251252
// Don't eagerly load realms.

0 commit comments

Comments
 (0)