Skip to content

chore: add php lint, phpcs and phpstan - #4

Merged
vitormattos merged 6 commits into
mainfrom
chore/static-analysis
Sep 26, 2026
Merged

vitormattos merged 6 commits into
mainfrom
chore/static-analysis

Conversation

@YvesCesar

@YvesCesar YvesCesar commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

The plugin had no Composer setup, no lint, no coding standard and no CI beyond DCO. This adds PHP lint, PHPCS and PHPStan, each as a Composer script and a GitHub workflow.

Tooling

  • composer lint, composer cs, composer stan and composer ci (all three, in this order).
  • Each tool lives in its own vendor-bin/ project (bamarni/composer-bin-plugin), so their dependencies do not mix.
  • One workflow per tool: php-lint.yml, phpcs.yml, phpstan.yml. The PHP versions come from composer.json (>=8.3 <8.6) through typisttech/php-matrix-action, so the matrix is 8.3, 8.4 and 8.5.
  • Third-party actions are pinned by commit SHA, with the version next to it.
  • PHPCS uses a lean ruleset (security, database, deprecated APIs, i18n, global prefixes and PHP compatibility) instead of the full WordPress-Extra, so the diff stays reviewable. PHPStan runs at level 5 with the WordPress extension.
  • The plugin header now declares Requires at least: 7.0 and Requires PHP: 8.3, the versions the checks run against.

Changes to the plugin

  • Prefix: functions used two prefixes, librecode_simple_smtp_ and wpss_. They now all use librecode_simple_smtp_, the one the function_exists() guard already checks. The admin page slug, the nonce and the form field names keep their current values, so URLs and the form are unchanged. Code that unhooks the old names, such as remove_filter( 'wp_mail_from', 'wpss_change_mail_from' ), silently stops working; a GitHub code search finds no such reference outside this repository.
  • Escaping: the field names and descriptions on the settings page are printed through esc_attr()/esc_html(). They are fixed strings, so the rendered page is the same.
  • Slashes in the saved settings: WordPress adds slashes to $_POST and sanitize_text_field() keeps them, so saving the password pa'ss stored pa\'ss, which breaks SMTP authentication. The value goes back into the form, so every later save added more slashes (pa\\\'ss), and O'Brien in smtp_name reached the From header as O\'Brien. The submitted values now go through wp_unslash() before sanitizing, which is also what PHPCS asked for.
  • Test email without nonce: the test email form prints a nonce, but only the save branch checked it. A page on another site could make a logged-in admin's browser post the form and send email through the site's SMTP to any address. The test email branch now calls check_admin_referer() with the same nonce. PHPCS did not flag this because its nonce sniff accepts any verification earlier in the same function, including the one inside the save branch.
  • Hidden test email page: PHPStan flagged add_submenu_page() with a null parent and a callback that was never defined. Opening wp-admin/admin.php?page=wpss-test-email ended in a fatal error (call_user_func_array(): Argument #1 ($callback) must be a valid callback). The test email form already lives on the settings page, so the hidden page was removed.

Verification

  • composer ci passes locally.
  • On a WordPress 7.0 install with this branch, submitting the settings form three times keeps pa'ss and O'Brien unchanged (before: one more level of slashes per save); a test email post without the nonce stops at wp_die() and sends nothing, and with the nonce it is sent.
  • On a WordPress 7.0 install with this branch: wp_mail() goes out over SMTP with the configured host, port and From, and arrives in Mailpit; the settings page renders all 13 fields.

Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
…nd php versions

Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
@vitormattos
vitormattos merged commit a1fc2e9 into main Sep 26, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants