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

The "phpcs" doesn't use error exit code when invalid standard is given #2412

Open
aik099 opened this issue Feb 9, 2019 · 10 comments
Open

The "phpcs" doesn't use error exit code when invalid standard is given #2412

aik099 opened this issue Feb 9, 2019 · 10 comments

Comments

@aik099
Copy link
Contributor

aik099 commented Feb 9, 2019

I was trying to use PHP_CodeSniffer 2.x standard on PHP_CodeSniffer 3.x binary. Since ruleset.xml format wasn't changed I wasn't getting any exception due wrongly named sniff classes.

I however did get this output:

ERROR: Referenced sniff "Squiz.PHP.DisallowObEndFlush" does not exist

Run "phpcs --help" for usage information

Unfortunately exit code from that operation was 0 (= success) and therefore any build system using PHP_CodeSniffer wouldn't fail.

@gsherwood gsherwood added this to the 3.5.0 milestone Feb 11, 2019
@jrfnl
Copy link
Contributor

jrfnl commented Mar 24, 2019

@aik099 I've just ran some tests with the below ruleset to try and reproduce this (using master), but with my tests, PHPCS exits with error code 3 which should fail the build.

Could you provide more information to help reproduce the issue ?

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="issue-2412">
  <description>PHP_CodeSniffer</description>
  <rule ref="PSR1"/>
  <rule ref="Standard.Category.SniffName"/>
</ruleset>

Output:

> phpcs . --standard=phpcs-2412.xml & echo Exit code is %errorlevel%

PHP_CodeSniffer version 3.4.2 (stable) by Squiz (http://www.squiz.net)

ERROR: Referenced sniff "Standard.Category.SniffName" does not exist

Run "phpcs --help" for usage information

Exit code is 3

@aik099
Copy link
Contributor Author

aik099 commented Mar 25, 2019

Ah, that's my bad. I was using Phing as my build system and haven't indicated a flag to fail the build based on phpcs command exit code.

When I did so now I've noticed that phpcs would also use non-zero exit code, when some sniffs reported warnings/errors. Is there any way to disable this behavior?

@jrfnl
Copy link
Contributor

jrfnl commented Mar 25, 2019

@aik099 Of course there is ;-)

If you always want to ignore warnings/errors for the exit code, you can use --config-set ignore_errors_on_exit 1 / --config-set ignore_warnings_on_exit 1 (one time only command, setting will be remembered).

If you only want to just ignore them for a particular run, like in CI, you can use --runtime-set ignore_errors_on_exit 1 / --runtime-set ignore_warnings_on_exit 1 (add it to your normal phpcs command in the CI).

See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#ignoring-errors-when-generating-the-exit-code

@aik099
Copy link
Contributor Author

aik099 commented Mar 25, 2019

@jrfnl , thanks. That is exactly, when I need.

I've looked at phpcs --help and these options weren't listed. List of supported config settings and their purpose as part of phpcs --help would help greatly.

@jrfnl
Copy link
Contributor

jrfnl commented Mar 25, 2019

Maybe a link to the wiki at the bottom of the output ?

For more detailed information and information about the non-runtime configuration options, please visit the wiki: https://github.com/squizlabs/PHP_CodeSniffer/wiki/

@aik099
Copy link
Contributor Author

aik099 commented Mar 26, 2019

Great idea. Thanks.

@aik099 aik099 closed this as completed Mar 26, 2019
@jrfnl
Copy link
Contributor

jrfnl commented Apr 5, 2019

@aik099 The PR got rejected. Do you want to re-open ?

@aik099
Copy link
Contributor Author

aik099 commented Apr 5, 2019

Since it was rejected reopening it with same changes won't do any good. Maybe we can return to originally proposed idea: list configuration options, that are supported (dynamically).

@jrfnl
Copy link
Contributor

jrfnl commented Apr 5, 2019

@aik099 Sorry, that's what I meant: re-opening this issue to see if we can come up with another solution.

@aik099
Copy link
Contributor Author

aik099 commented Apr 5, 2019

OK. Reopening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants