[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 jrf):

 Replying to [comment:6 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.


 No worries. We'll sort that out when actioning the
 [https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-
 for-modern-php/ WPCS update for modern PHP], though that does mean that
 the sniffs won't be moved (from `Extra` to `Core`) until that's all
 sorted.

 See the "Visibility should always be declared" section in that dev note
 for more information.


 > > 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?

 Sounds good to me. Might be an idea to post a link to this ticket in
 #53359 for visibility.


 > - If so, I'll update the PR on this ticket with the outcome of the auto-
 fixer across Core (see below for more detail on the auto-fixer) and it'll
 be ready for review.
 > - 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.

 As this is potentially a huge patch, it might make for easier reviewing
 and quicker commits to submit this in chunks, `wp-includes`, `wp-admin`,
 `tests/phpunit/tests` etc.


 > 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?

 Absolutely, but please be careful not to break BC. When in doubt, always
 use `public`. To quote myself:

   If any visibility modifiers are missing in WordPress Core code, they
 should be set to `public` so as not to break backward-compatibility.

 Source: [https://make.wordpress.org/core/2020/03/20/updating-the-coding-
 standards-for-modern-php/ WPCS update for modern PHP]




 > == 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...

 Darn! Hmm.. I think that may have been over-cautiousness on the side of
 PHPCS.


 > 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`.

 For the actual tests, I'm fine with making the underscore prefixed methods
 `private`, everywhere else, they should still be made `public` so as not
 to break BC.

 While the methods may have been intended for WP internal use only, this
 doesn't mean that has been respected by all plugins/themes.

 No visibility means that the previous behaviour was `public`, so the
 method visibility should remain `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

 Have you got a branch somewhere in a fork with these changes ? I'd
 recommend a few small tweaks and can push those up to your fork if you
 like.

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