-
Notifications
You must be signed in to change notification settings - Fork 41
Update add missing param type from calls sniff to add missing doc blocks #2874
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,22 +68,22 @@ public function process( File $phpcsFile, $stackPtr ) { | |
|
|
||
| $functionName = $tokens[ $functionNamePtr ]['content']; | ||
|
|
||
| // Check if function has a docblock. | ||
| $docblock = $this->findDocblock( $phpcsFile, $stackPtr ); | ||
|
|
||
| if ( false === $docblock ) { | ||
| return; | ||
| } | ||
|
|
||
| // Get function parameters. | ||
| $params = $this->getParameters( $phpcsFile, $stackPtr ); | ||
|
|
||
| if ( empty( $params ) ) { | ||
| return; | ||
| } | ||
|
|
||
| // Get existing @param tags with their types. | ||
| $existingParams = $this->getExistingParamTags( $phpcsFile, $docblock ); | ||
| // Check if function has a docblock. | ||
| $docblock = $this->findDocblock( $phpcsFile, $stackPtr ); | ||
| $hasDocblock = ( false !== $docblock ); | ||
| $existingParams = array(); | ||
|
|
||
| if ( $hasDocblock ) { | ||
| // Get existing @param tags with their types. | ||
| $existingParams = $this->getExistingParamTags( $phpcsFile, $docblock ); | ||
| } | ||
|
|
||
| // Find parameters that are missing @param comments entirely. | ||
| $missingParamComments = array(); | ||
|
|
@@ -145,6 +145,21 @@ public function process( File $phpcsFile, $stackPtr ) { | |
| return; | ||
| } | ||
|
|
||
| // If no docblock exists, create one with the detected @param tags. | ||
| if ( ! $hasDocblock ) { | ||
| $fix = $phpcsFile->addFixableError( | ||
| 'Missing docblock with @param tags (detected types from call site literals)', | ||
| $stackPtr, | ||
| 'MissingDocblockFromCalls' | ||
| ); | ||
|
|
||
| if ( $fix ) { | ||
| $this->addDocblock( $phpcsFile, $stackPtr, $paramsToAdd ); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Report and fix each missing @param. | ||
| foreach ( $paramsToAdd as $paramName => $info ) { | ||
| $typeString = implode( '|', $info['types'] ); | ||
|
|
@@ -513,4 +528,73 @@ private function addParamTag( File $phpcsFile, $docblock, $type, $paramName ) { | |
| $phpcsFile->fixer->replaceToken( $lastParamEnd, $newContent ); | ||
| $phpcsFile->fixer->endChangeset(); | ||
| } | ||
|
|
||
| /** | ||
| * Add a new docblock with @param tags. | ||
| * | ||
| * @param File $phpcsFile The file being scanned. | ||
| * @param int $stackPtr The function token position. | ||
| * @param array $paramsToAdd Array of param name => array('types' => array, 'index' => int). | ||
| * | ||
| * @return void | ||
| */ | ||
| private function addDocblock( File $phpcsFile, $stackPtr, $paramsToAdd ) { | ||
| $tokens = $phpcsFile->getTokens(); | ||
|
|
||
| // Find the position to insert the docblock (before any modifiers or the function keyword). | ||
| $insertBefore = $stackPtr; | ||
|
|
||
| $prev = $phpcsFile->findPrevious( | ||
| array( T_WHITESPACE, T_STATIC, T_PUBLIC, T_PRIVATE, T_PROTECTED, T_ABSTRACT, T_FINAL ), | ||
| $stackPtr - 1, | ||
| null, | ||
| true | ||
| ); | ||
|
|
||
| if ( false !== $prev ) { | ||
| // Find the first modifier/keyword after $prev. | ||
| $next = $phpcsFile->findNext( T_WHITESPACE, $prev + 1, $stackPtr, true ); | ||
|
|
||
| if ( false !== $next ) { | ||
| $insertBefore = $next; | ||
| } | ||
| } | ||
|
|
||
| // Determine indentation from the function line. | ||
| $indent = ''; | ||
|
|
||
| for ( $i = $insertBefore - 1; $i >= 0; $i-- ) { | ||
| if ( $tokens[ $i ]['code'] === T_WHITESPACE ) { | ||
| if ( strpos( $tokens[ $i ]['content'], "\n" ) !== false || strpos( $tokens[ $i ]['content'], "\r" ) !== false ) { | ||
| // Get the part after the newline. | ||
| $indent = preg_replace( '/^.*[\r\n]/', '', $tokens[ $i ]['content'] ); | ||
| break; | ||
| } | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Build the docblock. | ||
| $docblock = "/**\n"; | ||
|
|
||
| // Sort params by index to maintain order. | ||
| uasort( | ||
| $paramsToAdd, | ||
| function ( $a, $b ) { | ||
| return $a['index'] - $b['index']; | ||
| } | ||
| ); | ||
|
|
||
| foreach ( $paramsToAdd as $paramName => $info ) { | ||
| $typeString = implode( '|', $info['types'] ); | ||
| $docblock .= $indent . "\t * @param " . $typeString . ' ' . $paramName . "\n"; | ||
| } | ||
|
|
||
| $docblock .= $indent . "\t */\n" . $indent . "\t"; | ||
|
|
||
|
Comment on lines
+578
to
+595
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix indentation in generated docblocks. The current builder adds an extra tab for each 🛠️ Adjust indentation to match the function line- foreach ( $paramsToAdd as $paramName => $info ) {
- $typeString = implode( '|', $info['types'] );
- $docblock .= $indent . "\t * `@param` " . $typeString . ' ' . $paramName . "\n";
- }
+ foreach ( $paramsToAdd as $paramName => $info ) {
+ $typeString = implode( '|', $info['types'] );
+ $docblock .= $indent . " * `@param` " . $typeString . ' ' . $paramName . "\n";
+ }
- $docblock .= $indent . "\t */\n" . $indent . "\t";
+ $docblock .= $indent . " */\n" . $indent;🤖 Prompt for AI Agents |
||
| $phpcsFile->fixer->beginChangeset(); | ||
| $phpcsFile->fixer->addContentBefore( $insertBefore, $docblock ); | ||
| $phpcsFile->fixer->endChangeset(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -428,6 +428,9 @@ public function test_trigger_email_six() { | |||||||||||||||||||||||
| $this->check_senders( $expected, $mock_email ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @param string $subject | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| protected function prepare_subject( $subject ) { | ||||||||||||||||||||||||
| return wp_specialchars_decode( strip_tags( stripslashes( $subject ) ), ENT_QUOTES ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
@@ -747,6 +750,10 @@ public function test_plain_text_message() { | |||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @param string $setting_name | ||||||||||||||||||||||||
| * @param string $property | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| private function check_private_properties( $settings, $setting_name, $property = '' ) { | ||||||||||||||||||||||||
|
Comment on lines
+753
to
757
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing The docblock is missing the first parameter Proposed fix /**
+ * `@param` array $settings
* `@param` string $setting_name
* `@param` string $property
*/
private function check_private_properties( $settings, $setting_name, $property = '' ) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| if ( ! $property ) { | ||||||||||||||||||||||||
| $property = $setting_name; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -169,12 +169,20 @@ public function test_sanitize_field_opt() { | |||||||||||||||
| $this->assert_sanitize_field_opt_calc( $original_value, $original_value ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @param string $expected | ||||||||||||||||
| * @param string $original_value | ||||||||||||||||
| * @param string $message | ||||||||||||||||
| */ | ||||||||||||||||
| private function assert_sanitize_field_opt_calc( $expected, $original_value, $message = '' ) { | ||||||||||||||||
| $value = $original_value; | ||||||||||||||||
| $this->sanitize_field_opt( 'calc', $value ); | ||||||||||||||||
| $this->assertEquals( $expected, $value, $message ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @param string $opt | ||||||||||||||||
| */ | ||||||||||||||||
|
Comment on lines
+183
to
+185
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add missing
💡 Suggested docblock update /**
* `@param` string $opt
+ * `@param` string &$value
*/
private function sanitize_field_opt( $opt, &$value ) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| private function sanitize_field_opt( $opt, &$value ) { | ||||||||||||||||
| return $this->run_private_method( | ||||||||||||||||
| array( 'FrmForm', 'sanitize_field_opt' ), | ||||||||||||||||
|
|
@@ -196,6 +204,9 @@ public function test_normalize_calc_spaces() { | |||||||||||||||
| $this->assertEquals( '1 <= 2 && 3 <= 4 && 5 <= 6', $this->normalize_calc_spaces( '1<=2 && 3<=4 && 5<=6' ) ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @param string $calc | ||||||||||||||||
| */ | ||||||||||||||||
| private function normalize_calc_spaces( $calc ) { | ||||||||||||||||
| return $this->run_private_method( array( 'FrmForm', 'normalize_calc_spaces' ), array( $calc ) ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,6 +152,9 @@ public function test_front_head() { | |
| $this->assertEquals( FrmAppHelper::plugin_url() . '/js/' . $file, $formidable_js->src, $file . ' was not loaded' ); | ||
| } | ||
|
|
||
| /** | ||
| * @param array $post_content | ||
| */ | ||
|
Comment on lines
+155
to
+157
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document The new docblock only documents 💡 Suggested docblock update /**
+ * `@param` int $form_id
* `@param` array $post_content
*/
private function create_on_submit_action( $form_id, $post_content ) {🤖 Prompt for AI Agents |
||
| private function create_on_submit_action( $form_id, $post_content ) { | ||
| $post_data = array( | ||
| 'post_type' => FrmFormActionsController::$action_post_type, | ||
|
|
@@ -306,6 +309,9 @@ public function test_message_with_form_after_create() { | |
| $this->run_message_after_create( 1 ); | ||
| } | ||
|
|
||
| /** | ||
| * @param int $show_form | ||
| */ | ||
| public function run_message_after_create( $show_form = 0 ) { | ||
| $form = $this->factory->form->create_and_get( | ||
| array( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 150
🏁 Script executed:
# Check if the file exists and get its line count wc -l phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.phpRepository: Strategy11/formidable-forms
Length of output: 154
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 19995
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 2255
🌐 Web query:
PHP CodeSniffer T_ATTRIBUTE token PHP 8 attributes💡 Result:
In PHP_CodeSniffer (PHPCS), PHP 8 attributes are represented using the tokenizer’s
T_ATTRIBUTEtoken (marks the start of an attribute). PHPCS also defines an additional tokenT_ATTRIBUTE_END(marks the end of an attribute), and addsattribute_owner/attribute_closerindexes to tokens inside the attribute to help sniffs relate the attribute to the thing it decorates. This support was added in PHPCS 3.6.0 (released 2021-04-08), and PHPCS also “backfills” attribute tokenization when running on older PHP versions. [1] [2] [3]Practical sniff notes:
T_ATTRIBUTE(and often also useT_ATTRIBUTE_ENDto skip/scan cleanly). [1] [2]\PHP_CodeSniffer\Util\Tokens::T_ATTRIBUTE,::T_ATTRIBUTE_END). [2]Sources: [1] [2] [3]
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 143
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 144
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 2148
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 1013
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 949
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
Repository: Strategy11/formidable-forms
Length of output: 671
Add PHP 8 attribute token handling in both
findDocblock()andaddDocblock()methods.When a docblock sits above PHP 8 attributes (e.g.,
#[SomeAttribute]),findDocblock()will currently miss it because the backward scan stops at the attribute token, causing a duplicate docblock to be inserted below the attributes. The same issue exists inaddDocblock()which determines insertion position using an identical pattern. AddT_ATTRIBUTEandT_ATTRIBUTE_ENDto the ignore lists in both methods:🔧 Required changes in two locations
Location 1:
findDocblock()method (line 191-199)Location 2:
addDocblock()method (line 548)🤖 Prompt for AI Agents