Skip to content

Commit

Permalink
feat(cmd/gno): re-use teststore in lint (#3315)
Browse files Browse the repository at this point in the history
continuing the work of #3119.

this PR makes `gno lint` re-use the same test.Store, speeding up
execution (especially on the CI, which lints all packages) by not
needing to load all packages for each package that is linted.
  • Loading branch information
thehowl authored Dec 18, 2024
1 parent a7fdd70 commit d69b552
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 78 deletions.
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 @@ -753,15 +753,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 @@ import (
"fmt"
"go/scanner"
"go/types"
"io"
goio "io"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -97,6 +97,11 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {

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 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {
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)

Check failure on line 147 in gnovm/cmd/gno/lint.go

View workflow job for this annotation

GitHub Actions / Run Gno Fmt / fmt

not enough arguments in call to ts.BeginTransaction

Check failure on line 147 in gnovm/cmd/gno/lint.go

View workflow job for this annotation

GitHub Actions / test (1.22.x)

not enough arguments in call to ts.BeginTransaction

Check failure on line 147 in gnovm/cmd/gno/lint.go

View workflow job for this annotation

GitHub Actions / gno2go (1.22.x)

not enough arguments in call to ts.BeginTransaction

// Run type checking
if gmFile == nil || !gmFile.Draft {
foundErr, err := lintTypeCheck(io, memPkg, gs)
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)
}

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

// Check package
Expand Down Expand Up @@ -253,7 +264,7 @@ func guessSourcePath(pkg, source string) string {
// 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 @@ func issueFromError(pkgPath string, err error) lintIssue {
}
return issue
}

type nopReader struct{}

func (nopReader) Read(p []byte) (int, error) { return 0, goio.EOF }
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 @@ func packageImportsRecursive(root string, pkgPath string) []string {
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
}
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 @@ func packageImportsRecursive(root string, pkgPath string) []string {

for _, imp := range sub {
if !slices.Contains(res, imp) {
res = append(res, imp)
res = append(res, imp) //nolint:makezero
}
}
}
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 @@ import (
"io/fs"
"os"
"path/filepath"
"slices"
"strings"

"github.com/gnolang/gno/gnovm"
Expand Down Expand Up @@ -122,16 +121,19 @@ func ListPkgs(root string) (PkgList, error) {
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
}
imports := make([]string, 0, len(importsRaw))
for _, imp := range importsRaw {
// remove self and standard libraries from imports
if imp.PkgPath != gnoMod.Module.Mod.Path &&
!gnolang.IsStdlib(imp.PkgPath) {
imports = append(imports, imp.PkgPath)
}
}

// 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 @@ import (
)

// 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
}
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))
}

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

0 comments on commit d69b552

Please sign in to comment.