[wp-trac] [WordPress Trac] #61175: Integrate PHPStan into the core development workflow

[WordPress Trac]

#61175: Integrate PHPStan into the core development workflow
--------------------------------------+-------------------------
 Reporter:  westonruter               |       Owner:  justlevine
     Type:  enhancement               |      Status:  assigned
 Priority:  normal                    |   Milestone:  6.9
Component:  General                   |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+-------------------------

Comment (by justlevine):

 @dmsnell if I understand you correctly you seem to be saying that adopting
 PHPStan as a linting too means we need auto-accept/reject/anything, but I
 don't think that's the case.

 The workflow as built into [https://github.com/WordPress/wordpress-
 develop/pull/7619|PR #7619] is that:
 1. PHPStan/the CI surfaces _new_ errors.
 2. When faced with a PHPStan error, it can be triaged by:
 - fixing the error
 - `// @phpstan-ignore rule.type (Reason it's not a valid error)` to
 suppress a false-positive _in file_.
 - Adding it to the "baseline" of existing tech debt. This is like saying
 "We'll triage and decide how to handle it later"
 - adding it to our `ignoreErrors` ruleset. This is the equivalent of
 saying "this entire rule is invalid".

 So yeah at no point should this devolve into some automated
 "approve/reject mode without discussion" 🙏🤞


 > Has anyone collected data on which changes would have been rejected had
 we been employing this for the past year or so? It’d be informative to
 know the breakdown of caught errors that were missed in review against
 rejected valid work.

 So while like I said nothing would be *rejected*, but as for getting a
 feel for would be **flagged** as I tried and I guess failed to explain in
 the message [https://core.trac.wordpress.org/ticket/61175#comment:38|right
 above yours]:

 - the [https://github.com/justlevine/wordpress-
 develop/tree/feat/phpstan/tests/phpstan/baseline baselines] to see what
 actual existing tech-debt is
  being flagged, and the commit history on those baselines to see how
 remediation/tweaking the rulesets for WP for those rules are handled. The
 counts from that are what I've been sharing in this thread, and if you
 check the instructions on the PR, you'll see instructions on how to
 comment out a baseline or an error to see the LOC the violation is on as
 part of a remediation workflow.
 - my [https://github.com/justlevine/wordpress-
 develop/tree/chore/phpstan/level-0/| remediation branch] that I've been
 cherrypicking from. You can see how e.g. using `@phpstan-return`
 conditionals or asserts both narrow down the false positives _and_ improve
 the code type-safety, or how `// @phpstan-ignore ` is used to handle a
 false positive in a similar way to PHPCS. And all the individual
 remediations that I've been cherrypicking onto #63268 (and will hopefully
 become a blessed task and I can pair with a committer and batch them into
 larger PRs)

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