Update WordPress-VIP-Go/ruleset.xml to configure VariableAnalysis

[gudmdharalds]

This patch will configure VariableAnalysis for WordPress-VIP-Go so not to report on undefined variables under certain circumstances. To quote an issue reported for vip-go-ci:

When template partials are require'd from within a class, then the partial may use $this->... to insert display a value. At PHP runtime, it works, but for PHPCS analysis, it sees a PHP file with an undefined $this and so reports it via the VariableAnalysis package. Technically it's correct, and while it could be ignored, it's not practical or desirable to do that.

This patch will accomplish this, reducing noise a bit for our end-users.

See here for more discussion.

Details on testing

The following code was put into a file:

<?php
// Example foo.php template.
 
// Set defaults.
$args = wp_parse_args(
    $args,
    array(
        'class'          => '',
        'arbitrary_data' => array(
            'foo' => 'fooval',
            'bar' => false,
        )
    )
);
?>
 
<div class="widget <?php echo esc_html_class( $args['class'] ); ?>">
    <?php echo esc_html( $args['arbitrary_data']['foo'] ); ?>
</div>

And phpcs run with the patch applied:

./phpcs/bin/phpcs --severity=1 --standard=WordPress-VIP-GO,PHPCompatibilityWP /tmp/file10.php

--------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------
 17 | ERROR | All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found 'esc_html_class'.
--------------------------------------------------------------------------------------------------------------------------------------------------------------

With the patch removed, this warning here is added to the results:

  6 | WARNING | Variable $args is undefined.

The same pattern was seen with this code here, using the same command:

<?php
// Example foo.php template.

if ( ! empty( $this->vars['video-id'] ) ) {
	echo '<video...></video>';
}

echo time();

With the patch applied, this was reported:

---------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------------
 8 | ERROR | All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found 'time'.
---------------------------------------------------------------------------------------------------------------------------------------------------

and without the patch, this was added:

 4 | WARNING | Variable $this is undefined.

[GaryJones]

This PR is testable with the following phpcs.xml config:

<?xml version="1.0"?>
<ruleset>
	<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
	    <properties>
	        <property name="allowUndefinedVariablesInFileScope" value="false"/>
	        <property name="allowUnusedVariablesInFileScope" value="false"/>
	    </properties>
	</rule>
</ruleset>

And the following phpcs-test.php test file in the same directory:

<?php

$my_foo = 'foo';

echo $my_bar;

and running phpcs phpcs-test.php -s.

With both properties as false, two violations will appear. By changing each of the properties to true (as per the PR), then each of the violations will no longer be reported.

However...

There are two issues in VariableAnalysis 2.x (cc @sirbrillig) that I think make these not as useful as they could be:

• File-scope unused variable not detected when ?> is present
• Undefined variables in function calls are not detected

So while this PR will enable some redundant violations of unused or undefined variables in the file-scope (typically in WP template files) to not be reported, others still will be for now.

[sirbrillig]

There are two issues in VariableAnalysis 2.x (cc @sirbrillig) that I think make these not as useful as they could be:

Hopefully https://github.com/sirbrillig/phpcs-variable-analysis/releases/tag/v2.11.1 will resolve the issues you were seeing.

[GaryJones]

@sirbrillig - Thank you for your prompt work and release! cc @gudmdharalds as we'll want to bump the bot to use 2.11.1.

@rebeccahum We'll probably also want to bump

VIP-Coding-Standards/composer.json

Line 20 in 086b3da

"sirbrillig/phpcs-variable-analysis": "^2.8.3",

to ensure users pick up this fix when updating locally.

[gudmdharalds]

Updated phpcs-variable-analysis to 2.11.1 here.

[gudmdharalds]

@GaryJones : Given the discussion above, I guess it is safe to merge this PR?