CS/QA: make (nearly) all classes final
#2200
Conversation
The PHPCS autoloader does not always play nice with sniffs which extend other sniffs, especially if both sniffs (parent and child) are included in the same standard. To discourage sniffs extending each other, I propose making all non-abstract sniff classes `final`with the exception of four classes of which it is known that those are being extended based on a public code search. The four exceptions are: * `NamingConventions.ValidHookName` * `Security.EscapeOutput` * `Security.NonceVerification` * `Security.ValidatedSanitizedInput` If needs be, a sniff in an external standard can still _use_ logic from the WPCS sniffs. On the one hand, they can use the new `Helper` classes and traits. On the other hand, they can do a preliminary check on certain data and defer to a new or preinstantiated instance of a WPCS sniff for everything else. Note: this list of sniffs being extended is likely incomplete. If reports come in from external standards that they are running into trouble using alternative logic, we can still remove the `final` keyword from select sniffs in a patch release. However, adding the `final` keyword needs to be done in a major. Ref: * https://sourcegraph.com/search?q=context:global+use+WordPressCS+-repo:%5Egithub%5C.com/WordPress/WordPress-Coding-Standards%24+-repo:%5Egithub%5C.com/WPTT/WPThemeReview%24+-file:%5E%28.*/%29%3Fvendor/wp-coding-standards/.*%24&patternType=standard&case=yes&sm=0
... for the files in this package.
When I was tinkering before, I think I had trouble loading these from WPCS, so we might need some documentation here on what, if anything, needs to be done to make these available to external standards. |
If both standards are registered with PHPCS (which the Composer plugin takes care of) and if all files basically follow the namespace scheme which the PHPCS autoloader expects, everything should work.... I'd need to see the errors you were getting in VIPCS to advise on how to fix that, but that'll have to wait for now. |
QA: make all test classes final
QA: make all non-abstract sniff classes final
The PHPCS autoloader does not always play nice with sniffs which extend other sniffs, especially if both sniffs (parent and child) are included in the same standard.
To discourage sniffs extending each other, I propose making all non-abstract sniff classes
finalwith the exception of four classes of which it is known that those are being extended based on a public code search.The four exceptions are:
NamingConventions.ValidHookNameSecurity.EscapeOutputSecurity.NonceVerificationSecurity.ValidatedSanitizedInputIf needs be, a sniff in an external standard can still use logic from the WPCS sniffs. On the one hand, they can use the new
Helperclasses and traits. On the other hand, they can do a preliminary check on certain data and defer to a new or preinstantiated instance of a WPCS sniff for everything else.Note: this list of sniffs being extended is likely incomplete.
If reports come in from external standards that they are running into trouble using alternative logic, we can still remove the
finalkeyword from select sniffs in a patch release. However, adding thefinalkeyword needs to be done in a major.Ref:
PHPCS: enforce classes to be abstract or final
... for the files in this package.