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

feat: printing all the errors from goparser #2011

Merged
merged 7 commits into from
May 23, 2024

Conversation

Villaquiranm
Copy link
Contributor

@Villaquiranm Villaquiranm commented May 1, 2024

This PR aims to print all the errors that are found by the go/parser package.
Currently the parser.ParseFile function returns an scanner.ErrorList object that only prints the first error and a count of the remaining errors. #1933 everything is better explained here.

For the changes this PR introduces:

  • Create issueWithError function. This function will wrap the lines of code that processed an error (before this PR changes) in order to convert it on an lintIssue structure. This existent code applies a regex to localize the parse error more precisely.
  • The PR changes the behavior of run, test, and lint commands. Not they exit with a non-zero status code as soon as we found some error while parsing the files. This in order to not having the whole stacktrace for some parsing file error, improving visibility.
  • Add a new case on gnovm/cmd/gno/lint.go.catchRuntimeError to handle and print all errors of an error with type scanner.ErrorList
  • remove some wrapping during panic calls. This was done by simplicity some of the calls were doing panic(errors.Wrap(err, "parsing file "+mfile.Name)) here err has the type scanner.ErrorList but as we're wrapping it it would be more complex and difficult to read on catchRuntimeError to identify an error with the type ErrorList

Results: (Using issue example)

  • lint:
-- NOW
./.: missing 'gno.mod' file (code=1).
test.gno:6: expected ';', found example (code=2).
test.gno:7: expected '}', found 'EOF' (code=2).
exit status 1

---- BEFORE
./.: missing 'gno.mod' file (code=1).
test.gno:6: expected ';', found example (and 1 more errors) (code=2).
  • run:
-- NOW
test.gno:6: expected ';', found example (code=2).
test.gno:7: expected '}', found 'EOF' (code=2).
exit status 1

---- BEFORE
panic: test.gno:6:5: expected ';', found example (and 1 more errors)

goroutine 1 [running]:
github.com/gnolang/gno/gnovm/pkg/gnolang.MustReadFile(...)
        /Users/miguel/gnorigin/gnovm/pkg/gnolang/go2gno.go:49
main.parseFiles({0x140003b1720, 0x1, 0x1400031fb48?})
        /Users/miguel/gnorigin/gnovm/cmd/gno/run.go:132 +0x27c
main.parseFiles({0x140003b09a0, 0x1, 0x0?})
        /Users/miguel/gnorigin/gnovm/cmd/gno/run.go:121 +0x158
main.execRun(0x14000373230, {0x140003b09a0, 0x1, 0x1}, {0x100ca3e68, 0x140003b3bd0})
        /Users/miguel/gnorigin/gnovm/cmd/gno/run.go:89 +0x174
main.newRunCmd.func1({0x0?, 0x1400019c140?}, {0x140003b09a0?, 0x0?, 0x0?})
        /Users/miguel/gnorigin/gnovm/cmd/gno/run.go:35 +0x3c
github.com/gnolang/gno/tm2/pkg/commands.(*Command).Run(0x1400031fdc8?, {0x100c9af18?, 0x101249700?})
        /Users/miguel/gnorigin/tm2/pkg/commands/command.go:247 +0x17c
github.com/gnolang/gno/tm2/pkg/commands.(*Command).Run(0x140003ae2c0?, {0x100c9af18?, 0x101249700?})
        /Users/miguel/gnorigin/tm2/pkg/commands/command.go:251 +0x12c
github.com/gnolang/gno/tm2/pkg/commands.(*Command).ParseAndRun(0x140003ae6e0?, {0x100c9af18, 0x101249700}, {0x1400019c130?, 0x140003ae790?, 0x140003ae840?})
        /Users/miguel/gnorigin/tm2/pkg/commands/command.go:132 +0x4c
github.com/gnolang/gno/tm2/pkg/commands.(*Command).Execute(0x100ca3e68?, {0x100c9af18?, 0x101249700?}, {0x1400019c130?, 0x1011a5ef8?, 0x140000021a0?})
        /Users/miguel/gnorigin/tm2/pkg/commands/command.go:114 +0x28
main.main()
        /Users/miguel/gnorigin/gnovm/cmd/gno/main.go:13 +0x6c
Contributors' checklist...
  • 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
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label May 1, 2024
Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 67.39130% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 49.02%. Comparing base (7d939a0) to head (10e9b14).

Files Patch % Lines
gnovm/cmd/gno/test.go 0.00% 7 Missing ⚠️
gnovm/cmd/gno/lint.go 82.75% 4 Missing and 1 partial ⚠️
gnovm/cmd/gno/run.go 77.77% 1 Missing and 1 partial ⚠️
gnovm/pkg/gnolang/nodes.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2011      +/-   ##
==========================================
+ Coverage   49.01%   49.02%   +0.01%     
==========================================
  Files         576      576              
  Lines       77604    77617      +13     
==========================================
+ Hits        38040    38055      +15     
+ Misses      36481    36480       -1     
+ Partials     3083     3082       -1     
Flag Coverage Δ
gnovm 41.97% <67.39%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gnovm/cmd/gno/util.go Outdated Show resolved Hide resolved
@moul
Copy link
Member

moul commented May 1, 2024

Great addition! It's definitely needed. Could you simplify the implementation?

@Villaquiranm Villaquiranm requested a review from deelawn as a code owner May 2, 2024 18:18
@Villaquiranm Villaquiranm force-pushed the 1933-scanner-error-list branch 3 times, most recently from b48aafc to baf5cef Compare May 2, 2024 19:13
@Villaquiranm Villaquiranm requested a review from mvertes as a code owner May 8, 2024 11:53
gnovm/cmd/gno/lint.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/lint.go Outdated Show resolved Hide resolved
gnovm/tests/integ/several-lint-errors/main.gno Outdated Show resolved Hide resolved
@thehowl
Copy link
Member

thehowl commented May 21, 2024

Thank you for your contribution!

@Villaquiranm Villaquiranm force-pushed the 1933-scanner-error-list branch from 58ccdf1 to 91a66af Compare May 21, 2024 16:02
@Villaquiranm
Copy link
Contributor Author

Just a little remark on this PR,
With @omarsy we realized that even with this PR we could have some linter errors that are now shown.

This happens when having errors on different files
Let's say: a.gno (2 lint errors) b.gno (1 lint) error. We will only show all lint errors of only the first file.

I would be glad to work on this again in order to have all errors on all folder files something more Go like

@thehowl
Copy link
Member

thehowl commented May 23, 2024

We will only show all lint errors of only the first file.

Feel free to make a follow up :)

@thehowl thehowl merged commit f24690e into gnolang:master May 23, 2024
151 checks passed
@omarsy omarsy deleted the 1933-scanner-error-list branch May 23, 2024 21:35
thehowl pushed a commit that referenced this pull request Oct 21, 2024
This Pull request intents to follow up on #2011. As said on that Pull
request, currently we show all lint errors on the first analyzed file.
If in a folder we have `a.gno` & `b.gno` both with lint errors. `gno
lint | run | test` will only find the errors related to one of those
files.

**This PR aims to show all the lint errors present on the current
folder**.

Changes:
for lint &  test cmd:
- we modified ParseMemPackage function on gnovm/pkg/gnoland/nodes.go.
Before this function returned as soon as an error was found while
Parsing the gno file. So we introduced an error slice to keep track of
all Parse errors. After parsing all the files we panic with the list of
errors only if this list is not empty.
- we did the same on parseMemPackageTests function 
- create a function printRuntimeError that handles the print of the
errors inside `catchRuntimeError` function. We did this change in order
to be able to recursively call the funtion and handle the case of an
[]error type composed of scanner.ErrorList errors.

### Results
* running on gnovm/tests/integ/several-files-multiple/errors
LINT (before):
```sh
several-files-multiple-errors % gno lint .
file2.gno:3: expected 'IDENT', found '{' (code=2).
file2.gno:5: expected type, found '}' (code=2).
```

LINT (after):
```sh
gno lint .
file2.gno:3: expected 'IDENT', found '{' (code=2).
file2.gno:5: expected type, found '}' (code=2).
main.gno:5: expected ';', found example (code=2).
main.gno:6: expected '}', found 'EOF' (code=2).
exit status 1
```

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

<details>

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

- [ ] 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
- [ ] Added references to related issues and PRs
- [ ] 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
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants