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

fix(cmd/gno): only perform preprocessing in lint #3597

Open
wants to merge 3 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
20 changes: 9 additions & 11 deletions gnovm/cmd/gno/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
10 changes: 5 additions & 5 deletions gnovm/cmd/gno/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}(),
Expand Down
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
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 @@
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))

Check warning on line 462 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L462

Added line #L462 was not covered by tests
}
}
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 @@
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)
}

Check warning on line 70 in gnovm/pkg/test/imports.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/test/imports.go#L68-L70

Added lines #L68 - L70 were not covered by tests
}
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 @@
Store: store,
Context: ctx,
})
return m2.RunMemPackage(memPkg, true)
return processMemPackage(m2, memPkg, true)
}
}

Expand Down Expand Up @@ -129,7 +163,7 @@
}

// 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 @@
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 @@
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 @@
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
Loading