[wp-trac] [WordPress Trac] #63268: PHPStan code quality improvements for 6.9

[WordPress Trac]

#63268: PHPStan code quality improvements for 6.9
----------------------------+-------------------------------
 Reporter:  justlevine      |       Owner:  SergeyBiryukov
     Type:  task (blessed)  |      Status:  accepted
 Priority:  normal          |   Milestone:  6.9
Component:  General         |     Version:
 Severity:  normal          |  Resolution:
 Keywords:  has-patch       |     Focuses:  coding-standards
----------------------------+-------------------------------

Comment (by justlevine):

 > Maybe a specific task should be set for this purpose... But we cannot
 restrict this to PHPStan violations,

 @SirLouen not sure I follow.

 This ticket ''is'' the specific task set for triaging PHPStan issues. It
 was birthed as a request from
 https://core.trac.wordpress.org/ticket/52217#comment:112 which was also
 specific to PHPStan. (If I understood) you're suggesting that we rename
 and repurpose this ticket instead of opening up a new one to address
 broader "Code Quality" concerns. I'm suggesting a separate ticket can be
 created ''alongside'' this one (or after closing this one if that's what
 Sergey prefers, but until the PHPStan proposal is officially accepted
 there's value in keeping the history on this ticket specific and
 relevant).

 > don't agree that PHPStan violations are easier to triage
 Easier to triage ''than what''?

 My argument wasn't specific to PHPStan, it's "having a scoped list of
 errors that need evaluation". We already agree that 1) PHPStan is a
 helpful tool and 2) just because PHPStan flags an error doesn't mean we
 should listen to it. Having that scope filters out any other (possibly
 justifiable, but still irrelevant here) topics so it's focused on
 answering "is this PHPStan error worth remediating or just noise", then as
 with any core workflow there's obviously still independent context and
 code review or the proposed remediation.

 > don't even know if with this level, errors that are being spotted in the
 wild
 IIRC both on this ticket and on #52217 we caught some 6.8 regressions. If
 6.9 is similar we'll see a good number from GB and new features once we
 hit beta1.

 There's also the bugs that are hidden by less important PHPStan errors.
 For example, it can't alert you that about `str_starts_with(
 $not_a_string, $also_not_a_string )` throwing a fatal error, if it's
 warning you that "The PHPDoc for `$not_a_string` says `string|null`, but
 it's actually a `WP_Post|WP_Error`.

 If there's something I can document better or otherwise do to assist here,
 please let me know. The errors in the baseline are pretty self-
 documenting, and I tried to be as detailed about usage in the [PR
 description](https://github.com/WordPress/wordpress-develop/pull/7619).
 All you should really need to do is

 1. Checkout the PR and `composer install` to get the deps.
 2. Comment out one of the errors from one of the baselines (or an entire
 level from the root `phpstan.neon.dist` if you're feeling frisky)
 3. Run `composer run analyse -- -vv` (the first time will take a bit, but
 future runs are cached)
 4. Hop over to the location of the error shown and ''decide'' if/how to
 remediate it.

 You can also take a look at my remediation branch on
 https://github.com/WordPress/wordpress-develop/compare/trunk...justlevine
 :wordpress-develop:chore/phpstan/level-0 . I've been working there
 holistically (to help make sure that the remediations make sense in a
 larger context) and then cherry-pick out to a clean PR against `truck` the
 ones that are least likely to disrupt work-in-progress but are still
 independently justifiable changes.

 > PS: It appears that they don't agree. 🙊

 Heh, I was unnecessarily pedantic ''and'' wrong 💀 thanks for the catch!
 I'm rereading and have no idea why I said that, I think I was trying to
 highlight the difference between linting by line and linting by following
 the executing flow but thats very clearly not what I wrote and I'm also
 not sure why I thought that was tangentially relevant so 🥴 ?

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