Skip to content

Commit

Permalink
fix: unify the precompile output to make it easier to parse and read. (
Browse files Browse the repository at this point in the history
…gnolang#1670)

<!-- please provide a detailed description of the changes made in this
pull request. -->
Fix gnolang#1636 

Under the hood, use `scanner.ErrorList` from the stdlib all the way to
store all errors with the filename and the position in the file. Note
that kind of error is already returned by `parser.ParserFile`.

For the `go build` output, instead of printing it like it is, parse it
and shift it to represent the related gno file.

Run the tests:
```
$ go test ./gnovm/cmd/gno -v -run Test_ScriptsPrecompile
```

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
tbruyelle authored and leohhhn committed Feb 29, 2024
1 parent 15b10ca commit 0ae1876
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 55 deletions.
46 changes: 25 additions & 21 deletions gnovm/cmd/gno/precompile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package main

import (
"context"
"errors"
"flag"
"fmt"
"go/scanner"
"log"
"os"
"path/filepath"
Expand Down Expand Up @@ -132,41 +134,43 @@ func execPrecompile(cfg *precompileCfg, args []string, io commands.IO) error {
}

opts := newPrecompileOptions(cfg)
errCount := 0
var errlist scanner.ErrorList
for _, filepath := range paths {
err = precompileFile(filepath, opts)
if err != nil {
err = fmt.Errorf("%s: precompile: %w", filepath, err)
io.ErrPrintfln("%s", err.Error())
errCount++
if err := precompileFile(filepath, opts); err != nil {
var fileErrlist scanner.ErrorList
if !errors.As(err, &fileErrlist) {
// Not an scanner.ErrorList: return immediately.
return fmt.Errorf("%s: precompile: %w", filepath, err)
}
errlist = append(errlist, fileErrlist...)
}
}

if errCount > 0 {
return fmt.Errorf("%d precompile errors", errCount)
}

if cfg.gobuild {
if errlist.Len() == 0 && cfg.gobuild {
paths, err := gnoPackagesFromArgs(args)
if err != nil {
return fmt.Errorf("list packages: %w", err)
}

errCount = 0
for _, pkgPath := range paths {
_ = pkgPath
err = goBuildFileOrPkg(pkgPath, cfg)
err := goBuildFileOrPkg(pkgPath, cfg)
if err != nil {
err = fmt.Errorf("%s: build pkg: %w", pkgPath, err)
io.ErrPrintfln("%s\n", err.Error())
errCount++
var fileErrlist scanner.ErrorList
if !errors.As(err, &fileErrlist) {
// Not an scanner.ErrorList: return immediately.
return fmt.Errorf("%s: build: %w", pkgPath, err)
}
errlist = append(errlist, fileErrlist...)
}
}
if errCount > 0 {
return fmt.Errorf("%d build errors", errCount)
}
}

if errlist.Len() > 0 {
for _, err := range errlist {
io.ErrPrintfln(err.Error())
}
return fmt.Errorf("%d precompile error(s)", errlist.Len())
}
return nil
}

Expand Down Expand Up @@ -213,7 +217,7 @@ func precompileFile(srcPath string, opts *precompileOptions) error {
// preprocess.
precompileRes, err := gno.Precompile(string(source), tags, srcPath)
if err != nil {
return fmt.Errorf("%w", err)
return fmt.Errorf("precompile: %w", err)
}

// resolve target path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
! gno precompile .

! stdout .+
stderr 'precompile: parse: main.gno:3:1: expected declaration, found invalid'
stderr 'precompile: parse: sub/sub.gno:3:1: expected declaration, found invalid'
stderr '^main.gno:3:1: expected declaration, found invalid$'
stderr '^main.gno:7:2: expected declaration, found wrong$'
stderr '^sub/sub.gno:3:1: expected declaration, found invalid$'
stderr '^3 precompile error\(s\)$'

# no *.gen.go files are created
! exec test -f main.gno.gen.go
Expand All @@ -15,8 +17,10 @@ package main

invalid

func main() {}

func main() {
var x = 1
wrong
}
-- sub/sub.gno --
package sub

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
! gno precompile -gobuild .

! stdout .+
stderr './main.gno.gen.go:8:6: x declared and not used'
stderr '^./main.gno:4:6: x declared and not used$'
stderr '^./main.gno:5:6: y declared and not used$'
stderr '^2 precompile error\(s\)$'

cmp main.gno.gen.go main.gno.gen.go.golden
cmp sub/sub.gno.gen.go sub/sub.gno.gen.go.golden
Expand All @@ -13,6 +15,7 @@ package main

func main() {
var x = 1
var y = 2
}
-- sub/sub.gno --
package sub
Expand All @@ -25,6 +28,7 @@ package main

func main() {
var x = 1
var y = 2
}
-- sub/sub.gno.gen.go.golden --
// Code generated by github.com/gnolang/gno. DO NOT EDIT.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
! gno precompile -gobuild .

! stdout .+
stderr 'main.gno: precompile: parse: main.gno:3:1: expected declaration, found invalid'
stderr '^main.gno:3:1: expected declaration, found invalid$'

# no *.gen.go files are created
! exec test -f main.gno.gen.go
Expand All @@ -12,3 +12,7 @@ stderr 'main.gno: precompile: parse: main.gno:3:1: expected declaration, found i
package main

invalid

func main() {
var x = 1
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
! gno precompile .

! stdout .+
stderr 'main.gno: precompile: import "xxx" is not in the whitelist'
stderr 'sub/sub.gno: precompile: import "xxx" is not in the whitelist'
stderr '^main.gno:5:2: import "xxx" is not in the whitelist$'
stderr '^sub/sub.gno:3:8: import "xxx" is not in the whitelist$'
stderr '^2 precompile error\(s\)$'

# no *.gen.go files are created
! exec test -f main.gno.gen.go
Expand Down
78 changes: 58 additions & 20 deletions gnovm/pkg/gnolang/precompile.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import (
"go/ast"
"go/format"
"go/parser"
goscanner "go/scanner"
"go/token"
"os"
"os/exec"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"

"go.uber.org/multierr"
Expand Down Expand Up @@ -148,8 +151,6 @@ func PrecompileAndCheckMempkg(mempkg *std.MemPackage) error {
}

func Precompile(source string, tags string, filename string) (*precompileResult, error) {
var out bytes.Buffer

fset := token.NewFileSet()
f, err := parser.ParseFile(fset, filename, source, parser.ParseComments)
if err != nil {
Expand All @@ -161,18 +162,19 @@ func Precompile(source string, tags string, filename string) (*precompileResult,

transformed, err := precompileAST(fset, f, shouldCheckWhitelist)
if err != nil {
return nil, fmt.Errorf("%w", err)
return nil, fmt.Errorf("precompileAST: %w", err)
}

header := "// Code generated by github.com/gnolang/gno. DO NOT EDIT.\n\n"
if tags != "" {
header += "//go:build " + tags + "\n\n"
}
_, err = out.WriteString(header)
var out bytes.Buffer
out.WriteString(header)
err = format.Node(&out, fset, transformed)
if err != nil {
return nil, fmt.Errorf("write to buffer: %w", err)
return nil, fmt.Errorf("format.Node: %w", err)
}
err = format.Node(&out, fset, transformed)

res := &precompileResult{
Imports: f.Imports,
Expand Down Expand Up @@ -202,7 +204,7 @@ func PrecompileVerifyFile(path string, gofmtBinary string) error {
//
// This method is the most efficient to detect errors but requires that
// all the import are valid and available.
func PrecompileBuildPackage(fileOrPkg string, goBinary string) error {
func PrecompileBuildPackage(fileOrPkg, goBinary string) error {
// TODO: use cmd/compile instead of exec?
// TODO: find the nearest go.mod file, chdir in the same folder, rim prefix?
// TODO: temporarily create an in-memory go.mod or disable go modules for gno?
Expand All @@ -213,7 +215,7 @@ func PrecompileBuildPackage(fileOrPkg string, goBinary string) error {

info, err := os.Stat(fileOrPkg)
if err != nil {
return fmt.Errorf("invalid file or package path: %w", err)
return fmt.Errorf("invalid file or package path %s: %w", fileOrPkg, err)
}
if !info.IsDir() {
file := fileOrPkg
Expand All @@ -223,7 +225,7 @@ func PrecompileBuildPackage(fileOrPkg string, goBinary string) error {
goGlob := filepath.Join(pkgDir, "*.go")
goMatches, err := filepath.Glob(goGlob)
if err != nil {
return fmt.Errorf("glob: %w", err)
return fmt.Errorf("glob %s: %w", goGlob, err)
}
for _, goMatch := range goMatches {
switch {
Expand All @@ -246,16 +248,53 @@ func PrecompileBuildPackage(fileOrPkg string, goBinary string) error {
cmd.Dir = rootDir
}
out, err := cmd.CombinedOutput()
if err != nil {
fmt.Fprintln(os.Stderr, string(out))
return fmt.Errorf("std go compiler: %w", err)
if _, ok := err.(*exec.ExitError); ok {
// exit error
return parseGoBuildErrors(string(out))
}
return err
}

return nil
var errorRe = regexp.MustCompile(`(?m)^(\S+):(\d+):(\d+): (.+)$`)

// parseGoBuildErrors returns a scanner.ErrorList filled with all errors found
// in out, which is supposed to be the output of the `go build` command.
// Each errors are translated into their correlated gno files, by:
// - changing the filename from *.gno.gen.go to *.gno
// - shifting line number according to the added header in generated go files
// (see [Precompile] for that header).
func parseGoBuildErrors(out string) error {
var errList goscanner.ErrorList
matches := errorRe.FindAllStringSubmatch(out, -1)
for _, match := range matches {
filename := match[1]
line, err := strconv.Atoi(match[2])
if err != nil {
return fmt.Errorf("parse line go build error %s: %w", match, err)
}

column, err := strconv.Atoi(match[3])
if err != nil {
return fmt.Errorf("parse column go build error %s: %w", match, err)
}
msg := match[4]
errList.Add(token.Position{
// Remove .gen.go extension, we want to target the gno file
Filename: strings.TrimSuffix(filename, ".gen.go"),
// Shift the 4 lines header added in *.gen.go files.
// NOTE(tb): the 4 lines shift below assumes there's always a //go:build
// directive. But the tags are optional in the Precompile() function
// so that leaves some doubts... We might want something more reliable than
// constants to shift lines.
Line: line - 4,
Column: column,
}, msg)
}
return errList.Err()
}

func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.Node, error) {
var errs error
var errs goscanner.ErrorList

imports := astutil.Imports(fset, f)

Expand Down Expand Up @@ -294,7 +333,7 @@ func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.N
continue
}

errs = multierr.Append(errs, fmt.Errorf("import %q is not in the whitelist", importPath))
errs.Add(fset.Position(importSpec.Pos()), fmt.Sprintf("import %q is not in the whitelist", importPath))
}
}
}
Expand All @@ -307,7 +346,7 @@ func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.N
// std package
if importPath == GnoStdPkgBefore {
if !astutil.RewriteImport(fset, f, GnoStdPkgBefore, GnoStdPkgAfter) {
errs = multierr.Append(errs, fmt.Errorf("failed to replace the %q package with %q", GnoStdPkgBefore, GnoStdPkgAfter))
errs.Add(fset.Position(importSpec.Pos()), fmt.Sprintf("failed to replace the %q package with %q", GnoStdPkgBefore, GnoStdPkgAfter))
}
}

Expand All @@ -316,7 +355,7 @@ func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.N
target := GnoPackagePrefixAfter + strings.TrimPrefix(importPath, GnoPackagePrefixBefore)

if !astutil.RewriteImport(fset, f, importPath, target) {
errs = multierr.Append(errs, fmt.Errorf("failed to replace the %q package with %q", importPath, target))
errs.Add(fset.Position(importSpec.Pos()), fmt.Sprintf("failed to replace the %q package with %q", importPath, target))
}
}

Expand All @@ -325,7 +364,7 @@ func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.N
target := GnoRealmPkgsPrefixAfter + strings.TrimPrefix(importPath, GnoRealmPkgsPrefixBefore)

if !astutil.RewriteImport(fset, f, importPath, target) {
errs = multierr.Append(errs, fmt.Errorf("failed to replace the %q package with %q", importPath, target))
errs.Add(fset.Position(importSpec.Pos()), fmt.Sprintf("failed to replace the %q package with %q", importPath, target))
}
}
}
Expand All @@ -344,6 +383,5 @@ func precompileAST(fset *token.FileSet, f *ast.File, checkWhitelist bool) (ast.N
return true
},
)

return node, errs
return node, errs.Err()
}
Loading

0 comments on commit 0ae1876

Please sign in to comment.