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

Make the output of gno precompile more parseable #1636

Closed
tbruyelle opened this issue Feb 7, 2024 · 0 comments · Fixed by #1670
Closed

Make the output of gno precompile more parseable #1636

tbruyelle opened this issue Feb 7, 2024 · 0 comments · Fixed by #1670
Labels
🌟 improvement performance improvements, refactors ...

Comments

@tbruyelle
Copy link
Contributor

tbruyelle commented Feb 7, 2024

Description

The current output of gno precompile -gobuild is hard to parse because it mixes 3 kinds of errors, with different format:

  1. errors from ast.Parse the gno file content:
<file.gno>: precompile: parse: <file.gno>:<line>:<col>: <error>

Example:
gnovm/tests/backup/bad0.gno: precompile: parse: gnovm/tests/backup/bad0.gno:1:1: expected 'package', found println

  1. errors from precompileAst function, which basically verifies that all import are whitelisted:
<file.gno>: precompile: <error>

Examples:
import "<pkg>" is not in the white list
failed to replace the <pkg> package with <?>

  1. errors from the go build -v -tags=gno command:
<file.gno.gen.go>:<line>:<col>: <error>

Example:
./gnotest.gno.gen.go:9:2: undefined: Y

Improvement

To make it easier for tools like gnols to parse the output, all errors should have the same format, and should only output errors for gno files (and not .gno.gen.go files like in 3).

I think the best format should be something as simple as 3, but for gno file only:

<file.gno>:<line>:<col>: <error>
@tbruyelle tbruyelle added the 🌟 improvement performance improvements, refactors ... label Feb 7, 2024
tbruyelle added a commit to tbruyelle/gno that referenced this issue Feb 8, 2024
Add all kinds of errors described in gnolang#1636.
thehowl pushed a commit that referenced this issue Feb 8, 2024
Add all kinds of errors described in #1636.

To run these tests:
```
go test ./gnovm/cmd/gno -v -run Precompile
```

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

<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
- [x] 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>
leohhhn pushed a commit to leohhhn/gno that referenced this issue Feb 8, 2024
Add all kinds of errors described in gnolang#1636.

To run these tests:
```
go test ./gnovm/cmd/gno -v -run Precompile
```

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

<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
- [x] 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>
tbruyelle added a commit to tbruyelle/gno that referenced this issue Feb 15, 2024
This test was actually only testing the private `precompileAST` function
and not the public `Precompile` function.

The change makes it call the public function and add some more cases
outlined in comments like
- unknow-realm (no error)
- syntax error
- black list (was already done)
- multiple files (not possible, `Precompile` is single file only)

Also used multiple lines string for file and expected content, for better
readability.

Relates to gnolang#1636
tbruyelle added a commit to tbruyelle/gno that referenced this issue Feb 15, 2024
This test actually only tested the private `precompileAST` function,
not the public `Precompile` function.

The change makes it call the public function and adds some more cases
described in comments like:
- unknow-realm (no error)
- syntax error
- blacklist (was already done)
- multiple files (not possible, `Precompile` is single file only)

Also used multiple-line string for file and expected content, for better
readability.

Relates to gnolang#1636
thehowl pushed a commit that referenced this issue Feb 16, 2024
This test actually only tested the private `precompileAST()` function,
not the public `Precompile()` function.

The change makes it call the public function and adds some more cases
described in comments like:
- unknow-realm (no error)
- syntax error
- blacklist (was already done)
- multiple files (not possible, `Precompile()` is single file only)

Also used multiple-line string for file and expected content, for better
readability.

Relates to #1636

The test can be run with 
```
go test ./gnovm/pkg/gnolang/ -v -run Precompile
```

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

<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
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] 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>
thehowl pushed a commit that referenced this issue Feb 22, 2024
…#1670)

<!-- please provide a detailed description of the changes made in this
pull request. -->
Fix #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>
leohhhn pushed a commit to leohhhn/gno that referenced this issue Feb 29, 2024
This test actually only tested the private `precompileAST()` function,
not the public `Precompile()` function.

The change makes it call the public function and adds some more cases
described in comments like:
- unknow-realm (no error)
- syntax error
- blacklist (was already done)
- multiple files (not possible, `Precompile()` is single file only)

Also used multiple-line string for file and expected content, for better
readability.

Relates to gnolang#1636

The test can be run with 
```
go test ./gnovm/pkg/gnolang/ -v -run Precompile
```

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

<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
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] 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>
leohhhn pushed a commit to leohhhn/gno that referenced this issue Feb 29, 2024
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 improvement performance improvements, refactors ...
Projects
Development

Successfully merging a pull request may close this issue.

1 participant