Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args #2992

jrfnl · 2020-06-21T10:24:46Z

While researching how the result caching was implemented and playing around with it, I noticed that the --sniffs and --exclude arguments are not taken into account properly, leading to misleading results being cached and displayed for runs.

Steps to reproduce the issue when using the --sniffs CLI argument

Given the following file:

<?php

namespace Foo;

use const PHP_VERSION_ID;
use func in_array;

class my_class {
	function method() {
		$versions = ['10', '20'];
		return ( in_array( PHP_VERSION_ID, $versions ) );
	}
}

With this ruleset saved as phpcs.xml.dist:

<?xml version="1.0"?>
<ruleset name="Issue cache invalidation CLI args">
	<file>.</file>
	<arg value="sp"/>
	<arg name="basepath" value="."/>
	<arg name="cache" value="phpcs.cache"/>
	<rule ref="PSR12"/>
</ruleset>

If necessary, throw away an existing phpcs.cache file in the test directory.

Run phpcs --report=source
The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowTabIndent.TabsUsed                 4
[x] PSR2.Classes.ClassDeclaration.OpenBraceNewLine                1
[x] PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket      1
[x] PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket    1
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[ ] Squiz.Classes.ValidClassName.NotCamelCaps                     1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
[ ] Squiz.Scope.MethodScope.Missing                               1
-----------------------------------------------------------------------
A TOTAL OF 12 SNIFF VIOLATIONS WERE FOUND IN 9 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SOURCES AUTOMATICALLY (9 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Run phpcs --report=source --sniffs=Generic.WhiteSpace.DisallowTabIndent,Squiz.Classes.ValidClassName
The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowTabIndent.TabsUsed                4
[ ] Squiz.Classes.ValidClassName.NotCamelCaps                    1
----------------------------------------------------------------------
A TOTAL OF 5 SNIFF VIOLATIONS WERE FOUND IN 2 SOURCES
---------------------------------------------------------------------- 
PHPCBF CAN FIX THE 1 MARKED SOURCES AUTOMATICALLY (4 VIOLATIONS IN TOTAL) 
----------------------------------------------------------------------

Fix the issues reported by the above two mentioned sniffs in the file.
Run phpcs --report=source --sniffs=Generic.WhiteSpace.DisallowTabIndent,Squiz.Classes.ValidClassName again and get a clean bill of health.
Run phpcs --report=source --sniffs=Squiz.Scope.MethodScope,PSR2.Classes.ClassDeclaration and see no issues being reported.

Run phpcs --report=source --sniffs=Squiz.Scope.MethodScope,PSR2.Classes.ClassDeclaration --no-cache
The result will be as below. Take note of the discrepancy between the output in step 5 and step 6:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[x] PSR2.Classes.ClassDeclaration.OpenBraceNewLine               1
[ ] Squiz.Scope.MethodScope.Missing                              1
----------------------------------------------------------------------
A TOTAL OF 2 SNIFF VIOLATIONS WERE FOUND IN 2 SOURCES
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SOURCES AUTOMATICALLY (1 VIOLATIONS IN TOTAL)
----------------------------------------------------------------------

Steps to reproduce the issue when using the --exclude CLI argument

Given the following file:

<?php

namespace Foo;

use const PHP_VERSION_ID;
use func in_array;

class my_class {
	function method() {
		$versions = ['10', '20'];
		return ( in_array( PHP_VERSION_ID, $versions ) );
	}
}

With this ruleset saved as phpcs.xml.dist:

<?xml version="1.0"?>
<ruleset name="Issue cache invalidation CLI args">
	<file>.</file>
	<arg value="sp"/>
	<arg name="basepath" value="."/>
	<arg name="cache" value="phpcs.cache"/>
	<rule ref="PSR12"/>
</ruleset>

If necessary, throw away an existing phpcs.cache file in the test directory.

Run phpcs --report=source
The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowTabIndent.TabsUsed                 4
[x] PSR2.Classes.ClassDeclaration.OpenBraceNewLine                1
[x] PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket      1
[x] PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket    1
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[ ] Squiz.Classes.ValidClassName.NotCamelCaps                     1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
[ ] Squiz.Scope.MethodScope.Missing                               1
-----------------------------------------------------------------------
A TOTAL OF 12 SNIFF VIOLATIONS WERE FOUND IN 9 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SOURCES AUTOMATICALLY (9 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Run

phpcs --report=source --exclude=Generic.WhiteSpace.DisallowTabIndent,Squiz.Classes.ValidClassName,PSR2.Methods.FunctionCallSignature

The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] PSR2.Classes.ClassDeclaration.OpenBraceNewLine                1
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
[ ] Squiz.Scope.MethodScope.Missing                               1
-----------------------------------------------------------------------
A TOTAL OF 5 SNIFF VIOLATIONS WERE FOUND IN 5 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SOURCES AUTOMATICALLY (3 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Fix a few issues. For this example, I fixed the PSR2.Classes.ClassDeclaration.OpenBraceNewLine and the Squiz.Scope.MethodScope.Missing issues.

Run

phpcs --report=source --exclude=Generic.WhiteSpace.DisallowTabIndent,Squiz.Classes.ValidClassName,PSR2.Methods.FunctionCallSignature

again.
The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
-----------------------------------------------------------------------
A TOTAL OF 3 SNIFF VIOLATIONS WERE FOUND IN 3 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SOURCES AUTOMATICALLY (2 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Run phpcs --report=source
The output will be:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
SOURCE                                                        COUNT
-----------------------------------------------------------------------
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
-----------------------------------------------------------------------
A TOTAL OF 3 SNIFF VIOLATIONS WERE FOUND IN 3 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SOURCES AUTOMATICALLY (2 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Run phpcs --report=source --no-cache
The result will be as below. Take note of the discrepancy between the output in step 5 and step 6:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowTabIndent.TabsUsed                 4
[x] PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket      1
[x] PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket    1
[ ] PSR12.Files.FileHeader.IncorrectOrder                         1
[x] PSR12.Files.FileHeader.SpacingAfterUseConstBlock              1
[ ] Squiz.Classes.ValidClassName.NotCamelCaps                     1
[x] Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine  1
-----------------------------------------------------------------------
A TOTAL OF 10 SNIFF VIOLATIONS WERE FOUND IN 7 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SOURCES AUTOMATICALLY (8 VIOLATIONS IN TOTAL)
-----------------------------------------------------------------------

Now, there are multiple possible solutions for this:

Disregard any caching settings when --sniffs or --exclude is being used.
This will be detrimental for people who don't use a custom ruleset and have these arguments set up to be used by default in, for instance, a Composer script.
Invalidate the cache when the value for --sniffs or --exclude is not the same as for the cached run.
This will mean that the cache will be invalidated and recreated from scratch a lot more often for people often using these arguments, but the results will be accurate.
Handle these arguments when throwing the errors instead of when processing the ruleset and base the cache on the complete result without taking these arguments into account.
This would mean refactoring of parts of PHPCS, so if desired, should probably be left for PHPCS 4.x.

So, for now, I'm proposing to implement solution 2.

The text was updated successfully, but these errors were encountered:

gsherwood · 2020-06-22T07:23:53Z

Solution 3 is actually what PHPCS is actually trying to do, but I think the issue is the order of commands in this case.

When caching, PHPCS intentionally doesn't filter out any errors based on filtering rules:

PHP_CodeSniffer/src/Files/File.php

Lines 894 to 910 in a957a73

    
           $includeAll = true; 
        
           if ($this->configCache['cache'] === false 
        
               || $this->configCache['recordErrors'] === false 
        
           ) { 
        
               $includeAll = false; 
        
           } 
        
           // Filter out any messages for sniffs that shouldn't have run 
        
           // due to the use of the --sniffs command line argument. 
        
           if ($includeAll === false 
        
               && ((empty($this->configCache['sniffs']) === false 
        
               && in_array(strtolower($listenerCode), $this->configCache['sniffs'], true) === false) 
        
               || (empty($this->configCache['exclude']) === false 
        
               && in_array(strtolower($listenerCode), $this->configCache['exclude'], true) === true)) 
        
           ) { 
        
               return false; 
        
           }

When doing a normal run with a cache file, all errors are replayed to apply the filtering rules correctly:

PHP_CodeSniffer/src/Files/LocalFile.php

Lines 102 to 112 in a957a73

    
           if ($this->configCache['recordErrors'] === true) { 
        
               // Replay the cached errors and warnings to filter out the ones 
        
               // we don't need for this specific run. 
        
               $this->configCache['cache'] = false; 
        
               $this->replayErrors($cache['errors'], $cache['warnings']); 
        
               $this->configCache['cache'] = true; 
        
           } else { 
        
               $this->errorCount   = $cache['errorCount']; 
        
               $this->warningCount = $cache['warningCount']; 
        
               $this->fixableCount = $cache['fixableCount']; 
        
           }

I need to look into still, but I suspect what is happening is that the sniffs and exclude CLI arguments are still being applied even when caching is taking place, so the resulting cache file is not actually an unfiltered run.

gsherwood · 2020-11-20T04:30:53Z

I've looked into this a bit more and it looks like the problem is with ruleset parsing. Before the sample ruleset is parsed, PHPCS thinks that caching is disabled. Once the ruleset is parsed, it now knows that caching is enabled, but it's already set the sniffs restrictions.

The fix I'm currently testing is to blank out the sniff restrictions after the ruleset has been parsed, as we then know if caching is enabled or not.

…ache files when using --sniffs and --exclude CLI args

gsherwood · 2020-11-20T05:26:47Z

That fix looks to have worked, so I've committed that change. Thanks for reporting this.

jrfnl · 2020-11-20T05:59:58Z

@gsherwood Thanks a lot! I'll test it along the way.

This was referenced Jun 21, 2020

Feature request: CLI option to clear the results cache #2993

Closed

Result caching: minor optimization tweak #2995

Merged

Invalidate the cache when different --sniffs or --exclude settings are being used #2996

Closed

gsherwood added this to the 3.6.0 milestone Sep 24, 2020

gsherwood changed the title ~~Bug: result caching vs CLI --sniffs and --exclude arguments~~ Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args Nov 20, 2020

gsherwood added the Type: bug label Nov 20, 2020

gsherwood added a commit that referenced this issue Nov 20, 2020

Fixed bug #2992 : Enabling caching using a ruleset produces invalid c…

bb906ff

…ache files when using --sniffs and --exclude CLI args

gsherwood added a commit that referenced this issue Nov 20, 2020

Fixed bug #2992 : Enabling caching using a ruleset produces invalid c…

6f78d8c

…ache files when using --sniffs and --exclude CLI args

gsherwood closed this as completed Nov 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args #2992

Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args #2992

jrfnl commented Jun 21, 2020

gsherwood commented Jun 22, 2020

gsherwood commented Nov 20, 2020

gsherwood commented Nov 20, 2020

jrfnl commented Nov 20, 2020

Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args #2992

Enabling caching using a ruleset produces invalid cache files when using --sniffs and --exclude CLI args #2992

Comments

jrfnl commented Jun 21, 2020

gsherwood commented Jun 22, 2020

gsherwood commented Nov 20, 2020

gsherwood commented Nov 20, 2020

jrfnl commented Nov 20, 2020