Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve some of the plugin check errors and warnings #2772

Merged
merged 6 commits into from
Jan 17, 2025

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Jan 16, 2025

Changes proposed in this Pull Request:

This PR resolves a substantial amount of the errors and warnings which are identified in the WordPress.org plugin check.

Changes resolve the following errors:

  • Use of heredoc and nowdoc syntax ("<<<") is not allowed; use standard strings or inline HTML instead
  • json_encode() is discouraged. Use wp_json_encode() instead.
  • rand() or mt_rand() is discouraged. Use the far less predictable wp_rand() instead.
  • Missing "License" in Plugin Header. Please update your Plugin Header with a valid GPLv2 (or later) compatible license.

Note

We are ignoring the following errors/warnings:

  • WordPress.Security.EscapeOutput.ExceptionNotEscaped - Exceptions can be escaped late when output, not when they are thrown, we have a specific exclusion for this in the phpcs.xml file
  • trademarked_term Plugin naming has been done in discussion with trademark owners
  • Caching in uninstall.php - only gets run once when uninstalling, we have a specific exclusion for this in the phpcs.xml file

We can address these at a later date if these rules ever become fully required.

Detailed test instructions:

  1. Build a zip file from this branch composer install && nvm use && npm run build
  2. Install the zip file on a test site
  3. Install plugin check
  4. Go to Tools > Plugin Check and run the check for the selected plugin
  5. Confirm the amount of errors is now reduced there should be about 50 less down to @ 250
  6. Check for any errors left if they are in the ignore list above

Linting php

  1. Run npm run lint:php
  2. Run npm run lint:php-tests
  3. Confirm we don't get any errors with the revised linting rules

Changelog entry

  • Tweak - Resolve some of the plugin check errors and warnings.

@mikkamp mikkamp self-assigned this Jan 16, 2025
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jan 16, 2025
@mikkamp mikkamp requested a review from a team January 16, 2025 16:30
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 76.74419% with 10 lines in your changes missing coverage. Please review.

Project coverage is 67.2%. Comparing base (f88648c) to head (b371832).
Report is 9 commits behind head on develop.

Files with missing lines Patch % Lines
src/DB/Table/ShippingRateTable.php 0.0% 2 Missing ⚠️
src/API/Google/Connection.php 0.0% 1 Missing ⚠️
src/Assets/ScriptAsset.php 0.0% 1 Missing ⚠️
src/ConnectionTest.php 0.0% 1 Missing ⚠️
src/Coupon/CouponSyncer.php 85.7% 1 Missing ⚠️
src/DB/Migration/Migration20211228T1640692399.php 0.0% 1 Missing ⚠️
src/DB/Query/ShippingRateQuery.php 0.0% 1 Missing ⚠️
src/Jobs/ActionSchedulerJobMonitor.php 0.0% 1 Missing ⚠️
...Jobs/Notifications/AbstractItemNotificationJob.php 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             develop   #2772      +/-   ##
============================================
+ Coverage       66.7%   67.2%    +0.6%     
- Complexity         0    4677    +4677     
============================================
  Files            316     480     +164     
  Lines           4922   19571   +14649     
  Branches        1204       0    -1204     
============================================
+ Hits            3282   13159    +9877     
- Misses          1503    6412    +4909     
+ Partials         137       0     -137     
Flag Coverage Δ
js-unit-tests ?
php-unit-tests 67.2% <76.7%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
google-listings-and-ads.php 75.0% <ø> (ø)
src/API/Google/AdsConversionAction.php 97.6% <100.0%> (ø)
src/API/Google/Middleware.php 95.7% <100.0%> (ø)
src/API/WP/NotificationsService.php 95.5% <100.0%> (ø)
src/DB/Table/AttributeMappingRulesTable.php 46.7% <100.0%> (ø)
src/DB/Table/BudgetRecommendationTable.php 96.4% <100.0%> (ø)
src/DB/Table/MerchantIssueTable.php 40.0% <100.0%> (ø)
src/DB/Table/ShippingTimeTable.php 50.0% <100.0%> (ø)
src/MerchantCenter/MerchantStatuses.php 76.2% <100.0%> (ø)
src/Notes/LeaveReviewActionTrait.php 100.0% <100.0%> (ø)
... and 14 more

... and 772 files with indirect coverage changes

Copy link
Member

@eason9487 eason9487 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for resolving these plugin check errors. Confirmed the listed four categories of errors have been resolved. LGTM.

I'm guessing these two errors will also be ignored for now as they are test files?

  • WordPress.Security.EscapeOutput.OutputNotEscaped
  • PluginCheck.CodeAnalysis.Localhost.Found

image

@mikkamp
Copy link
Contributor Author

mikkamp commented Jan 17, 2025

I'm guessing these two errors will also be ignored for now as they are test files?

Yes. They also don't show up in the report when you test a release version of the plugin, as we don't bundle the tests folder.

@mikkamp mikkamp merged commit 9949c75 into develop Jan 17, 2025
13 checks passed
@mikkamp mikkamp deleted the tweak/plugin-check-errors-and-warnings branch January 17, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants