Skip to content

Fix PHPCS and PHPStan issues across multiple files #818

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.

Sign up for GitHub

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

Open
aslamdoctor wants to merge 7 commits into
base: master
Choose a base branch
from
+33 −27
Open

Fix PHPCS and PHPStan issues across multiple files #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7171654
Fix PHPCS and PHPStan issues across multiple files
aslamdoctor Feb 26, 2026
19d592e
Revert FIDO/U2F file changes per PR #818 review
aslamdoctor Mar 1, 2026
b1975bc
Add early return guards for false $user in email provider
aslamdoctor Mar 1, 2026
fb934b8
Consolidate plugin constants into a shared file
aslamdoctor Mar 3, 2026
ac5d59a
Move constants before ABSPATH guard and bump PHPStan to level 4
aslamdoctor Mar 3, 2026
464f782
Revert PHPStan level back to 0
aslamdoctor Mar 3, 2026
ac3dd5f
Remove unnecessary PHPStan ignore comment in compat class
aslamdoctor Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ public static function is_api_request() {
*
* @since 0.2.0
*
* @param WP_User $user WP_User object of the logged-in user.
* @param WP_User|false $user WP_User object of the logged-in user.
*/
public static function show_two_factor_login( $user ) {
if ( ! $user ) {
Expand Down Expand Up @@ -1750,9 +1750,9 @@ public static function _login_form_revalidate_2fa( $nonce = '', $provider = '',
*
* @since 0.9.0
*
* @param object $provider The Two Factor Provider.
* @param WP_User $user The user being authenticated.
* @param bool $is_post_request Whether the request is a POST request.
* @param object|null $provider The Two Factor Provider.
* @param WP_User $user The user being authenticated.
* @param bool $is_post_request Whether the request is a POST request.
* @return false|WP_Error|true WP_Error when an error occurs, true when the user is authenticated, false if no action occurred.
*/
public static function process_provider( $provider, $user, $is_post_request ) {
Expand Down Expand Up @@ -2059,7 +2059,7 @@ public static function user_two_factor_options( $user ) {
<h2><?php esc_html_e( 'Two-Factor Options', 'two-factor' ); ?></h2>

<?php foreach ( $notices as $notice_type => $notice ) : ?>
<div class="<?php echo esc_attr( $notice_type ? 'notice inline notice-' . $notice_type : '' ); ?>">
<div class="<?php echo esc_attr( 'notice inline notice-' . $notice_type ); ?>">
<p><?php echo wp_kses_post( $notice ); ?></p>
</div>
<?php endforeach; ?>
Expand Down
4 changes: 4 additions & 0 deletions phpstan.dist.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ parameters:
- class-two-factor-compat.php
- class-two-factor-core.php
- two-factor.php
excludePaths:
- providers/class-two-factor-fido-u2f.php
- providers/class-two-factor-fido-u2f-admin.php
- providers/class-two-factor-fido-u2f-admin-list-table.php
18 changes: 13 additions & 5 deletions providers/class-two-factor-email.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public function generate_and_email_token( $user ) {
*
* @since 0.1-dev
*
* @param WP_User $user WP_User object of the logged-in user.
* @param WP_User|false $user WP_User object of the logged-in user.
*/
public function authentication_page( $user ) {
if ( ! $user ) {
Expand Down Expand Up @@ -391,11 +391,15 @@ public function authentication_page( $user ) {
*
* @since 0.2.0
*
* @param WP_User $user WP_User object of the logged-in user.
* @param WP_User|false $user WP_User object of the logged-in user.
* @return boolean
*/
public function pre_process_authentication( $user ) {
if ( isset( $user->ID ) && isset( $_REQUEST[ self::INPUT_NAME_RESEND_CODE ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- non-distructive option that relies on user state.
if ( ! $user ) {
return false;
}

if ( isset( $_REQUEST[ self::INPUT_NAME_RESEND_CODE ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- non-distructive option that relies on user state.
$this->generate_and_email_token( $user );
return true;
}
Expand All @@ -408,12 +412,16 @@ public function pre_process_authentication( $user ) {
*
* @since 0.1-dev
*
* @param WP_User $user WP_User object of the logged-in user.
* @param WP_User|false $user WP_User object of the logged-in user.
* @return boolean
*/
public function validate_authentication( $user ) {
if ( ! $user ) {
return false;
}

$code = $this->sanitize_code_from_request( 'two-factor-email-code' );
if ( ! isset( $user->ID ) || ! $code ) {
Copy link
Collaborator

@kasparsd kasparsd Mar 3, 2026

Choose a reason for hiding this comment

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

question: Is phpstan complaining about the isset( $user->ID ) when the $user is false or not WP_User?

Copy link
Contributor Author

@aslamdoctor aslamdoctor Mar 3, 2026

Choose a reason for hiding this comment

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

PHPStan at level 0 doesn't flag the isset( $user->ID ) pattern — it would only become an issue at higher levels (accessing a property on false). The early return guards were added as a code quality improvement to make the false handling explicit and keep the @param docblock accurate (WP_User|false). Happy to revert these changes if you'd prefer keeping the original isset() approach.

if ( ! $code ) {
return false;
}

Expand Down
10 changes: 3 additions & 7 deletions providers/class-two-factor-totp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,6 @@ public static function generate_qr_code_url( $user, $secret_key ) {
* @codeCoverageIgnore
*/
public function user_two_factor_options( $user ) {
if ( ! isset( $user->ID ) ) {
return;
}

$key = $this->get_user_totp_key( $user->ID );

wp_enqueue_script( 'two-factor-qr-code-generator' );
Expand Down Expand Up @@ -720,11 +716,11 @@ public static function pack64( int $value ): string {
if ( 8 === PHP_INT_SIZE ) {
return pack( 'J', $value );
}

// 32-bit PHP fallback
$higher = ( $value >> 32 ) & 0xFFFFFFFF;
$lower = $value & 0xFFFFFFFF;

return pack( 'NN', $higher, $lower );
}

Expand Down Expand Up @@ -890,7 +886,7 @@ public static function base32_encode( $string ) {
$base32_string = '';

foreach ( $five_bit_sections as $five_bit_section ) {
$base32_string .= self::$base_32_chars[ base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
$base32_string .= self::$base_32_chars[ (int) base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
}

return $base32_string;
Expand Down
18 changes: 8 additions & 10 deletions two-factor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,17 @@
* Network: True
*/

if ( ! defined( 'ABSPATH' ) ) {
exit; // Exit if accessed directly.
if ( ! defined( 'TWO_FACTOR_DIR' ) ) {
define( 'TWO_FACTOR_DIR', __DIR__ . '/' );
}

/**
* Shortcut constant to the path of this file.
*/
define( 'TWO_FACTOR_DIR', plugin_dir_path( __FILE__ ) );
if ( ! defined( 'TWO_FACTOR_VERSION' ) ) {
define( 'TWO_FACTOR_VERSION', '0.15.0' );
}

/**
* Version of the plugin.
*/
define( 'TWO_FACTOR_VERSION', '0.15.0' );
if ( ! defined( 'ABSPATH' ) ) {
exit; // Exit if accessed directly.
}

/**
* Include the base class here, so that other plugins can also extend it.
Expand Down