Skip to content

Commit dfc0ea7

Browse files
committed
Fix up param handling.
1 parent 6f66992 commit dfc0ea7

File tree

4 files changed

+521
-1
lines changed

4 files changed

+521
-1
lines changed

PhpCollective/Sniffs/Commenting/DocBlockParamSniff.php

Lines changed: 251 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function process(File $phpCsFile, $stackPointer): void
6060
}
6161

6262
$docBlockParams = [];
63+
$hasMissingTypes = false;
6364
for ($i = $docBlockStartIndex + 1; $i < $docBlockEndIndex; $i++) {
6465
if ($tokens[$i]['type'] !== 'T_DOC_COMMENT_TAG') {
6566
continue;
@@ -72,12 +73,21 @@ public function process(File $phpCsFile, $stackPointer): void
7273

7374
if ($tokens[$classNameIndex]['type'] !== 'T_DOC_COMMENT_STRING') {
7475
$phpCsFile->addError('Missing type in param doc block', $i, 'MissingType');
76+
$hasMissingTypes = true;
7577

7678
continue;
7779
}
7880

7981
$content = $tokens[$classNameIndex]['content'];
8082

83+
// Check if the content starts with $ (missing type)
84+
if (str_starts_with($content, '$')) {
85+
$phpCsFile->addError('Missing type in param doc block', $i, 'MissingType');
86+
$hasMissingTypes = true;
87+
88+
continue;
89+
}
90+
8191
$appendix = '';
8292
$spacePos = strpos($content, ' ');
8393
if ($spacePos) {
@@ -96,7 +106,27 @@ public function process(File $phpCsFile, $stackPointer): void
96106
];
97107
}
98108

109+
// If no @param annotations found, check if all parameters are fully typed
110+
// Only skip validation if all parameters have type declarations
111+
if (count($docBlockParams) === 0) {
112+
if ($this->areAllParametersFullyTyped($methodSignature)) {
113+
return;
114+
}
115+
}
116+
99117
if (count($docBlockParams) !== count($methodSignature)) {
118+
// Check if we can fix by adding missing params (when all method params are typed and no missing types in existing params)
119+
if (!$hasMissingTypes && count($docBlockParams) < count($methodSignature) && $this->canAddMissingParams($phpCsFile, $docBlockStartIndex, $docBlockEndIndex, $docBlockParams, $methodSignature)) {
120+
return;
121+
}
122+
123+
// Check if we have extra params that can be removed
124+
if (count($docBlockParams) > count($methodSignature)) {
125+
$this->handleExtraParams($phpCsFile, $docBlockStartIndex, $docBlockEndIndex, $docBlockParams, $methodSignature);
126+
127+
return;
128+
}
129+
100130
$phpCsFile->addError('Doc Block params do not match method signature', $stackPointer, 'SignatureMismatch');
101131

102132
return;
@@ -140,7 +170,227 @@ protected function assertNoParams(File $phpCsFile, int $docBlockStartIndex, int
140170
continue;
141171
}
142172

143-
$phpCsFile->addError('Doc Block param does not match method signature and should be removed', $i, 'ExtraParam');
173+
$fix = $phpCsFile->addFixableError('Doc Block param does not match method signature and should be removed', $i, 'ExtraParam');
174+
175+
if ($fix === true) {
176+
$this->removeParamLine($phpCsFile, $i);
177+
}
178+
}
179+
}
180+
181+
/**
182+
* Check if all method parameters are fully typed.
183+
*
184+
* @param array<int, array<string, mixed>> $methodSignature
185+
*
186+
* @return bool
187+
*/
188+
protected function areAllParametersFullyTyped(array $methodSignature): bool
189+
{
190+
foreach ($methodSignature as $param) {
191+
// Parameter must have a type hint
192+
if (empty($param['typehint'])) {
193+
return false;
194+
}
195+
}
196+
197+
return true;
198+
}
199+
200+
/**
201+
* @param \PHP_CodeSniffer\Files\File $phpCsFile
202+
* @param int $docBlockStartIndex
203+
* @param int $docBlockEndIndex
204+
* @param array<array<string, mixed>> $docBlockParams
205+
* @param array<int, array<string, mixed>> $methodSignature
206+
*
207+
* @return bool
208+
*/
209+
protected function canAddMissingParams(File $phpCsFile, int $docBlockStartIndex, int $docBlockEndIndex, array $docBlockParams, array $methodSignature): bool
210+
{
211+
$tokens = $phpCsFile->getTokens();
212+
213+
// Check if all params have types so we can add them
214+
foreach ($methodSignature as $param) {
215+
if (empty($param['typehintFull'])) {
216+
return false;
217+
}
218+
}
219+
220+
// Find the position to insert new params (after last @param or before close comment)
221+
$insertPosition = $docBlockEndIndex - 1;
222+
$lastParamIndex = null;
223+
224+
for ($i = $docBlockStartIndex + 1; $i < $docBlockEndIndex; $i++) {
225+
if ($tokens[$i]['type'] === 'T_DOC_COMMENT_TAG' && $tokens[$i]['content'] === '@param') {
226+
$lastParamIndex = $i;
227+
// Find the end of this param line
228+
for ($j = $i + 1; $j < $docBlockEndIndex; $j++) {
229+
if ($tokens[$j]['content'] === "\n" || $tokens[$j]['type'] === 'T_DOC_COMMENT_CLOSE_TAG') {
230+
$insertPosition = $j;
231+
232+
break;
233+
}
234+
}
235+
}
236+
}
237+
238+
$fix = $phpCsFile->addFixableError('Doc Block params do not match method signature', $docBlockStartIndex + 1, 'SignatureMismatch');
239+
240+
if ($fix === true) {
241+
$phpCsFile->fixer->beginChangeset();
242+
243+
// Build list of existing param variables
244+
$existingVars = [];
245+
foreach ($docBlockParams as $param) {
246+
$existingVars[] = $param['variable'];
247+
}
248+
249+
// Add missing params
250+
foreach ($methodSignature as $methodParam) {
251+
$variable = $tokens[$methodParam['variableIndex']]['content'];
252+
if (!in_array($variable, $existingVars, true)) {
253+
$indent = $this->getIndentForParam($phpCsFile, $docBlockStartIndex, $docBlockEndIndex);
254+
$paramLine = "\n" . $indent . '* @param ' . $methodParam['typehintFull'] . ' ' . $variable;
255+
256+
$phpCsFile->fixer->addContentBefore($insertPosition, $paramLine);
257+
}
258+
}
259+
260+
$phpCsFile->fixer->endChangeset();
261+
}
262+
263+
return true;
264+
}
265+
266+
/**
267+
* @param \PHP_CodeSniffer\Files\File $phpCsFile
268+
* @param int $paramTagIndex
269+
*
270+
* @return void
271+
*/
272+
protected function removeParamLine(File $phpCsFile, int $paramTagIndex): void
273+
{
274+
$tokens = $phpCsFile->getTokens();
275+
276+
$phpCsFile->fixer->beginChangeset();
277+
278+
// Find the start of the line
279+
$lineStart = $paramTagIndex;
280+
for ($i = $paramTagIndex - 1; $i >= 0; $i--) {
281+
if ($tokens[$i]['content'] === "\n") {
282+
break;
283+
}
284+
$lineStart = $i;
285+
}
286+
287+
// Find the end of the line
288+
$lineEnd = $paramTagIndex;
289+
$count = count($tokens);
290+
for ($i = $paramTagIndex + 1; $i < $count; $i++) {
291+
$lineEnd = $i;
292+
if ($tokens[$i]['content'] === "\n") {
293+
break;
294+
}
295+
}
296+
297+
// Remove the entire line
298+
for ($i = $lineStart; $i <= $lineEnd; $i++) {
299+
$phpCsFile->fixer->replaceToken($i, '');
300+
}
301+
302+
$phpCsFile->fixer->endChangeset();
303+
}
304+
305+
/**
306+
* @param \PHP_CodeSniffer\Files\File $phpCsFile
307+
* @param int $docBlockStartIndex
308+
* @param int $docBlockEndIndex
309+
*
310+
* @return string
311+
*/
312+
protected function getIndentForParam(File $phpCsFile, int $docBlockStartIndex, int $docBlockEndIndex): string
313+
{
314+
$tokens = $phpCsFile->getTokens();
315+
316+
// Find an existing @param or use the doc block start
317+
for ($i = $docBlockStartIndex + 1; $i < $docBlockEndIndex; $i++) {
318+
if ($tokens[$i]['type'] === 'T_DOC_COMMENT_TAG') {
319+
// Get the indent from this line
320+
for ($j = $i - 1; $j >= 0; $j--) {
321+
if ($tokens[$j]['content'] === "\n") {
322+
if (isset($tokens[$j + 1]) && $tokens[$j + 1]['type'] === 'T_DOC_COMMENT_WHITESPACE') {
323+
return $tokens[$j + 1]['content'];
324+
}
325+
326+
break;
327+
}
328+
}
329+
}
330+
}
331+
332+
return ' ';
333+
}
334+
335+
/**
336+
* @param \PHP_CodeSniffer\Files\File $phpCsFile
337+
* @param int $docBlockStartIndex
338+
* @param int $docBlockEndIndex
339+
* @param array<array<string, mixed>> $docBlockParams
340+
* @param array<int, array<string, mixed>> $methodSignature
341+
*
342+
* @return void
343+
*/
344+
protected function handleExtraParams(File $phpCsFile, int $docBlockStartIndex, int $docBlockEndIndex, array $docBlockParams, array $methodSignature): void
345+
{
346+
$tokens = $phpCsFile->getTokens();
347+
348+
// Build list of expected param variables
349+
$expectedVars = [];
350+
foreach ($methodSignature as $param) {
351+
$expectedVars[] = $tokens[$param['variableIndex']]['content'];
352+
}
353+
354+
// Find and mark extra params for removal
355+
$hasFixableError = false;
356+
for ($i = $docBlockStartIndex + 1; $i < $docBlockEndIndex; $i++) {
357+
if ($tokens[$i]['type'] !== 'T_DOC_COMMENT_TAG' || $tokens[$i]['content'] !== '@param') {
358+
continue;
359+
}
360+
361+
// Find the variable name for this @param
362+
$variable = null;
363+
$classNameIndex = $i + 2;
364+
if (isset($tokens[$classNameIndex]) && $tokens[$classNameIndex]['type'] === 'T_DOC_COMMENT_STRING') {
365+
$content = $tokens[$classNameIndex]['content'];
366+
367+
// Check if content starts with $ (missing type)
368+
if (str_starts_with($content, '$')) {
369+
$variable = explode(' ', $content)[0];
370+
} else {
371+
// Extract variable from content
372+
$spacePos = strpos($content, ' ');
373+
if ($spacePos) {
374+
$appendix = substr($content, $spacePos);
375+
preg_match('/\$[^\s]+/', $appendix, $matches);
376+
$variable = $matches ? $matches[0] : null;
377+
}
378+
}
379+
}
380+
381+
// If this param is not in the expected list, mark for removal
382+
if ($variable && !in_array($variable, $expectedVars, true)) {
383+
$fix = $phpCsFile->addFixableError('Doc Block param does not match method signature and should be removed', $i, 'ExtraParam');
384+
385+
if ($fix === true) {
386+
$hasFixableError = true;
387+
$this->removeParamLine($phpCsFile, $i);
388+
}
389+
}
390+
}
391+
392+
if (!$hasFixableError) {
393+
$phpCsFile->addError('Doc Block params do not match method signature', $docBlockStartIndex + 1, 'SignatureMismatch');
144394
}
145395
}
146396
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/**
4+
* MIT License
5+
* For full license information, please view the LICENSE file that was distributed with this source code.
6+
*/
7+
8+
namespace PhpCollective\Test\PhpCollective\Sniffs\Commenting;
9+
10+
use PhpCollective\Sniffs\Commenting\DocBlockParamSniff;
11+
use PhpCollective\Test\TestCase;
12+
13+
class DocBlockParamSniffTest extends TestCase
14+
{
15+
/**
16+
* @return void
17+
*/
18+
public function testDocBlockParamSniffer(): void
19+
{
20+
// Expected errors:
21+
// Line 32: SignatureMismatch - not fully typed with @dataProvider (not fixable - missing type)
22+
// Line 37: SignatureMismatch - partially documented (fixable - can add missing param)
23+
// Line 51: VariableWrong - wrong variable name (not fixable - needs manual correction)
24+
// Line 59: SignatureMismatch - extra @param count mismatch (not fixable - complex case)
25+
// Line 64: ExtraParam - extra @param $extra (fixable - can remove extra param)
26+
// Line 74: ExtraParam - no params but has @param (fixable - can remove param)
27+
// Line 84: MissingType - missing type in @param (not fixable - needs manual type)
28+
// Line 87: SignatureMismatch - params don't match after missing type (not fixable)
29+
$this->assertSnifferFindsErrors(new DocBlockParamSniff(), 8);
30+
}
31+
32+
/**
33+
* @return void
34+
*/
35+
public function testDocBlockParamFixer(): void
36+
{
37+
// 3 fixable errors:
38+
// Line 37: Can add missing @param
39+
// Line 64: Can remove extra @param
40+
// Line 74: Can remove @param when no params
41+
$this->assertSnifferCanFixErrors(new DocBlockParamSniff(), 3);
42+
}
43+
}

0 commit comments

Comments
 (0)