Audit Public Sniff Properties

[GaryJones]

What problem would the enhancement address for VIP?

There are 47 instances of public properties within sniff classes. Public properties allow changing sniff behaviour by customising a ruleset by passing different types of values via the ruleset. VIP uses itself to adjust how a WPCS sniff runs.

However, I suspect that some of the instances are NOT meant to be changed - whitelisted Batcache params, restricted constant names and declarations, etc.

Describe the solution you'd like

Audit the uses and convert to private where suitable. Document the rest on a wiki page.

I'll create a linked list of the 47 instances, possibly along with who added them, to check if they are meant to be public or not.

For those that are meant to be changeable, then a better approach for some cases is to have a private default list of values, and then a public custom list, and the two lists merged together - that way, the custom rulesets don't need to also add in the default values as well.

Additional context

Note that this would technically be a breaking change, since it is possible that someone was already making use of these unintentional public properties in their own rulesets. As such, this would require a bump from 0.y.z to 0.y+1.0, or a normal major release if 1.0.0 has been released in the meantime.

[GaryJones]

One approach here would be to initially make all of the public properties private, and then introduce other public properties if needed, and have a process for merging these two sets of data together. One example is a simple case in WPCS.

[GaryJones]

Here are all of the public properties. Some of them will be false poitives (i.e. should be left as public):

• AbstractVariableRestrictionsSniff.php
  • public $exclude;
• Classes/DeclarationCompatibilitySniff.php
  • public $checkClasses;
  • public $checkClassesGroups;
• Constants/RestrictionConstantssSniff.php
  • public $restrictedConstantNames;
  • public $restrictedConstantDeclaration;
• Files/IncludingFileSniff.php
  • public $getPathFunctions;
  • public $restrictedConstants;
  • public $allowedConstants;
  • public $slashingFunctions;
• Functions/CheckReturnValueSniff.php
  • public $catch;
  • public $notFunctions;
• JS/DangerouslySetInnerHTMLSniff.php
  • public $supportedTokenizers;
• JS/HTMLExecutingFunctionsSniff.php
  • public $HTMLExecutingFunctions;
  • public $supportedTokenizers;
• JS/InnerHTMLSniff.php
  • public $supportedTokenizers;
• JS/StringConcatSniff.php
  • public $supportedTokenizers;
• JS/StrippingTagsSniff.php
  • public $supportedTokenizers;
• JS/WindowSniff.php
  • public $supportedTokenizers;
• Performance/BatcacheWhitelistedParamsSniff.php
  • public $whitelistes_batcache_params;
• Performance/TaxonomyMetaInOptionsSniff.php
  • public $option_functions;
  • public $taxonomy_term_patterns;
• Security/MustacheSniff.php
  • public $supportedTokenizers;
• Security/ProperEscapingFunctionSniff.php
  • public $escaping_functions;
• Security/TwigSniff.php
  • public $supportedTokenizers;
• Security/UnderscorejsSniff.php
  • public $supportedTokenizers;
• Security/VuejsSniff.php
  • public $supportedTokenizers;
• UserExperience/AdminBarRemovalSniff.php
  • public $supportedTokenizers;
  • public $remove_only;
• Variables/ServerVariablesSniff.php
  • public $restrictedVariables;
• Variables/VariableAnalysisHelper.php
  • public $owner;
  • public $opener;
  • public $closer;
  • public $variables;
  • public $name;
  • public $scopeType;
  • public $typeHint;
  • public $passByReference;
  • public $firstDeclared;
  • public $firstInitialized;
  • public $firstRead;
  • public $ignoreUnused;
• Variables/VariableAnalysisSniff.php
  • public $sitePassByRefFunctions;
  • public $allowUnusedCaughtExceptions;
  • public $allowUnusedFunctionParameters;
  • public $validUnusedVariableNames;
• VersionControl/MergeConflictSniff.php
  • public $supportedTokenizers;

[GaryJones]

Working list (will be edited) for properties not yet checked or excluded:

• AbstractVariableRestrictionsSniff.php
  • public $exclude;
• Classes/DeclarationCompatibilitySniff.php
  • public $checkClasses;
  • public $checkClassesGroups;
• Constants/RestrictionConstantssSniff.php
  • public $restrictedConstantNames;
  • public $restrictedConstantDeclaration;
• Files/IncludingFileSniff.php
  • public $getPathFunctions;
  • public $restrictedConstants;
  • public $allowedConstants;
  • public $slashingFunctions;
• Functions/CheckReturnValueSniff.php
  • public $catch;
  • public $notFunctions;
• JS/HTMLExecutingFunctionsSniff.php
  • public $HTMLExecutingFunctions;
• Performance/BatcacheWhitelistedParamsSniff.php
  • public $whitelistes_batcache_params;
• Performance/TaxonomyMetaInOptionsSniff.php
  • public $option_functions;
  • public $taxonomy_term_patterns;
• Security/ProperEscapingFunctionSniff.php
  • public $escaping_functions;
• UserExperience/AdminBarRemovalSniff.php
  • public $remove_only;
• Variables/ServerVariablesSniff.php
  • public $restrictedVariables;
• Variables/VariableAnalysisHelper.php
  • public $owner;
  • public $opener;
  • public $closer;
  • public $variables;
  • public $name;
  • public $scopeType;
  • public $typeHint;
  • public $passByReference;
  • public $firstDeclared;
  • public $firstInitialized;
  • public $firstRead;
  • public $ignoreUnused;
• Variables/VariableAnalysisSniff.php
  • public $sitePassByRefFunctions;
  • public $allowUnusedCaughtExceptions;
  • public $allowUnusedFunctionParameters;
  • public $validUnusedVariableNames;

[GaryJones]

At this point, I don't think we've seen a sufficient number of users trying to amend public property values to the point where the expected default content is not present.

While it would be nice to be confident that some minimum default values are present, that users can supplement, I don't think that adding this in, in a more formalised way, is the best use of time right now.

Can be re-opened again if the issue is deemed worthy, or individual properties can be dealt with on a case-by-case basis.

[jrfnl]

Proposal: for those properties for which the visibility ought to be changes, add the new private properties in VIPCS 2.x and throw a deprecation warning if the value of the public property is not the same as the private one (= changed from within a custom ruleset).

That way customers, who would be (ab)using the fact that these properties are currently public, would get some warning about this upcoming change.