Upgrade wpcs #314
Upgrade wpcs #314
Conversation
Also upgrades to vipcs 3.0 for necessary changes there.
|
Looks like the duplicate ignore checks may not be working as I think |
These sniffs have been renamed upstream; I'm updating the ignore refernces to them here to avoid errors parsing the HM ruleset.
`customPropertyWhitelist` has been renamed to `allowed_custom_properties`.
| * | ||
| * @see https://github.com/WordPress/WordPress-Coding-Standards/issues/1864 | ||
| */ | ||
| #[AllowDynamicProperties] |
There was a problem hiding this comment.
@goldenapples Is this necessary? Seems like we should know the properties being used?
There was a problem hiding this comment.
Huh. Yeah, this was pretty hacky.
The problem is that $printingFunctions is changed in the parent class from protected to private, so trying to unset items from $this->printingFunctions here is not going to work, and will trigger an accessing dynamic properties warning.
coding-standards/HM/Sniffs/Security/EscapeOutputSniff.php
Lines 54 to 56 in c5f69a8
This annotation silences the dynamic properties deprecation warning. It does not, however, make the code here work to ignore the safe printing functions.
|
@rmccue @goldenapples is there work still pending on this? It's unclear what the status of the PR is currently, from first glances it looks like Travis CI is the blocker. This is causing an issue on a client repo so I'd like to scope out what needs doing so we can action it |
|
@tomjn I've been using this in production on a project via tracking the |
| "type": "phpcodesniffer-standard", | ||
| "license": "GPL-2.0-or-later", | ||
| "require": { | ||
| "php": ">=7.1", |
There was a problem hiding this comment.
Should we bump the PHP requirement?
| "php": ">=7.1", | |
| "php": ">=8.0", |
|
@kadamwhite I've been in a similar boat, from my POV the only blocker left for this is the failing Travis tests for a missing trait |
|
I tried updating PHPBasicAuth with this branch and ran into the PHPCompatibility trim error. There's a v10 alpha that should fix this but given that it's an alpha would we even consider including that here? I know some projects have patches that work around the problem but fundamentally there are language syntax differences only v10 can account for |
Fixes #313.
Have not exhaustively checked all the differences here yet to see what breaks.