IncludingFile: Support runtime setting for custom variables in paths.

[GaryJones]

trafficstars

What problem would the enhancement address for VIP?

When a file includes a load of other includes/requires, then part of the path might be assigned to a variable, to make code DRY.

e.g.

$my_path = 'path/to/includes/;
require $my_path . 'foo.php';
require $my_path . 'bar.php';
require $my_path . 'baz.php';

The IncludingFileSniff checks the path, and other than a few exclusions for common functions and constants, it throws warnings for any other constants, functions and variables.

Since the prefix is often consistent for the file, the bot will add comment for each line.

The arrays of known constants and functions are all in public properties, so they can be overridden (though not implemented in the best way - see https://github.com/Automattic/VIP-Coding-Standards/issues/234), but variables cannot.

Describe the solution you'd like

How about we made the sniff look for a runtime setting comment that defined that a particular variable was OK / should be ignored for file inclusion? It would be set it at the top before all the includes, do the includes, then unset it again after.

$my_path = 'path/to/includes/;

// phpcs:set WordPressVIPMinimum.Files.IncludingFile custom_variables[] $my_path
require $my_path . 'foo.php';
require $my_path . 'bar.php';
require $my_path . 'baz.php';
// phpcs:set WordPressVIPMinimum.Files.IncludingFile custom_variables[] 

The bot workflow should be able to follow it since it would be an inline comment, not in a PHPCS config file.

The same could already be done with the public properties for known functions and constants, but as can be seen from the snippet above, the ideal unsetting would need to include the current values, which is awkward. As such, public custom* properties that are merged with the existing properties would be better.

What code should be reported as a violation?

No new code.

What code should not be reported as a violation?

Same as example above, where a runtime setting matches the name of a variable.

Anything else

Unit tests should also cover items like require $my_path . $my_other_path . 'baz.php'; where there are multiple variables, or require MY_CONSTANT . $my_path . 'baz.php'; where there are multiple custom items.