feat: lint all files in folder before panicking #2202
Conversation
Villaquiranm
requested review from
jaekwon,
moul,
piux2,
deelawn,
thehowl and
harry-hov
as code owners
Villaquiranm
force-pushed
the
lint-multiple-files
branch
from
5f97b9f
to
21b3e02
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2202 +/- ##
=======================================
Coverage 55.02% 55.02%
=======================================
Files 595 595
Lines 79662 79676 +14
=======================================
+ Hits 43834 43842 +8
- Misses 32512 32519 +7
+ Partials 3316 3315 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
gnovm/cmd/gno/lint.go
Outdated
| }() | ||
|
|
||
| action() | ||
| return | ||
| } | ||
|
|
||
| func printRuntimeError(r interface{}, pkgPath string, stderr io.WriteCloser) { |
There was a problem hiding this comment.
Is this new function creation necessary? It seemed better to handle the runtime error within the existing catchRuntimeError function.
There was a problem hiding this comment.
The reason I created the function is because I calling it recursively on this case:
When analysing several files we will panic with an array of errors
[]error or after the change you proposed a multierr.
knowing that inside each of these error there is a scanner.Errorlist that needs to be handled we have 2 options
- with []error (multierr) first time we will enter on the case and call recursively
printRuntimeErrorfunction again for each element (scanner.ErrorList)
case []error:
for _, err := range verr {
// recursive call to handle specifically each error type ex: scanner.ErrorList
printRuntimeError(err, pkgPath, stderr)
}Second time we will enter
case scanner.ErrorList:
for _, err := range verr {
fmt.Fprint(stderr, issueFromError(pkgPath, err).String()+"\n")
}- Second possibility is to keep the function as it were but having an extra for loop something like
case []error (multiErr):
for _, errL := range verr {
errorList := errL.(scanner.ErrorList)
for _, err := range errorList {
fmt.Fprint(stderr, issueFromError(pkgPath, errors.New(err)).String()+"\n")
}
}There was a problem hiding this comment.
implemented second possibility on 18fe895
Don't know if you consider it better
gnovm/cmd/gno/test.go
Outdated
| @@ -638,6 +638,7 @@ func loadTestFuncs(pkgName string, t *testFuncs, tfiles *gno.FileSet) *testFuncs | |||
| func parseMemPackageTests(memPkg *std.MemPackage) (tset, itset *gno.FileSet) { | |||
| tset = &gno.FileSet{} | |||
| itset = &gno.FileSet{} | |||
| errors := []error{} | |||
There was a problem hiding this comment.
We typically use multierr for this. Can you switch to it or explain why your solution is more suitable in this case?
There was a problem hiding this comment.
I'll try to use multiErr I think it will fit perfectly
There was a problem hiding this comment.
fixed on 18fe895
Villaquiranm
force-pushed
the
lint-multiple-files
branch
3 times, most recently
from
955cb1a
to
6be80ff
Compare
Villaquiranm
force-pushed
the
lint-multiple-files
branch
from
6be80ff
to
5819472
Compare
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.gnoboth with lint errors.gno lint | run | testwill 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:
catchRuntimeErrorfunction. 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
LINT (before):
LINT (after):
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description