From 988baee9fa82648b3a3372ebd6ded4a24df8f1fd Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Thu, 15 Jan 2026 16:19:02 -0400 Subject: [PATCH 1/3] New sniff to prefer assert string contains string --- .../PreferAssertStringContainsSniff.php | 220 ++++++++++++++++++ phpcs-sniffs/Formidable/ruleset.xml | 1 + tests/phpunit/database/test_FrmMigrate.php | 6 +- .../misc/test_FrmSimpleBlocksController.php | 2 +- tests/phpunit/styles/test_FrmStylesHelper.php | 4 +- 5 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php diff --git a/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php new file mode 100644 index 0000000000..b60a20c78d --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php @@ -0,0 +1,220 @@ +assertTrue(str_contains(...)) and converts to $this->assertStringContainsString(...). + * Note: str_contains($haystack, $needle) becomes assertStringContainsString($needle, $haystack). + * + * @package Formidable\Sniffs + */ + +namespace Formidable\Sniffs\PHPUnit; + +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Files\File; + +/** + * Converts assertTrue(str_contains()) to assertStringContainsString(). + * + * Bad: + * $this->assertTrue( str_contains( $haystack, $needle ) ); + * + * Good: + * $this->assertStringContainsString( $needle, $haystack ); + */ +class PreferAssertStringContainsSniff implements Sniff { + + /** + * Returns an array of tokens this test wants to listen for. + * + * @return array + */ + public function register() { + return array( T_STRING ); + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * + * @return void + */ + public function process( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + $token = $tokens[ $stackPtr ]; + + // Check for assertTrue or assertFalse. + $methodName = strtolower( $token['content'] ); + + if ( 'asserttrue' !== $methodName && 'assertfalse' !== $methodName ) { + return; + } + + // Check that this is a method call ($this-> or self:: or static::). + $prevToken = $phpcsFile->findPrevious( T_WHITESPACE, $stackPtr - 1, null, true ); + + if ( false === $prevToken ) { + return; + } + + if ( $tokens[ $prevToken ]['code'] !== T_OBJECT_OPERATOR && $tokens[ $prevToken ]['code'] !== T_DOUBLE_COLON ) { + return; + } + + // Find the opening parenthesis. + $openParen = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true ); + + if ( false === $openParen || $tokens[ $openParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return; + } + + // Find the first argument (skip whitespace). + $firstArg = $phpcsFile->findNext( T_WHITESPACE, $openParen + 1, null, true ); + + if ( false === $firstArg ) { + return; + } + + // Check if first argument is str_contains function call. + if ( $tokens[ $firstArg ]['code'] !== T_STRING ) { + return; + } + + $functionName = strtolower( $tokens[ $firstArg ]['content'] ); + + if ( 'str_contains' !== $functionName ) { + return; + } + + // Find the opening parenthesis of str_contains. + $funcOpenParen = $phpcsFile->findNext( T_WHITESPACE, $firstArg + 1, null, true ); + + if ( false === $funcOpenParen || $tokens[ $funcOpenParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return; + } + + // Find the matching closing parenthesis of str_contains. + if ( ! isset( $tokens[ $funcOpenParen ]['parenthesis_closer'] ) ) { + return; + } + $funcCloseParen = $tokens[ $funcOpenParen ]['parenthesis_closer']; + + // Find the closing parenthesis of assertTrue/assertFalse. + if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) { + return; + } + $assertCloseParen = $tokens[ $openParen ]['parenthesis_closer']; + + // Check that str_contains is the only argument (no && or || after it). + $nextAfterFunc = $phpcsFile->findNext( T_WHITESPACE, $funcCloseParen + 1, $assertCloseParen, true ); + + if ( false !== $nextAfterFunc ) { + // There's something else after the str_contains call - skip this. + return; + } + + // Parse the two arguments of str_contains. + $args = $this->parseArguments( $phpcsFile, $funcOpenParen, $funcCloseParen ); + + if ( count( $args ) !== 2 ) { + return; + } + + // Determine the new method name. + $newMethodName = 'asserttrue' === $methodName ? 'assertStringContainsString' : 'assertStringNotContainsString'; + + $fix = $phpcsFile->addFixableError( + 'Use %s() instead of %s(str_contains()).', + $stackPtr, + 'Found', + array( $newMethodName, $token['content'] ) + ); + + if ( true === $fix ) { + $this->applyFix( $phpcsFile, $stackPtr, $openParen, $assertCloseParen, $newMethodName, $args ); + } + } + + /** + * Parse the arguments of a function call. + * + * @param File $phpcsFile The file being scanned. + * @param int $funcOpenParen The opening parenthesis position. + * @param int $funcCloseParen The closing parenthesis position. + * + * @return array Array of argument strings. + */ + private function parseArguments( File $phpcsFile, $funcOpenParen, $funcCloseParen ) { + $tokens = $phpcsFile->getTokens(); + $args = array(); + $start = $funcOpenParen + 1; + $depth = 0; + + $currentArg = ''; + + for ( $i = $start; $i < $funcCloseParen; $i++ ) { + $tokenCode = $tokens[ $i ]['code']; + + // Track nested parentheses. + if ( $tokenCode === T_OPEN_PARENTHESIS || $tokenCode === T_OPEN_SQUARE_BRACKET ) { + $depth++; + $currentArg .= $tokens[ $i ]['content']; + } elseif ( $tokenCode === T_CLOSE_PARENTHESIS || $tokenCode === T_CLOSE_SQUARE_BRACKET ) { + $depth--; + $currentArg .= $tokens[ $i ]['content']; + } elseif ( $tokenCode === T_COMMA && 0 === $depth ) { + // End of argument. + $args[] = trim( $currentArg ); + $currentArg = ''; + } else { + $currentArg .= $tokens[ $i ]['content']; + } + } + + // Add the last argument. + $trimmed = trim( $currentArg ); + if ( '' !== $trimmed ) { + $args[] = $trimmed; + } + + return $args; + } + + /** + * Apply the fix to convert assertTrue(str_contains($haystack, $needle)) to assertStringContainsString($needle, $haystack). + * + * @param File $phpcsFile The file being scanned. + * @param int $methodNamePtr The assertTrue/assertFalse token position. + * @param int $openParen The opening parenthesis of assertTrue. + * @param int $assertCloseParen The closing parenthesis of assertTrue. + * @param string $newMethodName The new method name. + * @param array $args The parsed arguments (haystack, needle). + * + * @return void + */ + private function applyFix( File $phpcsFile, $methodNamePtr, $openParen, $assertCloseParen, $newMethodName, $args ) { + $fixer = $phpcsFile->fixer; + + $fixer->beginChangeset(); + + // Replace the method name. + $fixer->replaceToken( $methodNamePtr, $newMethodName ); + + // Replace everything between the parentheses with swapped arguments. + // str_contains($haystack, $needle) -> assertStringContainsString($needle, $haystack) + $haystack = $args[0]; + $needle = $args[1]; + + // Remove all tokens between open and close paren. + for ( $i = $openParen + 1; $i < $assertCloseParen; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + + // Insert the new content after the opening paren. + $fixer->addContent( $openParen, ' ' . $needle . ', ' . $haystack . ' ' ); + + $fixer->endChangeset(); + } +} diff --git a/phpcs-sniffs/Formidable/ruleset.xml b/phpcs-sniffs/Formidable/ruleset.xml index 2c0f54c1a3..e07917bf8e 100644 --- a/phpcs-sniffs/Formidable/ruleset.xml +++ b/phpcs-sniffs/Formidable/ruleset.xml @@ -39,6 +39,7 @@ + diff --git a/tests/phpunit/database/test_FrmMigrate.php b/tests/phpunit/database/test_FrmMigrate.php index cd91bb7c6b..547bb8a71a 100644 --- a/tests/phpunit/database/test_FrmMigrate.php +++ b/tests/phpunit/database/test_FrmMigrate.php @@ -315,11 +315,11 @@ private function assert_collation() { $collation = $frmdb->collation(); if ( ! empty( $wpdb->charset ) ) { - $this->assertTrue( str_contains( $collation, 'DEFAULT CHARACTER SET' ) ); + $this->assertStringContainsString( 'DEFAULT CHARACTER SET', $collation ); } if ( ! empty( $wpdb->collate ) ) { - $this->assertTrue( str_contains( $collation, 'COLLATE' ) ); + $this->assertStringContainsString( 'COLLATE', $collation ); } } @@ -367,7 +367,7 @@ public function test_migrate_from_12_to_current() { $this->assertEquals( 1, count( $form_actions ), 'Old form settings are not converted to email action.' ); foreach ( $form_actions as $action ) { - $this->assertTrue( str_contains( $action->post_content['email_to'], 'emailto@test.com' ) ); + $this->assertStringContainsString( 'emailto@test.com', $action->post_content['email_to'] ); } } diff --git a/tests/phpunit/misc/test_FrmSimpleBlocksController.php b/tests/phpunit/misc/test_FrmSimpleBlocksController.php index fbddcf07a7..a9d3c7480f 100644 --- a/tests/phpunit/misc/test_FrmSimpleBlocksController.php +++ b/tests/phpunit/misc/test_FrmSimpleBlocksController.php @@ -16,7 +16,7 @@ public function test_maybe_remove_fade_on_load_for_block_preview() { } unset( $_SERVER['HTTP_ACCEPT'], $_SERVER['CONTENT_TYPE'] ); - $this->assertTrue( str_contains( $this->maybe_remove_fade_on_load_for_block_preview( $form ), 'frm_logic_form' ) ); + $this->assertStringContainsString( 'frm_logic_form', $this->maybe_remove_fade_on_load_for_block_preview( $form ) ); } private function maybe_remove_fade_on_load_for_block_preview( $form ) { diff --git a/tests/phpunit/styles/test_FrmStylesHelper.php b/tests/phpunit/styles/test_FrmStylesHelper.php index 6b1a32e68f..ea31fb3fb2 100644 --- a/tests/phpunit/styles/test_FrmStylesHelper.php +++ b/tests/phpunit/styles/test_FrmStylesHelper.php @@ -11,11 +11,11 @@ class test_FrmStylesHelper extends FrmUnitTest { public function test_get_upload_base() { $base = FrmStylesHelper::get_upload_base(); $this->assertTrue( isset( $base['baseurl'] ) ); - $this->assertTrue( str_contains( $base['baseurl'], 'http://' ) ); + $this->assertStringContainsString( 'http://', $base['baseurl'] ); $_SERVER['HTTPS'] = 'on'; $base = FrmStylesHelper::get_upload_base(); - $this->assertTrue( str_contains( $base['baseurl'], 'https://' ) ); + $this->assertStringContainsString( 'https://', $base['baseurl'] ); } /** From a4909875b7e4deff2ec39c4e3c4ebf348c927887 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Thu, 15 Jan 2026 16:20:14 -0400 Subject: [PATCH 2/3] Run new sniff through php cs fixer --- .../Sniffs/PHPUnit/PreferAssertStringContainsSniff.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php index b60a20c78d..3c252bd4de 100644 --- a/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php @@ -175,6 +175,7 @@ private function parseArguments( File $phpcsFile, $funcOpenParen, $funcClosePare // Add the last argument. $trimmed = trim( $currentArg ); + if ( '' !== $trimmed ) { $args[] = $trimmed; } From 55420c2fe192acb020e6b08e2125b5dd0f97136d Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Thu, 15 Jan 2026 16:23:19 -0400 Subject: [PATCH 3/3] Also catch strpos asserts --- .../PreferAssertStringContainsSniff.php | 27 +++++++++++++------ tests/phpunit/emails/test_FrmEmail.php | 4 +-- tests/phpunit/fields/test_FrmFieldName.php | 6 ++--- tests/phpunit/fields/test_FrmFieldType.php | 6 ++--- .../misc/test_FrmSimpleBlocksController.php | 2 +- .../styles/test_FrmStylesController.php | 2 +- 6 files changed, 29 insertions(+), 18 deletions(-) diff --git a/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php index 3c252bd4de..5bb62882e6 100644 --- a/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/PHPUnit/PreferAssertStringContainsSniff.php @@ -14,10 +14,11 @@ use PHP_CodeSniffer\Files\File; /** - * Converts assertTrue(str_contains()) to assertStringContainsString(). + * Converts assertTrue(str_contains()) and assertNotFalse(strpos()) to assertStringContainsString(). * * Bad: * $this->assertTrue( str_contains( $haystack, $needle ) ); + * $this->assertNotFalse( strpos( $haystack, $needle ) ); * * Good: * $this->assertStringContainsString( $needle, $haystack ); @@ -45,10 +46,12 @@ public function process( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); $token = $tokens[ $stackPtr ]; - // Check for assertTrue or assertFalse. + // Check for assertTrue, assertFalse, assertNotFalse. $methodName = strtolower( $token['content'] ); - if ( 'asserttrue' !== $methodName && 'assertfalse' !== $methodName ) { + $validMethods = array( 'asserttrue', 'assertfalse', 'assertnotfalse' ); + + if ( ! in_array( $methodName, $validMethods, true ) ) { return; } @@ -77,14 +80,14 @@ public function process( File $phpcsFile, $stackPtr ) { return; } - // Check if first argument is str_contains function call. + // Check if first argument is str_contains or strpos function call. if ( $tokens[ $firstArg ]['code'] !== T_STRING ) { return; } $functionName = strtolower( $tokens[ $firstArg ]['content'] ); - if ( 'str_contains' !== $functionName ) { + if ( 'str_contains' !== $functionName && 'strpos' !== $functionName ) { return; } @@ -123,13 +126,21 @@ public function process( File $phpcsFile, $stackPtr ) { } // Determine the new method name. - $newMethodName = 'asserttrue' === $methodName ? 'assertStringContainsString' : 'assertStringNotContainsString'; + // assertTrue(str_contains()) -> assertStringContainsString + // assertFalse(str_contains()) -> assertStringNotContainsString + // assertNotFalse(strpos()) -> assertStringContainsString + // assertFalse(strpos()) -> assertStringNotContainsString + if ( 'asserttrue' === $methodName || 'assertnotfalse' === $methodName ) { + $newMethodName = 'assertStringContainsString'; + } else { + $newMethodName = 'assertStringNotContainsString'; + } $fix = $phpcsFile->addFixableError( - 'Use %s() instead of %s(str_contains()).', + 'Use %s() instead of %s(%s()).', $stackPtr, 'Found', - array( $newMethodName, $token['content'] ) + array( $newMethodName, $token['content'], $functionName ) ); if ( true === $fix ) { diff --git a/tests/phpunit/emails/test_FrmEmail.php b/tests/phpunit/emails/test_FrmEmail.php index e006a0b078..988928ec9b 100644 --- a/tests/phpunit/emails/test_FrmEmail.php +++ b/tests/phpunit/emails/test_FrmEmail.php @@ -678,9 +678,9 @@ public function test_message_user_info() { $actual = $this->get_private_property( $email, 'message' ); if ( $setting['compare'] === 'Contains' ) { - $this->assertNotFalse( strpos( $actual, 'Referrer:' ) ); + $this->assertStringContainsString( 'Referrer:', $actual ); } else { - $this->assertFalse( strpos( $actual, 'Referrer:' ) ); + $this->assertStringNotContainsString( 'Referrer:', $actual ); } } } diff --git a/tests/phpunit/fields/test_FrmFieldName.php b/tests/phpunit/fields/test_FrmFieldName.php index 88e405cde3..d1cb6e6d5a 100644 --- a/tests/phpunit/fields/test_FrmFieldName.php +++ b/tests/phpunit/fields/test_FrmFieldName.php @@ -19,8 +19,8 @@ public function test_get_processed_sub_fields() { $processed_sub_fields = $this->run_private_method( array( $name_field, 'get_processed_sub_fields' ) ); $this->assertEquals( array( 'first', 'middle', 'last' ), array_keys( $processed_sub_fields ) ); - $this->assertNotFalse( strpos( $processed_sub_fields['first']['wrapper_classes'], 'frm4' ) ); - $this->assertNotFalse( strpos( $processed_sub_fields['middle']['wrapper_classes'], 'frm4' ) ); - $this->assertNotFalse( strpos( $processed_sub_fields['last']['wrapper_classes'], 'frm4' ) ); + $this->assertStringContainsString( 'frm4', $processed_sub_fields['first']['wrapper_classes'] ); + $this->assertStringContainsString( 'frm4', $processed_sub_fields['middle']['wrapper_classes'] ); + $this->assertStringContainsString( 'frm4', $processed_sub_fields['last']['wrapper_classes'] ); } } diff --git a/tests/phpunit/fields/test_FrmFieldType.php b/tests/phpunit/fields/test_FrmFieldType.php index 7839970349..90155bc9ce 100644 --- a/tests/phpunit/fields/test_FrmFieldType.php +++ b/tests/phpunit/fields/test_FrmFieldType.php @@ -28,9 +28,9 @@ public function test_html_min_number() { 'id' => $form_id, ) ); - $this->assertNotFalse( strpos( $form, ' min="10"' ) ); - $this->assertNotFalse( strpos( $form, ' max="999"' ) ); - $this->assertNotFalse( strpos( $form, ' step="any"' ) ); + $this->assertStringContainsString( ' min="10"', $form ); + $this->assertStringContainsString( ' max="999"', $form ); + $this->assertStringContainsString( ' step="any"', $form ); } /** diff --git a/tests/phpunit/misc/test_FrmSimpleBlocksController.php b/tests/phpunit/misc/test_FrmSimpleBlocksController.php index a9d3c7480f..3fe3a6e497 100644 --- a/tests/phpunit/misc/test_FrmSimpleBlocksController.php +++ b/tests/phpunit/misc/test_FrmSimpleBlocksController.php @@ -12,7 +12,7 @@ public function test_maybe_remove_fade_on_load_for_block_preview() { $_SERVER['CONTENT_TYPE'] = 'application/json'; if ( is_callable( 'wp_is_json_request' ) ) { - $this->assertFalse( strpos( $this->maybe_remove_fade_on_load_for_block_preview( $form ), 'frm_logic_form' ) ); + $this->assertStringNotContainsString( 'frm_logic_form', $this->maybe_remove_fade_on_load_for_block_preview( $form ) ); } unset( $_SERVER['HTTP_ACCEPT'], $_SERVER['CONTENT_TYPE'] ); diff --git a/tests/phpunit/styles/test_FrmStylesController.php b/tests/phpunit/styles/test_FrmStylesController.php index 8e605064b7..7aee7f8c5b 100644 --- a/tests/phpunit/styles/test_FrmStylesController.php +++ b/tests/phpunit/styles/test_FrmStylesController.php @@ -73,7 +73,7 @@ public function test_save() { FrmStylesController::save(); $returned = ob_get_clean(); - $this->assertNotFalse( strpos( $returned, 'Your styling settings have been saved.' ) ); + $this->assertStringContainsString( 'Your styling settings have been saved.', $returned ); $frm_style = new FrmStyle( $style->ID ); $updated_style = $frm_style->get_one(); $this->assertEquals( $style->post_title . ' Updated', $updated_style->post_title );