Ruleset problem: Twig sniff is never run

[jrfnl]

Bug Description

The WordPressVIPMinimum standard's ruleset.xml file contains the following snippet:

VIP-Coding-Standards/WordPressVIPMinimum/ruleset.xml

Lines 30 to 32 in 1753c9f

	<rule ref="WordPressVIPMinimum.Security.Twig">
	<include-pattern>*.twig</include-pattern>
	</rule>

By inheritance, this is also applied to the WordPress-VIP-Go standard (which includes the WordPressVIPMinimum standard.

This snippet causes the sniff to only be run for files with a .twig file extension, but the .twig file extension is not one of the default extensions searched for by PHP_CodeSniffer and the PHPCS extensions setting is not overruled for the WordPressVIPMinimum/WordPress-VIP-Go standard, which means files using the .twig extension will never be scanned.

This effectively renders the TwigSniff useless as it will never run. (and never has either as the above snippet has been in the ruleset since the sniff's introduction....)

And even if the extensions setting would be set in the ruleset, like so <arg name="extensions" value="php,inc,js,css,twig"/>, the twig part would need to be annotated with the language to tell PHPCS which tokenizer to use for twig files, like for example: twig/js.

Typically, twig files are basically HTML files, but PHPCS does not have a tokenizer for HTML files and tokenizing the files as JS or CSS would probably get really weird tokenization results. Aside from the fact that JS/CSS support will be removed in PHPCS 4.0.
Tokenizing as PHP may work as the code should all be tokenized as T_INLINE_HTML, which is handled by the sniff.

If we look at the TwigSniff itself:

• the $supportedTokenizers property indicates that both JS as well as PHP files are (supposedly) supported.
• the code inside the process_token() method, however looks like code which would only work for PHP files, but as there is no js test case file, this cannot be verified.

The actual sniff code is tested for use with PHP files, but due to the ruleset configuration, will in reality never run on PHP files.

To Reproduce

Save the following as test.twig:

{% autoescape false %}
    Everything will be outputted as is in this block
{% endautoescape %}

{% autoescape %}
    {{ safe_value|raw }}
{% endautoescape %}

Run:

phpcs -ps ./test.twig --standard=WordPressVIPMinimum --sniffs=WordPressVIPMinimum.Security.Twig -v

It will show that no files are being scanned.

If run with --extensions=twig, the twig extension will be interpreted as PHP, so should render some output:

phpcs -ps ./test.twig --standard=WordPressVIPMinimum --sniffs=WordPressVIPMinimum.Security.Twig --extensions=twig

For plain HTML/twig files, this should work, but we cannot be certain that all .twig files only contain plain HTML/twig code.

What to do ?

Based on the above, there are a couple of action paths open:

1. Remove the sniff. It wasn't being run anyway, so nobody will miss it.
2. Add the <arg name="extensions" value="php,inc/php,js,css,twig/php"/> setting to the WordPressVIPMinimum ruleset.xml file and hope for the best.
   This means the sniff will start running over .twig files and interpret them as PHP files.
   This may have unintended side-effects, as all other PHP sniffs will now also be run over .twig files.
3. Remove the <include-pattern>*.twig</include-pattern> within the <rule ref="WordPressVIPMinimum.Security.Twig"> from the ruleset.xml file.
   This means the sniff will start running over JS and PHP files.
   For PHP files, this should work. For JS files, I have no clue as there are no tests available.
   As for how useful that would be, that would still remain to be seen, as saving twig files with a php file extension is uncommon.
   Additionally, this may yield false positives for code in other PHP files, which looks like twig code, but isn't.

Opinions on what direction to take welcome.

Environment

Question	Answer
PHP version	irrelevant
PHP_CodeSniffer version	3.13.2
PHPCSUtils version	develop
VIPCS version	develop
WordPressCS version	develop
PHPCSExtra version	develop
VariableAnalysis version	develop

Tested Against main branch?

• I have verified the issue still exists in the main branch of VIPCS.
• I have verified the issue still exists in the develop branch of VIPCS.

[GaryJones]

Let's go ahead and remove the sniff please.