search

squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.67k stars 1.48k forks source link

STDIN is ignored when phpcs.xml exists #873

Closed aymanrady closed 8 years ago

aymanrady commented 8 years ago

running command cat fixture.php | phpcs expected: scan fixture.php actual: scans src/

<?xml version="1.0"?>
<ruleset name="TEST">
    <file>src</file>
    <rule ref="PSR2"/>
</ruleset>

this used to work in v2.4.0 and is broken in v2.5.x

gsherwood commented 8 years ago

I tested this with 2.4.0 and it works the same as 2.5.x. The behaviour actually changed in 2.2.0 when the phpcs.xml file was first supported.

What did change in 2.5.0 is that the phpcs.xml file will be used even if files are specified on the command line. But in this case, the files specified in the ruleset.xml file are ignored, but all other settings are used. It also added support for phpcs.xml.dist files, which were previously ignored.

I'm not sure how you managed to get that command working in 2.4.0, but I'll keep looking. Are you sure it was actually version 2.4.0?

aymanrady commented 8 years ago

I'll try to write a reduced test case and update the issue with it

gsherwood commented 8 years ago

I'll try to write a reduced test case and update the issue with it

I just want to be clear that I can see the issue and am trying to figure out if I can fix it without introducing a new command line argument for STDIN (as I have in 3.0), but I can't find a change between 2.4 and 2.5 that would have modified the behaviour.

If I can't find anything, or can't find a good solution, the change I made in the 3.0 branch is to force you to pass - on the command line to indicate that you want STDIN checked.

gsherwood commented 8 years ago

I've pushed a change that checks STDIN to see if anything has been passed and uses that content if it has. This ignores all files specified on the command line or in a ruleset file.

I'm still keen to see the BC break you found between 2.4 and 2.5 if you can get a good test case.

gsherwood commented 8 years ago

Another commit to move the code to the same location as in 3.0. It's different logic because 3.0 has the - CLI arg for this, but at least it is in the same method.

aymanrady commented 8 years ago

I'm still keen to see the BC break you found between 2.4 and 2.5 if you can get a good test case.

I don't know how I got it to work with 2.4, I tried to reproduce it and failed. just tested your commit and its working great, when will you release this version?

gsherwood commented 8 years ago

I don't know how I got it to work with 2.4, I tried to reproduce it and failed.

Thanks for trying anyway. Maybe you ran the command from a directory lower down in the tree? 2.5.0 introduced the feature where PHPCS will look for a phpcs.xml file in parent directories as well.

Either way, the problem was obvious once you pointed it out. Thanks for testing the patch.

when will you release this version?

I only just released version 2.5.1 (2 days ago), so it's not likely to be for another 2-3 weeks. The next version will be 2.6.0 and a feature release, so I can't push it out any quicker.