Non-zero exit code breaks CI compatibility

[madlabman]

Since the version 4.5.0 there's a zero exit code (see #554) even with reported errors. It makes it hard to use within actions in CI. My personal opinion is to have the similar behaviour other linters preserve - return 1 in case of errors.

I assume the implemented change was an attempt to fix kinda niche issue (even it has fixed my workflow as well), and it was done in bad manner, I believe. My suggestion is to use command line flags to achieve the desired behaviour, such as --exit-code (to return non-zero exit code back) or, which is even better, to re-introduce the non-zero exit code and use a flag to change the behaviour on demand.

[dbale-altoros]

I will check this as soon as i can
thanks @madlabman for the suggestion

[juanpcapurro]

FWIW I overhauled the exit codes on solhint-community with:

solhint-community/solhint-community#134 -- to exit early and with a different code if there's a misconfiguration
solhint-community/solhint-community#76 -- to have --quiet not override --max-warnings

[dbale-altoros]

FWIW I overhauled the exit codes on solhint-community with:

solhint-community/solhint-community#134 -- to exit early and with a different code if there's a misconfiguration solhint-community/solhint-community#76 -- to have --quiet not override --max-warnings

cool!
Thanks @juanpcapurro for that!

[MaxenceAdnot]

Hello @dbale-altoros

Do you plan to implement this in solhint or should we move to solhint-community ?

[dbale-altoros]

Hey @MaxenceAdnot I will implement this fix as well
Probably next week
solhint community is a fork from protofire solhint
both are open source

We will mantain this original solhint because it is an important part of protofire products

[dbale-altoros]

@MaxenceAdnot
@madlabman
Expect new solhint version on Monday

[dbale-altoros]

Will merge this
#578
To fix this issue

[dbale-altoros]

New version
https://www.npmjs.com/package/solhint

[MaxenceAdnot]

Thanks a lot @dbale-altoros

[madlabman]

Welp, still doesn't work for me.

Probably, the reason is the following code

if (reports[0].errorCount > 0) process.exit(EXIT_CODES.REPORTED_ERRORS)

should iterate over the all the reports instead.

@dbale-altoros

[dbale-altoros]

@madlabman
Yes , sorry. You're right
I mean, reports[0] contains all errors from the first contract analyzed
If you have more than one contract this will not provide correct info
Like you said I need to loop through all contracts ( all reports[] )
I will fix it today on my afternoon

Thanks a lot for the feedback !! Really appreciate

[dbale-altoros]

@madlabman made a new package on npm
whenever you can, please confirm if it is working

[madlabman]

@madlabman made a new package on npm whenever you can, please confirm if it is working

LGTM, thank you @dbale-altoros !