Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gnovm): categorize imports #3323

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6c5d83e
feat: categorize imports
n0izn0iz Dec 11, 2024
d05c43b
Merge branch 'master' into imports-map
n0izn0iz Dec 11, 2024
702e3d9
Merge branch 'master' into imports-map
n0izn0iz Dec 11, 2024
1d17b9f
Merge branch 'master' into imports-map
n0izn0iz Dec 13, 2024
c4d82dc
Merge branch 'master' into imports-map
n0izn0iz Dec 17, 2024
27bb085
Merge branch 'master' into imports-map
n0izn0iz Dec 18, 2024
3e04787
chore: Merge remote-tracking branch 'origin/master' into imports-map
n0izn0iz Dec 19, 2024
a0dd0d8
chore: move code for easier review
n0izn0iz Dec 19, 2024
d2d043e
chore: pass fset to GetFileKind
n0izn0iz Dec 19, 2024
efda22d
fix: usage of packages.Imports
n0izn0iz Dec 19, 2024
fcd66a5
chore: don't expose SortImports
n0izn0iz Dec 19, 2024
37d96ce
chore: error msg with loc
n0izn0iz Dec 19, 2024
e591002
Merge branch 'master' into imports-map
n0izn0iz Dec 19, 2024
315b37f
Merge branch 'master' into imports-map
n0izn0iz Dec 19, 2024
cd91edb
Merge branch 'master' into imports-map
n0izn0iz Dec 20, 2024
c0998d7
chore: nit
n0izn0iz Dec 21, 2024
41ddaa6
chore: revert merge fail
n0izn0iz Dec 21, 2024
4cfda1e
chore: FileKindCompiled -> FileKindPackageSource
n0izn0iz Dec 21, 2024
2100a57
chore: FileKindXtest -> FilKindXTest
n0izn0iz Dec 21, 2024
0be8c69
chore: add FileKind doc
n0izn0iz Dec 21, 2024
1fcf4e3
Merge branch 'master' into imports-map
n0izn0iz Dec 21, 2024
0ebf4f5
Merge branch 'master' into imports-map
n0izn0iz Dec 25, 2024
0490b9e
Merge branch 'master' into imports-map
n0izn0iz Jan 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gno.land/pkg/integration/testing_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,10 +753,11 @@ func (pl *pkgsLoader) LoadPackage(modroot string, path, name string) error {
if err != nil {
return fmt.Errorf("unable to read package at %q: %w", currentPkg.Dir, err)
}
imports, err := packages.Imports(pkg, nil)
importsMap, err := packages.Imports(pkg, nil)
if err != nil {
return fmt.Errorf("unable to load package imports in %q: %w", currentPkg.Dir, err)
}
imports := importsMap.Merge(packages.FileKindCompiled, packages.FileKindTest, packages.FileKindXtest)
for _, imp := range imports {
if imp.PkgPath == currentPkg.Name || gnolang.IsStdlib(imp.PkgPath) {
continue
Expand Down
3 changes: 2 additions & 1 deletion gnovm/cmd/gno/download_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ func downloadDeps(io commands.IO, pkgDir string, gnoMod *gnomod.File, fetcher pk
if err != nil {
return fmt.Errorf("read package at %q: %w", pkgDir, err)
}
imports, err := packages.Imports(pkg, nil)
importsMap, err := packages.Imports(pkg, nil)
if err != nil {
return fmt.Errorf("read imports at %q: %w", pkgDir, err)
}
imports := importsMap.Merge(packages.FileKindCompiled, packages.FileKindTest, packages.FileKindXtest)

for _, pkgPath := range imports {
resolved := gnoMod.Resolve(module.Version{Path: pkgPath.PkgPath})
Expand Down
5 changes: 3 additions & 2 deletions gnovm/pkg/doc/dirs.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@
pkg = &gnovm.MemPackage{}
}

resRaw, err := packages.Imports(pkg, nil)
importsMap, err := packages.Imports(pkg, nil)
if err != nil {
// ignore packages with invalid imports
resRaw = nil
importsMap = nil

Check warning on line 111 in gnovm/pkg/doc/dirs.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/doc/dirs.go#L111

Added line #L111 was not covered by tests
}
resRaw := importsMap.Merge(packages.FileKindCompiled, packages.FileKindTest, packages.FileKindXtest)
res := make([]string, len(resRaw))
for idx, imp := range resRaw {
res[idx] = imp.PkgPath
Expand Down
6 changes: 4 additions & 2 deletions gnovm/pkg/gnomod/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,13 @@
pkg = &gnovm.MemPackage{}
}

importsRaw, err := packages.Imports(pkg, nil)
importsMap, err := packages.Imports(pkg, nil)
if err != nil {
// ignore imports on error
importsRaw = nil
importsMap = nil

Check warning on line 127 in gnovm/pkg/gnomod/pkg.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnomod/pkg.go#L127

Added line #L127 was not covered by tests
}
importsRaw := importsMap.Merge(packages.FileKindCompiled, packages.FileKindTest, packages.FileKindXtest)

imports := make([]string, 0, len(importsRaw))
for _, imp := range importsRaw {
// remove self and standard libraries from imports
Expand Down
47 changes: 47 additions & 0 deletions gnovm/pkg/packages/filekind.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package packages

import (
"fmt"
"go/parser"
"go/token"
"strings"
)

type FileKind uint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add godoc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


const (
FileKindUnknown = FileKind(iota)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FileKindUnknown = FileKind(iota)
FileKindUnknown FileKind = iota

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileKindCompiled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FileKindCompiled
FileKindPackageSource

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileKindTest
FileKindXtest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileKindFiletest
)

// GetFileKind analyzes a file's name and body to get it's [FileKind], fset is optional
func GetFileKind(filename string, body string, fset *token.FileSet) (FileKind, error) {
if !strings.HasSuffix(filename, ".gno") {
return FileKindUnknown, fmt.Errorf("%s:1:1: not a gno file", filename)
}

if strings.HasSuffix(filename, "_filetest.gno") {
return FileKindFiletest, nil
}

if !strings.HasSuffix(filename, "_test.gno") {
return FileKindCompiled, nil
}

if fset == nil {
fset = token.NewFileSet()
}
ast, err := parser.ParseFile(fset, filename, body, parser.PackageClauseOnly)
if err != nil {
return FileKindUnknown, err
}
packageName := ast.Name.Name

if strings.HasSuffix(packageName, "_test") {
return FileKindXtest, nil
}
return FileKindTest, nil
}
64 changes: 64 additions & 0 deletions gnovm/pkg/packages/filekind_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package packages_test

import (
"testing"

. "github.com/gnolang/gno/gnovm/pkg/packages"
"github.com/stretchr/testify/require"
)

func TestGetFileKind(t *testing.T) {
tcs := []struct {
name string
filename string
body string
fileKind FileKind
errorContains string
}{
{
name: "compiled",
filename: "foo.gno",
fileKind: FileKindCompiled,
},
{
name: "test",
filename: "foo_test.gno",
body: "package foo",
fileKind: FileKindTest,
},
{
name: "xtest",
filename: "foo_test.gno",
body: "package foo_test",
fileKind: FileKindXtest,
},
{
name: "filetest",
filename: "foo_filetest.gno",
fileKind: FileKindFiletest,
},
{
name: "err_badpkgclause",
filename: "foo_test.gno",
body: "pakage foo",
errorContains: "foo_test.gno:1:1: expected 'package', found pakage",
},
{
name: "err_notgnofile",
filename: "foo.gno.bck",
errorContains: `foo.gno.bck:1:1: not a gno file`,
},
}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
out, err := GetFileKind(tc.filename, tc.body, nil)
if len(tc.errorContains) != 0 {
require.ErrorContains(t, err, tc.errorContains)
} else {
require.NoError(t, err)
}
require.Equal(t, tc.fileKind, out)
})
}
}
60 changes: 47 additions & 13 deletions gnovm/pkg/packages/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,40 @@
)

// Imports returns the list of gno imports from a [gnovm.MemPackage].
// fset is optional.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would still keep it as a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad, conflict resolution fail: 41ddaa6

func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) ([]FileImport, error) {
allImports := make([]FileImport, 0, 16)
seen := make(map[string]struct{}, 16)
func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) {
res := make(ImportsMap, 16)
seen := make(map[FileKind]map[string]struct{}, 16)

for _, file := range pkg.Files {
if !strings.HasSuffix(file.Name, ".gno") {
continue
}
if strings.HasSuffix(file.Name, "_filetest.gno") {
continue

fileKind, err := GetFileKind(file.Name, file.Body, fset)
if err != nil {
return nil, err

Check warning on line 27 in gnovm/pkg/packages/imports.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/packages/imports.go#L27

Added line #L27 was not covered by tests
}
imports, err := FileImports(file.Name, file.Body, fset)
if err != nil {
return nil, err
}
for _, im := range imports {
if _, ok := seen[im.PkgPath]; ok {
if _, ok := seen[fileKind][im.PkgPath]; ok {
continue
}
allImports = append(allImports, im)
seen[im.PkgPath] = struct{}{}
res[fileKind] = append(res[fileKind], im)
if _, ok := seen[fileKind]; !ok {
seen[fileKind] = make(map[string]struct{}, 16)
}
seen[fileKind][im.PkgPath] = struct{}{}
}
}
sort.Slice(allImports, func(i, j int) bool {
return allImports[i].PkgPath < allImports[j].PkgPath
})

return allImports, nil
for _, imports := range res {
sortImports(imports)
}

return res, nil
}

// FileImport represents an import
Expand Down Expand Up @@ -75,3 +81,31 @@
}
return res, nil
}

type ImportsMap map[FileKind][]FileImport

// Merge merges imports, it removes duplicates and sorts the result
func (imap ImportsMap) Merge(kinds ...FileKind) []FileImport {
res := make([]FileImport, 0, 16)
seen := make(map[string]struct{}, 16)

for _, kind := range kinds {
for _, im := range imap[kind] {
if _, ok := seen[im.PkgPath]; ok {
continue
}
seen[im.PkgPath] = struct{}{}

res = append(res, im)
}
}

sortImports(res)
return res
}

func sortImports(imports []FileImport) {
sort.Slice(imports, func(i, j int) bool {
return imports[i].PkgPath < imports[j].PkgPath
})
}
62 changes: 47 additions & 15 deletions gnovm/pkg/packages/imports_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package packages
package packages_test

import (
"os"
"path/filepath"
"testing"

"github.com/gnolang/gno/gnovm/pkg/gnolang"
. "github.com/gnolang/gno/gnovm/pkg/packages"
"github.com/stretchr/testify/require"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

@n0izn0iz n0izn0iz Dec 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw in the go standard libraries sources that they are using dot import for the current package in external tests, thus thought it was the idiomatic go way. I can use a normal import if you prefer

)

Expand Down Expand Up @@ -58,13 +59,26 @@ func TestImports(t *testing.T) {
)
`,
},
{
name: "file2_test.gno",
data: `
package tmp_test

import (
"testing"

"gno.land/p/demo/testpkg"
"gno.land/p/demo/xtestdep"
)
`,
},
{
name: "z_0_filetest.gno",
data: `
package main

import (
"gno.land/p/demo/filetestpkg"
"gno.land/p/demo/filetestdep"
)
`,
},
Expand Down Expand Up @@ -95,17 +109,28 @@ func TestImports(t *testing.T) {
},
}

// Expected list of imports
// Expected lists of imports
// - ignore subdirs
// - ignore duplicate
// - ignore *_filetest.gno
// - should be sorted
expected := []string{
"gno.land/p/demo/pkg1",
"gno.land/p/demo/pkg2",
"gno.land/p/demo/testpkg",
"std",
"testing",
expected := map[FileKind][]string{
FileKindCompiled: {
"gno.land/p/demo/pkg1",
"gno.land/p/demo/pkg2",
"std",
},
FileKindTest: {
"gno.land/p/demo/testpkg",
"testing",
},
FileKindXtest: {
"gno.land/p/demo/testpkg",
"gno.land/p/demo/xtestdep",
"testing",
},
FileKindFiletest: {
"gno.land/p/demo/filetestdep",
},
}

// Create subpkg dir
Expand All @@ -120,12 +145,19 @@ func TestImports(t *testing.T) {

pkg, err := gnolang.ReadMemPackage(tmpDir, "test")
require.NoError(t, err)
imports, err := Imports(pkg, nil)

importsMap, err := Imports(pkg, nil)
require.NoError(t, err)
importsStrings := make([]string, len(imports))
for idx, imp := range imports {
importsStrings[idx] = imp.PkgPath

// ignore specs
got := map[FileKind][]string{}
for key, vals := range importsMap {
stringVals := make([]string, len(vals))
for i, val := range vals {
stringVals[i] = val.PkgPath
}
got[key] = stringVals
}

require.Equal(t, expected, importsStrings)
require.Equal(t, expected, got)
}
3 changes: 2 additions & 1 deletion gnovm/pkg/test/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,11 @@ func LoadImports(store gno.Store, memPkg *gnovm.MemPackage) (err error) {
}()

fset := token.NewFileSet()
imports, err := packages.Imports(memPkg, fset)
importsMap, err := packages.Imports(memPkg, fset)
if err != nil {
return err
}
imports := importsMap.Merge(packages.FileKindCompiled, packages.FileKindTest, packages.FileKindXtest)
for _, imp := range imports {
if gno.IsRealmPath(imp.PkgPath) {
// Don't eagerly load realms.
Expand Down
Loading