Skip to content

Phpcs reduce ignores - #955

Merged
masteradhoc merged 8 commits into
WordPress:masterfrom
peterwilsoncc:phpcs-reduce-ignores
Oct 20, 2026
Merged

masteradhoc merged 8 commits into
WordPress:masterfrom
peterwilsoncc:phpcs-reduce-ignores

Conversation

@peterwilsoncc

@peterwilsoncc peterwilsoncc commented Aug 10, 2026 •

Copy link
Copy Markdown
Collaborator

What?

Reducing phpcs:ignore and phpcs:disable instances.

Fixes #

Why?

It's not unknown that these come back cause problems later as code is removed.

How?

Modifies code to remove the need where possible.

Commonly:

  • moves sanitization to run early
  • moves nonce checks to run alongside form input. They're cheap so can be duplicated if needs be.

Use of AI Tools

N/A

Testing Instructions

TBA once this comes out of draft.

Screenshots or screencast

Before After

Changelog Entry

Development Update - Improve compliance with WordPress Coding Standards.

Open WordPress Playground Preview

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces inline PHPCS suppressions by restructuring input handling and nonce validation so that WordPress security sniffs can be satisfied without phpcs:ignore/phpcs:disable, while preserving existing behavior in the Two-Factor plugin’s settings and revalidation flows.

Changes:

  • Replaces $_GET['page'] usage (with nonce-verification ignore) by using get_current_screen() to detect the Two-Factor settings screen.
  • Introduces two_factor_is_valid_user_action() and updates core logic/tests to use it, while deprecating Two_Factor_Core::is_valid_user_action().
  • Sanitizes posted provider arrays earlier (settings page + user profile options) and adds PHPCS config for custom nonce verification helper.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
two-factor.php Uses get_current_screen() for settings-page detection and adds two_factor_is_valid_user_action() helper.
class-two-factor-core.php Deprecates is_valid_user_action() in favor of the new helper; adjusts request parsing and revalidation nonce checks.
settings/class-two-factor-settings.php Sanitizes enabled provider inputs inline to remove PHPCS suppressions.
tests/class-two-factor-core.php Updates coverage and assertions to target two_factor_is_valid_user_action().
phpcs.xml.dist Configures PHPCS nonce verification sniff to recognize the new helper as a custom nonce verification function.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread class-two-factor-core.php Outdated
Comment thread class-two-factor-core.php Outdated

@masteradhoc masteradhoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Just added the final versions to finalize this :)

Comment thread two-factor.php Outdated
Comment thread class-two-factor-core.php Outdated
Comment thread class-two-factor-core.php Outdated
@masteradhoc
masteradhoc marked this pull request as ready for review October 20, 2026 15:40
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-authored-by: masteradhoc <masteradhoc@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@masteradhoc
masteradhoc merged commit b5626e0 into WordPress:master Oct 20, 2026
38 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.

3 participants