From 8ee370f7f22c92e61aff33da9e58377a52a8c9df Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Thu, 23 Jan 2025 19:41:22 +0100 Subject: [PATCH 1/4] fix(cmd/gno): only perform preprocessing in lint --- gnovm/cmd/gno/lint.go | 12 +++++------- gnovm/cmd/gno/lint_test.go | 10 +++++----- gnovm/pkg/gnolang/machine.go | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index ce3465b484e..5f25bc08112 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -162,13 +162,10 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { tm := test.Machine(gs, goio.Discard, memPkg.Path) defer tm.Release() - // Check package - tm.RunMemPackage(memPkg, true) - // Check test files - testFiles := lintTestFiles(memPkg) + packageFiles := sourceAndTestFileset(memPkg) - tm.RunFiles(testFiles.Files...) + tm.PreprocessFiles(memPkg.Name, memPkg.Path, packageFiles) }) if hasRuntimeErr { hasError = true @@ -221,7 +218,7 @@ func lintTypeCheck(io commands.IO, memPkg *gnovm.MemPackage, testStore gno.Store return true, nil } -func lintTestFiles(memPkg *gnovm.MemPackage) *gno.FileSet { +func sourceAndTestFileset(memPkg *gnovm.MemPackage) *gno.FileSet { testfiles := &gno.FileSet{} for _, mfile := range memPkg.Files { if !strings.HasSuffix(mfile.Name, ".gno") { @@ -234,7 +231,8 @@ func lintTestFiles(memPkg *gnovm.MemPackage) *gno.FileSet { } // XXX: package ending with `_test` is not supported yet - if strings.HasSuffix(mfile.Name, "_test.gno") && !strings.HasSuffix(string(n.PkgName), "_test") { + if !strings.HasSuffix(mfile.Name, "_filetest.gno") && + !strings.HasSuffix(string(n.PkgName), "_test") { // Keep only test files testfiles.AddFiles(n) } diff --git a/gnovm/cmd/gno/lint_test.go b/gnovm/cmd/gno/lint_test.go index 4589fc55f92..933bb2f02f0 100644 --- a/gnovm/cmd/gno/lint_test.go +++ b/gnovm/cmd/gno/lint_test.go @@ -24,16 +24,16 @@ func TestLintApp(t *testing.T) { errShouldBe: "exit code: 1", }, { args: []string{"lint", "../../tests/integ/several-lint-errors/main.gno"}, - stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5:5: expected ';', found example (code=2)\n../../tests/integ/several-lint-errors/main.gno:6", + stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5:5: expected ';', found example (code=3)\n../../tests/integ/several-lint-errors/main.gno:6", errShouldBe: "exit code: 1", }, { args: []string{"lint", "../../tests/integ/several-files-multiple-errors/main.gno"}, stderrShouldContain: func() string { lines := []string{ - "../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=2)", - "../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=2)", - "../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=2)", - "../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=2)", + "../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=3)", + "../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=3)", + "../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=3)", + "../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=3)", } return strings.Join(lines, "\n") + "\n" }(), diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index 75d12ac5402..d5103f49f81 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -454,6 +454,20 @@ func (m *Machine) RunFiles(fns ...*FileNode) { m.runInitFromUpdates(pv, updates) } +// PreprocessFiles runs Preprocess on the given files, without saving +// them to the store. It is used to detect compile-time errors in the package. +func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet) { + if err := checkDuplicates(fset); err != nil { + panic(fmt.Errorf("running package %q: %w", pkgName, err)) + } + pn := NewPackageNode(Name(pkgName), pkgPath, fset) + m.Store.SetBlockNode(pn) + PredefineFileSet(m.Store, pn, fset) + for _, fn := range fset.Files { + fn = Preprocess(m.Store, pn, fn).(*FileNode) + } +} + // Add files to the package's *FileSet and run decls in them. // This will also run each init function encountered. // Returns the updated typed values of package. From c36364d99f498913450cfb92bed0cabe51160951 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Thu, 23 Jan 2025 20:03:38 +0100 Subject: [PATCH 2/4] make it work also in test imports --- gnovm/cmd/gno/lint.go | 6 ++-- gnovm/cmd/gno/run.go | 2 +- gnovm/pkg/gnolang/debugger_test.go | 2 +- gnovm/pkg/gnolang/files_test.go | 6 ++-- gnovm/pkg/gnolang/machine.go | 3 +- gnovm/pkg/repl/repl.go | 2 +- gnovm/pkg/test/imports.go | 44 ++++++++++++++++++++++++++---- gnovm/pkg/test/test.go | 5 +--- 8 files changed, 51 insertions(+), 19 deletions(-) diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index 5f25bc08112..9f81f663d32 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -97,9 +97,9 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { hasError := false - bs, ts := test.Store( - rootDir, false, - nopReader{}, goio.Discard, goio.Discard, + bs, ts := test.StoreWithOptions( + rootDir, nopReader{}, goio.Discard, goio.Discard, + test.StoreOptions{PreprocessOnly: true}, ) for _, pkgPath := range pkgPaths { diff --git a/gnovm/cmd/gno/run.go b/gnovm/cmd/gno/run.go index 9a9beac5cd1..b4fe934661a 100644 --- a/gnovm/cmd/gno/run.go +++ b/gnovm/cmd/gno/run.go @@ -93,7 +93,7 @@ func execRun(cfg *runCfg, args []string, io commands.IO) error { // init store and machine _, testStore := test.Store( - cfg.rootDir, false, + cfg.rootDir, stdin, stdout, stderr) if cfg.verbose { testStore.SetLogStoreOps(true) diff --git a/gnovm/pkg/gnolang/debugger_test.go b/gnovm/pkg/gnolang/debugger_test.go index 926ff0595e6..a9e0a4834d5 100644 --- a/gnovm/pkg/gnolang/debugger_test.go +++ b/gnovm/pkg/gnolang/debugger_test.go @@ -39,7 +39,7 @@ func evalTest(debugAddr, in, file string) (out, err string) { err = strings.TrimSpace(strings.ReplaceAll(err, "../../tests/files/", "files/")) }() - _, testStore := test.Store(gnoenv.RootDir(), false, stdin, stdout, stderr) + _, testStore := test.Store(gnoenv.RootDir(), stdin, stdout, stderr) f := gnolang.MustReadFile(file) diff --git a/gnovm/pkg/gnolang/files_test.go b/gnovm/pkg/gnolang/files_test.go index 2c82f6d8f29..31f04087855 100644 --- a/gnovm/pkg/gnolang/files_test.go +++ b/gnovm/pkg/gnolang/files_test.go @@ -45,9 +45,9 @@ func TestFiles(t *testing.T) { Error: io.Discard, Sync: *withSync, } - o.BaseStore, o.TestStore = test.Store( - rootDir, true, - nopReader{}, o.WriterForStore(), io.Discard, + o.BaseStore, o.TestStore = test.StoreWithOptions( + rootDir, nopReader{}, o.WriterForStore(), io.Discard, + test.StoreOptions{WithExtern: true}, ) return o } diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index d5103f49f81..92ceb2a2f0b 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -456,7 +456,7 @@ func (m *Machine) RunFiles(fns ...*FileNode) { // PreprocessFiles runs Preprocess on the given files, without saving // them to the store. It is used to detect compile-time errors in the package. -func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet) { +func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet) (*PackageNode, *PackageValue) { if err := checkDuplicates(fset); err != nil { panic(fmt.Errorf("running package %q: %w", pkgName, err)) } @@ -466,6 +466,7 @@ func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet) { for _, fn := range fset.Files { fn = Preprocess(m.Store, pn, fn).(*FileNode) } + return pn, pn.NewPackage() } // Add files to the package's *FileSet and run decls in them. diff --git a/gnovm/pkg/repl/repl.go b/gnovm/pkg/repl/repl.go index b0944d21646..fff80d672dc 100644 --- a/gnovm/pkg/repl/repl.go +++ b/gnovm/pkg/repl/repl.go @@ -125,7 +125,7 @@ func NewRepl(opts ...ReplOption) *Repl { r.stderr = &b r.storeFunc = func() gno.Store { - _, st := test.Store(gnoenv.RootDir(), false, r.stdin, r.stdout, r.stderr) + _, st := test.Store(gnoenv.RootDir(), r.stdin, r.stdout, r.stderr) return st } diff --git a/gnovm/pkg/test/imports.go b/gnovm/pkg/test/imports.go index a8dd709e501..1b3359f2055 100644 --- a/gnovm/pkg/test/imports.go +++ b/gnovm/pkg/test/imports.go @@ -25,22 +25,56 @@ import ( storetypes "github.com/gnolang/gno/tm2/pkg/store/types" ) +type StoreOptions struct { + // WithExtern interprets imports of packages under "github.com/gnolang/gno/_test/" + // as imports under the directory in gnovm/tests/files/extern. + // This should only be used for GnoVM internal filetests (gnovm/tests/files). + WithExtern bool + + // PreprocessOnly instructs the PackageGetter to run the imported files using + // [gno.Machine.PreprocessFiles]. It avoids executing code for contexts + // which only intend to perform a type check, ie. `gno lint`. + PreprocessOnly bool +} + // NOTE: this isn't safe, should only be used for testing. func Store( rootDir string, - withExtern bool, stdin io.Reader, stdout, stderr io.Writer, ) ( baseStore storetypes.CommitStore, resStore gno.Store, ) { + return StoreWithOptions(rootDir, stdin, stdout, stderr, StoreOptions{}) +} + +// StoreWithOptions is a variant of [Store] which additionally accepts a +// [StoreOptions] argument. +func StoreWithOptions( + rootDir string, + stdin io.Reader, + stdout, stderr io.Writer, + opts StoreOptions, +) ( + baseStore storetypes.CommitStore, + resStore gno.Store, +) { + processMemPackage := func(m *gno.Machine, memPkg *gnovm.MemPackage, save bool) (*gno.PackageNode, *gno.PackageValue) { + return m.RunMemPackage(memPkg, save) + } + if opts.PreprocessOnly { + processMemPackage = func(m *gno.Machine, memPkg *gnovm.MemPackage, save bool) (*gno.PackageNode, *gno.PackageValue) { + m.Store.AddMemPackage(memPkg) + return m.PreprocessFiles(memPkg.Name, memPkg.Path, gno.ParseMemPackage(memPkg)) + } + } getPackage := func(pkgPath string, store gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) { if pkgPath == "" { panic(fmt.Sprintf("invalid zero package path in testStore().pkgGetter")) } - if withExtern { + if opts.WithExtern { // if _test package... const testPath = "github.com/gnolang/gno/_test/" if strings.HasPrefix(pkgPath, testPath) { @@ -54,7 +88,7 @@ func Store( Store: store, Context: ctx, }) - return m2.RunMemPackage(memPkg, true) + return processMemPackage(m2, memPkg, true) } } @@ -150,8 +184,7 @@ func Store( Store: store, Context: ctx, }) - pn, pv = m2.RunMemPackage(memPkg, true) - return + return processMemPackage(m2, memPkg, true) } return nil, nil } @@ -202,6 +235,7 @@ func loadStdlib(rootDir, pkgPath string, store gno.Store, stdout io.Writer) (*gn Output: stdout, Store: store, }) + // TODO: make this work when using gno lint. return m2.RunMemPackageWithOverrides(memPkg, true) } diff --git a/gnovm/pkg/test/test.go b/gnovm/pkg/test/test.go index d06540761d7..92a867e1886 100644 --- a/gnovm/pkg/test/test.go +++ b/gnovm/pkg/test/test.go @@ -139,10 +139,7 @@ func NewTestOptions(rootDir string, stdin io.Reader, stdout, stderr io.Writer) * Output: stdout, Error: stderr, } - opts.BaseStore, opts.TestStore = Store( - rootDir, false, - stdin, opts.WriterForStore(), stderr, - ) + opts.BaseStore, opts.TestStore = Store(rootDir, stdin, opts.WriterForStore(), stderr) return opts } From b0f1f5342c872045ca1138e2f76b7c2662176f06 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Thu, 23 Jan 2025 20:34:33 +0100 Subject: [PATCH 3/4] make it work also in imports --- gnovm/cmd/gno/lint.go | 4 ++-- gnovm/pkg/gnolang/machine.go | 42 ++++++++++++++++++++++++++++++------ gnovm/pkg/test/imports.go | 10 ++++++--- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index 9f81f663d32..6983175cea0 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -165,7 +165,7 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error { // Check test files packageFiles := sourceAndTestFileset(memPkg) - tm.PreprocessFiles(memPkg.Name, memPkg.Path, packageFiles) + tm.PreprocessFiles(memPkg.Name, memPkg.Path, packageFiles, false, false) }) if hasRuntimeErr { hasError = true @@ -225,7 +225,7 @@ func sourceAndTestFileset(memPkg *gnovm.MemPackage) *gno.FileSet { continue // Skip non-GNO files } - n, _ := gno.ParseFile(mfile.Name, mfile.Body) + n := gno.MustParseFile(mfile.Name, mfile.Body) if n == nil { continue // Skip empty files } diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index 92ceb2a2f0b..f7d2cf10f2c 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -454,19 +454,49 @@ func (m *Machine) RunFiles(fns ...*FileNode) { m.runInitFromUpdates(pv, updates) } -// PreprocessFiles runs Preprocess on the given files, without saving -// them to the store. It is used to detect compile-time errors in the package. -func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet) (*PackageNode, *PackageValue) { - if err := checkDuplicates(fset); err != nil { - panic(fmt.Errorf("running package %q: %w", pkgName, err)) +// PreprocessFiles runs Preprocess on the given files. It is used to detect +// compile-time errors in the package. +func (m *Machine) PreprocessFiles(pkgName, pkgPath string, fset *FileSet, save, withOverrides bool) (*PackageNode, *PackageValue) { + if !withOverrides { + if err := checkDuplicates(fset); err != nil { + panic(fmt.Errorf("running package %q: %w", pkgName, err)) + } } pn := NewPackageNode(Name(pkgName), pkgPath, fset) + pv := pn.NewPackage() + pb := pv.GetBlock(m.Store) + m.SetActivePackage(pv) m.Store.SetBlockNode(pn) PredefineFileSet(m.Store, pn, fset) for _, fn := range fset.Files { fn = Preprocess(m.Store, pn, fn).(*FileNode) + // After preprocessing, save blocknodes to store. + SaveBlockNodes(m.Store, fn) + // Make block for fn. + // Each file for each *PackageValue gets its own file *Block, + // with values copied over from each file's + // *FileNode.StaticBlock. + fb := m.Alloc.NewBlock(fn, pb) + fb.Values = make([]TypedValue, len(fn.StaticBlock.Values)) + copy(fb.Values, fn.StaticBlock.Values) + pv.AddFileBlock(fn.Name, fb) } - return pn, pn.NewPackage() + // Get new values across all files in package. + pn.PrepareNewValues(pv) + // save package value. + var throwaway *Realm + if save { + // store new package values and types + throwaway = m.saveNewPackageValuesAndTypes() + if throwaway != nil { + m.Realm = throwaway + } + m.resavePackageValues(throwaway) + if throwaway != nil { + m.Realm = nil + } + } + return pn, pv } // Add files to the package's *FileSet and run decls in them. diff --git a/gnovm/pkg/test/imports.go b/gnovm/pkg/test/imports.go index 1b3359f2055..95302ecffb0 100644 --- a/gnovm/pkg/test/imports.go +++ b/gnovm/pkg/test/imports.go @@ -66,7 +66,7 @@ func StoreWithOptions( if opts.PreprocessOnly { processMemPackage = func(m *gno.Machine, memPkg *gnovm.MemPackage, save bool) (*gno.PackageNode, *gno.PackageValue) { m.Store.AddMemPackage(memPkg) - return m.PreprocessFiles(memPkg.Name, memPkg.Path, gno.ParseMemPackage(memPkg)) + return m.PreprocessFiles(memPkg.Name, memPkg.Path, gno.ParseMemPackage(memPkg), save, false) } } getPackage := func(pkgPath string, store gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) { @@ -163,7 +163,7 @@ func StoreWithOptions( } // Load normal stdlib. - pn, pv = loadStdlib(rootDir, pkgPath, store, stdout) + pn, pv = loadStdlib(rootDir, pkgPath, store, stdout, opts.PreprocessOnly) if pn != nil { return } @@ -197,7 +197,7 @@ func StoreWithOptions( return } -func loadStdlib(rootDir, pkgPath string, store gno.Store, stdout io.Writer) (*gno.PackageNode, *gno.PackageValue) { +func loadStdlib(rootDir, pkgPath string, store gno.Store, stdout io.Writer, preprocessOnly bool) (*gno.PackageNode, *gno.PackageValue) { dirs := [...]string{ // Normal stdlib path. filepath.Join(rootDir, "gnovm", "stdlibs", pkgPath), @@ -235,6 +235,10 @@ func loadStdlib(rootDir, pkgPath string, store gno.Store, stdout io.Writer) (*gn Output: stdout, Store: store, }) + if preprocessOnly { + m2.Store.AddMemPackage(memPkg) + return m2.PreprocessFiles(memPkg.Name, memPkg.Path, gno.ParseMemPackage(memPkg), true, true) + } // TODO: make this work when using gno lint. return m2.RunMemPackageWithOverrides(memPkg, true) } From 34222b98227a977adcadffab52f80924d0c1885a Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Thu, 6 Feb 2025 14:39:37 +0100 Subject: [PATCH 4/4] add regression test --- gnovm/cmd/gno/tool_lint_test.go | 32 +++++++++++++++++++++++--------- gnovm/tests/integ/init/gno.mod | 1 + gnovm/tests/integ/init/main.gno | 10 ++++++++++ 3 files changed, 34 insertions(+), 9 deletions(-) create mode 100644 gnovm/tests/integ/init/gno.mod create mode 100644 gnovm/tests/integ/init/main.gno diff --git a/gnovm/cmd/gno/tool_lint_test.go b/gnovm/cmd/gno/tool_lint_test.go index 4040215c4d5..3f9e5cd59ba 100644 --- a/gnovm/cmd/gno/tool_lint_test.go +++ b/gnovm/cmd/gno/tool_lint_test.go @@ -10,23 +10,28 @@ func TestLintApp(t *testing.T) { { args: []string{"tool", "lint"}, errShouldBe: "flag: help requested", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/run_main/"}, stderrShouldContain: "./../../tests/integ/run_main: gno.mod file not found in current or any parent directory (code=1)", errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/undefined_variable_test/undefined_variables_test.gno"}, stderrShouldContain: "undefined_variables_test.gno:6:28: name toto not declared (code=2)", errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/package_not_declared/main.gno"}, stderrShouldContain: "main.gno:4:2: name fmt not declared (code=2)", errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/several-lint-errors/main.gno"}, stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5:5: expected ';', found example (code=3)\n../../tests/integ/several-lint-errors/main.gno:6", errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/several-files-multiple-errors/main.gno"}, stderrShouldContain: func() string { lines := []string{ @@ -38,21 +43,30 @@ func TestLintApp(t *testing.T) { return strings.Join(lines, "\n") + "\n" }(), errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/minimalist_gnomod/"}, // TODO: raise an error because there is a gno.mod, but no .gno files - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/invalid_module_name/"}, // TODO: raise an error because gno.mod is invalid - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/invalid_gno_file/"}, stderrShouldContain: "../../tests/integ/invalid_gno_file/invalid.gno:1:1: expected 'package', found packag (code=2)", errShouldBe: "exit code: 1", - }, { + }, + { args: []string{"tool", "lint", "../../tests/integ/typecheck_missing_return/"}, stderrShouldContain: "../../tests/integ/typecheck_missing_return/main.gno:5:1: missing return (code=4)", errShouldBe: "exit code: 1", }, + { + args: []string{"tool", "lint", "../../tests/integ/init/"}, + // stderr / stdout should be empty; the init function and statements + // should not be executed + }, // TODO: 'gno mod' is valid? // TODO: are dependencies valid? diff --git a/gnovm/tests/integ/init/gno.mod b/gnovm/tests/integ/init/gno.mod new file mode 100644 index 00000000000..28c7e51b750 --- /dev/null +++ b/gnovm/tests/integ/init/gno.mod @@ -0,0 +1 @@ +module gno.land/r/demo/init diff --git a/gnovm/tests/integ/init/main.gno b/gnovm/tests/integ/init/main.gno new file mode 100644 index 00000000000..88cfafb9f24 --- /dev/null +++ b/gnovm/tests/integ/init/main.gno @@ -0,0 +1,10 @@ +package main + +var _ = func() int { + println("HELLO HELLO!!") + return 1 +}() + +func init() { + println("HELLO WORLD!") +}