PCP Plugin Check#109
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens WordPress/PHPCS compliance across the plugin by applying automated PHPCS fixes plus several targeted manual refactors (escaping correctness, minor performance cleanups, safer redirects, and improved logging).
Changes:
- Replaced non-preferred patterns (e.g.,
sizeof()→count(), unnecessary string concatenation,wp_redirect()→wp_safe_redirect(),Class::method()→self::method()where appropriate). - Improved error handling/observability by logging previously swallowed exceptions and routing some debug output through
Tiny_Logger. - Adjusted mocking and metadata parsing to use
maybe_unserialize()(and updated PHPCS ruleset configuration).
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/helpers/wordpress.php | Adds a stubbed maybe_unserialize() to the test WordPress mocks. |
| src/views/optimization-chart.php | Simplifies string concatenation in inline SVG/CSS output. |
| src/views/compress-details.php | Fixes escaping usage and tweaks inline output for size labels/status text. |
| src/views/bulk-optimization.php | Replaces sizeof() and reduces repeated count() calls inside loops/conditions. |
| src/views/account-status-connected.php | Fixes incorrect esc_html() usage and slightly simplifies conditional rendering. |
| src/compatibility/as3cf/class-tiny-as3cf.php | Uses self:: calls and narrows hook callback accepted args/signature. |
| src/class-tiny-settings.php | Logs caught Tiny_Exceptions; corrects esc_html() usage and minor formatting. |
| src/class-tiny-plugin.php | Uses wp_safe_redirect() and adjusts comment formatting/docblock spacing. |
| src/class-tiny-picture.php | Uses self:: for static calls and aligns docblock parameter formatting. |
| src/class-tiny-php.php | Uses self:: for static calls in capability checks. |
| src/class-tiny-logger.php | Tweaks hook registration args, callback signature, and adds PHPCS ignore for error_log. |
| src/class-tiny-image.php | Routes a debug error_log() through Tiny_Logger. |
| src/class-tiny-image-size.php | Renames private memoized properties (removes leading underscores). |
| src/class-tiny-helpers.php | Cleans up docblock formatting (alignment/indentation). |
| src/class-tiny-compress.php | Adjusts docblock alignment/formatting for method parameters. |
| src/class-tiny-compress-fopen.php | Adds PHPCS justification for base64_encode() usage in auth header. |
| src/class-tiny-compress-client.php | Escapes Tinify exception messages before wrapping in Tiny_Exception; comment formatting. |
| src/class-tiny-cli.php | Removes extra blank line in docblock. |
| src/class-tiny-bulk-optimization.php | Moves counts outside loops and switches unserialize() to maybe_unserialize(). |
| src/class-tiny-apache-rewrite.php | Docblock spacing/alignment tweaks. |
| phpcs.xml | Switches ruleset ref and adds multiple excludes (including security-related sniffs). |
Comments suppressed due to low confidence (2)
src/class-tiny-compress-client.php:171
- Tiny_Exception messages are now HTML-escaped at the point they are thrown. These exceptions are also used in non-HTML contexts (CLI output, logs) and are already escaped at render time in admin notices; pre-escaping here can cause double-escaping and reduces message usability. Pass the raw $err->getMessage() into Tiny_Exception and escape only at the final output boundary.
} catch ( \Tinify\Exception $err ) {
$this->last_error_code = $err->status;
Tiny_Logger::error(
'client compress error',
array(
'error' => $err->getMessage(),
'status' => $err->status,
)
);
throw new Tiny_Exception(
esc_html( $err->getMessage() ),
get_class( $err ),
$err->status
);
src/class-tiny-compress-client.php:190
- Tiny_Exception messages are now HTML-escaped at the point they are thrown. These exceptions are also used in non-HTML contexts (CLI output, logs) and are already escaped at render time in admin notices; pre-escaping here can cause double-escaping and reduces message usability. Pass the raw $err->getMessage() into Tiny_Exception and escape only at the final output boundary.
\Tinify\createKey( $email, $options );
} catch ( \Tinify\Exception $err ) {
$this->last_error_code = $err->status;
throw new Tiny_Exception(
esc_html( $err->getMessage() ),
get_class( $err ),
$err->status
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rkoopmans
approved these changes
May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running PCP will give over 200 violations. Running PHPCS with WordPress Guidelines will give even more. To stay compliant with plug-in guidelines I have increased strictness and also solves some of the issues. More will be incoming.
Changes
Fixed automatically by PHPCS
c1d32ab
Manual Fixes