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

Include-pattern for multiple patterns on v3.x #1481

Closed gmponos closed 7 years ago

gmponos commented 7 years ago

Hello,

I am using the latest tagged v3.

I have the following part inside a ruleset:

    <rule ref="Squiz.Strings.DoubleQuoteUsage" >
        <include-pattern>*/tests/*</include-pattern>
        <include-pattern>*/src/AppBundle/*</include-pattern>
    </rule>

Once I remove the first pattern of */tests/* errors are found to the second one but if I leave them as above no errors are found to the second path.

justechn commented 7 years ago

I am running into the same problem. It seems like the include patterns are AND instead of OR. In other words the file has to match both for it to be checked.

I have the following pattern

<rule ref="Generic.WhiteSpace.DisallowTabIndent">
        <include-pattern>*/protected/*</include-pattern>
        <include-pattern>*.tpl</include-pattern>
    </rule>

this does not return any errors

<rule ref="Generic.WhiteSpace.DisallowTabIndent">
        <include-pattern>*/protected/*</include-pattern>
        <include-pattern>*.php</include-pattern>
    </rule>

this returns a lot of errors

it would be great to get this fixed

justechn commented 7 years ago

The problem is in this piece of code https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Files/File.php#L409-L422

It allows the rules to cancel each other out. I made a small change so that as soon as it finds a passing pattern it exits and continues. It will only ignore if it does not find any matching patterns.

$ignoreListener = true;
foreach ($listenerData['include'] as $pattern) {
    if (DIRECTORY_SEPARATOR === '\\') {
        $pattern = str_replace('/', '\\\\', $pattern);
    }

    $pattern = '`' . $pattern . '`i';
    if (preg_match($pattern, $this->path) === 1) {
        $ignoreListener = false;
        break;
    }
}

if ($ignoreListener === true) {
    $this->ignoredListeners[$class] = true;
}

and now it works as I would expect.

gsherwood commented 7 years ago

I've modified the behaviour of the include-pattern rules so the logic is OR, not AND.

@justechn Your code was helpful and I did it in almost the same way, but your code would end up always running the sniff (there is no continue) and always adding it to the ignored list even if the sniff didn't specify any ignore patterns. If you have still have this change applied in your copy, PHPCS may not be working correctly. Please take a look at the commit for this issue and see if that works for you as well.

mr-feek commented 6 years ago

Anyone else still noticing this issue? I'm on version 3.3.2.

<include-pattern>^(*/tests/*|*/app/Http/*)</include-pattern> works for me, however the following does not

<include-pattern>*/tests/*</include-pattern>
<include-pattern>*/app/Http/*</include-pattern>