Skip to content

Commit

Permalink
fix(cmd/gno): only perform preprocessing in lint (#3597)
Browse files Browse the repository at this point in the history
fixes #3547 

cc @leohhhn
  • Loading branch information
thehowl authored Feb 6, 2025
1 parent 0bc4423 commit 7e1f5b5
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 42 deletions.
2 changes: 1 addition & 1 deletion gnovm/cmd/gno/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 9 additions & 11 deletions gnovm/cmd/gno/tool_lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, false, false)
})
if hasRuntimeErr {
hasError = true
Expand Down Expand Up @@ -221,20 +218,21 @@ 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") {
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
}

// 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)
}
Expand Down
42 changes: 28 additions & 14 deletions gnovm/cmd/gno/tool_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,63 @@ 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=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{"tool", "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"
}(),
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?
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/debugger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions gnovm/pkg/gnolang/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
45 changes: 45 additions & 0 deletions gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,51 @@ func (m *Machine) RunFiles(fns ...*FileNode) {
m.runInitFromUpdates(pv, updates)
}

// 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)
}
// 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.
// This will also run each init function encountered.
// Returns the updated typed values of package.
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/repl/repl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
52 changes: 45 additions & 7 deletions gnovm/pkg/test/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -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), save, false)
}
}
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) {
Expand All @@ -54,7 +88,7 @@ func Store(
Store: store,
Context: ctx,
})
return m2.RunMemPackage(memPkg, true)
return processMemPackage(m2, memPkg, true)
}
}

Expand Down Expand Up @@ -129,7 +163,7 @@ func Store(
}

// Load normal stdlib.
pn, pv = loadStdlib(rootDir, pkgPath, store, stdout)
pn, pv = loadStdlib(rootDir, pkgPath, store, stdout, opts.PreprocessOnly)
if pn != nil {
return
}
Expand All @@ -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
}
Expand All @@ -164,7 +197,7 @@ func Store(
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),
Expand Down Expand Up @@ -202,6 +235,11 @@ 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)
}

Expand Down
5 changes: 1 addition & 4 deletions gnovm/pkg/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions gnovm/tests/integ/init/gno.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module gno.land/r/demo/init
10 changes: 10 additions & 0 deletions gnovm/tests/integ/init/main.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package main

var _ = func() int {
println("HELLO HELLO!!")
return 1
}()

func init() {
println("HELLO WORLD!")
}

0 comments on commit 7e1f5b5

Please sign in to comment.