[wp-trac] [WordPress Trac] #54177: Add visibility to test class methods

[WordPress Trac]

#54177: Add visibility to test class methods
--------------------------------------+-------------------------------
 Reporter:  costdev                   |       Owner:  (none)
     Type:  enhancement               |      Status:  new
 Priority:  normal                    |   Milestone:  Future Release
Component:  Build/Test Tools          |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  coding-standards
--------------------------------------+-------------------------------

Comment (by costdev):

 > @jrf To allow for a PR to that effect in the WPCS repo, the Core coding
 standards handbook page would need to be updated first.
 >>I've looked into adding the four rules mentioned, but I'm not confident
 that I could write the updates to the handbook in an easily digestible
 way.
 > IMO no exclusion configuration should be introduced for these rules.
 >>Agreed - the more consistent the better!
 >>
 >>So from here, shall we consider this ticket's scope to apply not only to
 tests, but to all of Core?
 >>
 >>If so, I'll update the PR on this ticket with the outcome of the auto-
 fixer across Core (see below).
 >>If not, then the PR on this ticket is ready for review and I'll open
 another PR on #53359 to handle the rest of Core.
 >>
 >>Given that we're not going to be adding exclusion configuration, when
 updating the whole codebase are we also fixing files in
 `tests/phpunit/includes` for example?

 ----

 == Auto-Fixes

 I ran into an issue with the auto-fixer - it doesn't appear to have fixes
 for `Squiz.Scope.MethodScope` or `PSR2.Classes.PropertyDeclaration`. That
 or I'm missing something...

 Anyway, with a lack of awareness of any auto-fixes for these, I wrote some
 (added below for reference). For our case, they add `private` if an
 underscore is found, otherwise `public`.

 From there it was a case of doing a PHPUnit run and adjusting the
 `private` scopes to `protected` or `public` until the tests passed.

 **Spoiler Alert:** There were four, all in `wp-db.php` and all had to be
 made `public` in the end.

 ----
 == Sniff Fixes

 === Squiz.Scope.MethodScope
 Added
 [https://github.com/squizlabs/PHP_CodeSniffer/blob/b10c327dcf4f30151101858fa79053b6ecc99fbb/src/Standards/Squiz/Sniffs/Scope/MethodScopeSniff.php#L70
 at this location].

 {{{#!php
 <?php
             $phpcsFile->addFixableError($error, $stackPtr,
 'ScopeMissing');
             $methodName = $phpcsFile->getDeclarationName($stackPtr);

             if ('__' !== substr($methodName, 0, 2) && '_' ===
 $methodName[0]) {
                 $phpcsFile->fixer->addContentBefore($stackPtr, 'private
 ');
             } else {
                 // In general, these should still have a review to
 determine
                 // if they should actually have a 'private' scope.
                 // For now, all PHPUnit tests pass.
                 $phpcsFile->fixer->addContentBefore($stackPtr, 'public ');
             }
 }}}

 === PSR2.Classes.PropertyDeclaration

 Added
 [https://github.com/squizlabs/PHP_CodeSniffer/blob/b10c327dcf4f30151101858fa79053b6ecc99fbb/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php#L117
 at this location].

 {{{#!php
 <?php
             $phpcsFile->addFixableError($error, $stackPtr,
 'ScopeMissing');
             $propertyName = $tokens[$stackPtr]['content'];

             if ('_' === $propertyName[1]) {
                 $phpcsFile->fixer->addContentBefore($stackPtr, 'private
 ');
             } else {
                 // In general, these should still have a review to
 determine
                 // if they should actually have a 'private' scope.
                 // For now, all PHPUnit tests pass.
                 $phpcsFile->fixer->addContentBefore($stackPtr, 'public ');
             }
 }}}

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54177#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform