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(cmd/gno): re-use teststore in lint #3315

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions gno.land/pkg/integration/testing_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,15 +750,15 @@ 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)
imports, err := packages.Imports(pkg, nil)
if err != nil {
return fmt.Errorf("unable to load package imports in %q: %w", currentPkg.Dir, err)
}
for _, imp := range imports {
if imp == currentPkg.Name || gnolang.IsStdlib(imp) {
if imp.PkgPath == currentPkg.Name || gnolang.IsStdlib(imp.PkgPath) {
continue
}
currentPkg.Imports = append(currentPkg.Imports, imp)
currentPkg.Imports = append(currentPkg.Imports, imp.PkgPath)
}
}

Expand Down
4 changes: 2 additions & 2 deletions gnovm/cmd/gno/download_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ 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)
imports, err := packages.Imports(pkg, nil)
if err != nil {
return fmt.Errorf("read imports at %q: %w", pkgDir, err)
}

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

if !isRemotePkgPath(resolvedPkgPath) {
Expand Down
55 changes: 35 additions & 20 deletions gnovm/cmd/gno/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"fmt"
"go/scanner"
"go/types"
"io"
goio "io"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -97,6 +97,11 @@

hasError := false

bs, ts := test.Store(
rootDir, false,
nopReader{}, goio.Discard, goio.Discard,
)

for _, pkgPath := range pkgPaths {
if verbose {
io.ErrPrintln(pkgPath)
Expand All @@ -120,35 +125,41 @@
hasError = true
}

stdout, stdin, stderr := io.Out(), io.In(), io.Err()
_, testStore := test.Store(
rootDir, false,
stdin, stdout, stderr,
)

memPkg, err := gno.ReadMemPackage(pkgPath, pkgPath)
if err != nil {
io.ErrPrintln(issueFromError(pkgPath, err).String())
hasError = true
continue
}

// Run type checking
if gmFile == nil || !gmFile.Draft {
foundErr, err := lintTypeCheck(io, memPkg, testStore)
if err != nil {
io.ErrPrintln(err)
hasError = true
} else if foundErr {
hasError = true
}
} else if verbose {
io.ErrPrintfln("%s: module is draft, skipping type check", pkgPath)
// Perform imports using the parent store.
if err := test.LoadImports(ts, memPkg); err != nil {
io.ErrPrintln(issueFromError(pkgPath, err).String())
hasError = true
continue
}

// Handle runtime errors
hasRuntimeErr := catchRuntimeError(pkgPath, io.Err(), func() {
tm := test.Machine(testStore, stdout, memPkg.Path)
// Wrap in cache wrap so execution of the linter doesn't impact
// other packages.
cw := bs.CacheWrap()
gs := ts.BeginTransaction(cw, cw)

// Run type checking
if gmFile == nil || !gmFile.Draft {
foundErr, err := lintTypeCheck(io, memPkg, gs)
if err != nil {
io.ErrPrintln(err)
hasError = true

Check warning on line 154 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L153-L154

Added lines #L153 - L154 were not covered by tests
} else if foundErr {
hasError = true
}
} else if verbose {
io.ErrPrintfln("%s: module is draft, skipping type check", pkgPath)
}

Check warning on line 160 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L158-L160

Added lines #L158 - L160 were not covered by tests

tm := test.Machine(gs, goio.Discard, memPkg.Path)
defer tm.Release()

// Check package
Expand Down Expand Up @@ -253,7 +264,7 @@
// XXX: Ideally, error handling should encapsulate location details within a dedicated error type.
var reParseRecover = regexp.MustCompile(`^([^:]+)((?::(?:\d+)){1,2}):? *(.*)$`)

func catchRuntimeError(pkgPath string, stderr io.WriteCloser, action func()) (hasError bool) {
func catchRuntimeError(pkgPath string, stderr goio.WriteCloser, action func()) (hasError bool) {
defer func() {
// Errors catched here mostly come from: gnovm/pkg/gnolang/preprocess.go
r := recover()
Expand Down Expand Up @@ -307,3 +318,7 @@
}
return issue
}

type nopReader struct{}

func (nopReader) Read(p []byte) (int, error) { return 0, goio.EOF }

Check warning on line 324 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L324

Added line #L324 was not covered by tests
5 changes: 1 addition & 4 deletions gnovm/cmd/gno/mod.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,15 +362,12 @@ func getImportToFilesMap(pkgPath string) (map[string][]string, error) {
if err != nil {
return nil, err
}
imports, _, err := packages.FileImports(filename, string(data))
imports, err := packages.FileImports(filename, string(data), nil)
if err != nil {
return nil, err
}

for _, imp := range imports {
if imp.Error != nil {
return nil, err
}
m[imp.PkgPath] = append(m[imp.PkgPath], filename)
}
}
Expand Down
1 change: 0 additions & 1 deletion gnovm/cmd/gno/testdata/lint/bad_import.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ module gno.land/p/test

-- stdout.golden --
-- stderr.golden --
bad_file.gno:3:8: could not import python (import not found: python) (code=4)
bad_file.gno:3:8: unknown import path python (code=2)
10 changes: 7 additions & 3 deletions gnovm/pkg/doc/dirs.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,14 @@
pkg = &gnovm.MemPackage{}
}

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

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/doc/dirs.go#L111-L112

Added lines #L111 - L112 were not covered by tests
res := make([]string, len(resRaw))
for idx, imp := range resRaw {
res[idx] = imp.PkgPath
}

entries, err := os.ReadDir(root)
Expand All @@ -127,7 +131,7 @@

for _, imp := range sub {
if !slices.Contains(res, imp) {
res = append(res, imp)
res = append(res, imp) //nolint:makezero

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/doc/dirs.go#L134

Added line #L134 was not covered by tests
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions gnovm/pkg/gnomod/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"io/fs"
"os"
"path/filepath"
"slices"
"strings"

"github.com/gnolang/gno/gnovm"
Expand Down Expand Up @@ -122,16 +121,19 @@
pkg = &gnovm.MemPackage{}
}

imports, err := packages.Imports(pkg)
importsRaw, err := packages.Imports(pkg, nil)
if err != nil {
// ignore imports on error
imports = []string{}
importsRaw = 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
}
imports := make([]string, 0, len(importsRaw))
for _, imp := range importsRaw {
// remove self and standard libraries from imports

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnomod/pkg.go#L131

Added line #L131 was not covered by tests
if imp.PkgPath != gnoMod.Module.Mod.Path &&
!gnolang.IsStdlib(imp.PkgPath) {
imports = append(imports, imp.PkgPath)
}

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnomod/pkg.go#L133-L135

Added lines #L133 - L135 were not covered by tests
}

// remove self and standard libraries from imports
imports = slices.DeleteFunc(imports, func(imp string) bool {
return imp == gnoMod.Module.Mod.Path || gnolang.IsStdlib(imp)
})

pkgs = append(pkgs, Pkg{
Dir: path,
Expand Down
47 changes: 26 additions & 21 deletions gnovm/pkg/packages/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,60 +13,65 @@
)

// Imports returns the list of gno imports from a [gnovm.MemPackage].
func Imports(pkg *gnovm.MemPackage) ([]string, error) {
allImports := make([]string, 0)
seen := make(map[string]struct{})
// fset is optional.
func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) ([]FileImport, error) {
allImports := make([]FileImport, 0, 16)
seen := make(map[string]struct{}, 16)
for _, file := range pkg.Files {
if !strings.HasSuffix(file.Name, ".gno") {
continue
}
if strings.HasSuffix(file.Name, "_filetest.gno") {
continue
}
imports, _, err := FileImports(file.Name, file.Body)
imports, err := FileImports(file.Name, file.Body, fset)
if err != nil {
return nil, err
}
for _, im := range imports {
if im.Error != nil {
return nil, err
}
if _, ok := seen[im.PkgPath]; ok {
continue
}
allImports = append(allImports, im.PkgPath)
allImports = append(allImports, im)
seen[im.PkgPath] = struct{}{}
}
}
sort.Strings(allImports)
sort.Slice(allImports, func(i, j int) bool {
return allImports[i].PkgPath < allImports[j].PkgPath
})

return allImports, nil
}

// FileImport represents an import
type FileImport struct {
PkgPath string
Spec *ast.ImportSpec
Error error
}

// FileImports returns the list of gno imports in the given file src.
// The given filename is only used when recording position information.
func FileImports(filename string, src string) ([]*FileImport, *token.FileSet, error) {
fs := token.NewFileSet()
f, err := parser.ParseFile(fs, filename, src, parser.ImportsOnly)
func FileImports(filename string, src string, fset *token.FileSet) ([]FileImport, error) {
if fset == nil {
fset = token.NewFileSet()
}
f, err := parser.ParseFile(fset, filename, src, parser.ImportsOnly)
if err != nil {
return nil, nil, err
return nil, err

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/packages/imports.go#L60

Added line #L60 was not covered by tests
}
res := make([]*FileImport, len(f.Imports))
res := make([]FileImport, len(f.Imports))
for i, im := range f.Imports {
fi := FileImport{Spec: im}
importPath, err := strconv.Unquote(im.Path.Value)
if err != nil {
fi.Error = fmt.Errorf("%v: unexpected invalid import path: %v", fs.Position(im.Pos()).String(), im.Path.Value)
} else {
fi.PkgPath = importPath
// should not happen - parser.ParseFile should already ensure we get
// a valid string literal here.
panic(fmt.Errorf("%v: unexpected invalid import path: %v", fset.Position(im.Pos()).String(), im.Path.Value))

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/packages/imports.go#L66-L68

Added lines #L66 - L68 were not covered by tests
}

res[i] = FileImport{
PkgPath: importPath,
Spec: im,
}
res[i] = &fi
}
return res, fs, nil
return res, nil
}
8 changes: 6 additions & 2 deletions gnovm/pkg/packages/imports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,12 @@ func TestImports(t *testing.T) {

pkg, err := gnolang.ReadMemPackage(tmpDir, "test")
require.NoError(t, err)
imports, err := Imports(pkg)
imports, err := Imports(pkg, nil)
require.NoError(t, err)
importsStrings := make([]string, len(imports))
for idx, imp := range imports {
importsStrings[idx] = imp.PkgPath
}

require.Equal(t, expected, imports)
require.Equal(t, expected, importsStrings)
}
11 changes: 9 additions & 2 deletions gnovm/pkg/test/filetest.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,20 @@ type runResult struct {
}

func (opts *TestOptions) runTest(m *gno.Machine, pkgPath, filename string, content []byte) (rr runResult) {
pkgName := gno.Name(pkgPath[strings.LastIndexByte(pkgPath, '/')+1:])

// Eagerly load imports.
// This is executed using opts.Store, rather than the transaction store;
// it allows us to only have to load the imports once (and re-use the cached
// versions). Running the tests in separate "transactions" means that they
// don't get the parent store dirty.
if err := LoadImports(opts.TestStore, filename, content); err != nil {
if err := LoadImports(opts.TestStore, &gnovm.MemPackage{
Name: string(pkgName),
Path: pkgPath,
Files: []*gnovm.MemFile{
{Name: filename, Body: string(content)},
},
}); err != nil {
// NOTE: we perform this here, so we can capture the runResult.
return runResult{Error: err.Error()}
}
Expand Down Expand Up @@ -210,7 +218,6 @@ func (opts *TestOptions) runTest(m *gno.Machine, pkgPath, filename string, conte
}()

// Use last element after / (works also if slash is missing).
pkgName := gno.Name(pkgPath[strings.LastIndexByte(pkgPath, '/')+1:])
if !gno.IsRealmPath(pkgPath) {
// Simple case - pure package.
pn := gno.NewPackageNode(pkgName, pkgPath, &gno.FileSet{})
Expand Down
14 changes: 7 additions & 7 deletions gnovm/pkg/test/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"go/token"
"io"
"math/big"
"os"
Expand All @@ -12,6 +13,7 @@ import (
"strings"
"time"

"github.com/gnolang/gno/gnovm"
gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
"github.com/gnolang/gno/gnovm/pkg/packages"
teststdlibs "github.com/gnolang/gno/gnovm/tests/stdlibs"
Expand Down Expand Up @@ -214,13 +216,13 @@ func (e *stackWrappedError) String() string {
return fmt.Sprintf("%v\nstack:\n%v", e.err, string(e.stack))
}

// LoadImports parses the given file and attempts to retrieve all pure packages
// LoadImports parses the given MemPackage and attempts to retrieve all pure packages
// from the store. This is mostly useful for "eager import loading", whereby all
// imports are pre-loaded in a permanent store, so that the tests can use
// ephemeral transaction stores.
func LoadImports(store gno.Store, filename string, content []byte) (err error) {
func LoadImports(store gno.Store, memPkg *gnovm.MemPackage) (err error) {
defer func() {
// This is slightly different from the handling below; we do not have a
// This is slightly different from other similar error handling; we do not have a
// machine to work with, as this comes from an import; so we need
// "machine-less" alternatives. (like v.String instead of v.Sprint)
if r := recover(); r != nil {
Expand All @@ -239,14 +241,12 @@ func LoadImports(store gno.Store, filename string, content []byte) (err error) {
}
}()

imports, fset, err := packages.FileImports(filename, string(content))
fset := token.NewFileSet()
imports, err := packages.Imports(memPkg, fset)
if err != nil {
return err
}
for _, imp := range imports {
if imp.Error != nil {
return imp.Error
}
if gno.IsRealmPath(imp.PkgPath) {
// Don't eagerly load realms.
// Realms persist state and can change the state of other realms in initialization.
Expand Down
Loading
Loading