Skip to content

Commit 0b84fec

Browse files
dereuromarkdereuromark
authored
Fix three autofixes that produced broken or wrong output (#84)
PreferCastOverFunction rewrote the first-class callable `strval(...)` to `(string)(...)`, which is a parse error. The callable form is now skipped. DocBlockParamAllowDefaultValue did not treat `non-empty-array`, `non-empty-list` and a bare `list` as covering an `array` typehint and appended `|array` to the param type. DocBlockThrows wrote a class from the current namespace as if it were global, so `throw new LocalException()` in namespace `App` produced `\LocalException`. Unimported names are now resolved against the namespace the method sits in, and a qualified relative name such as `Sub\NestedException`, which was skipped before, is resolved as well.
1 parent 709dc26 commit 0b84fec

10 files changed

Lines changed: 177 additions & 9 deletions

File tree

‎PhpCollective/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,12 @@ protected function isPrimitiveGenerics(string $type, array $parts): bool
237237
return false;
238238
}
239239

240-
foreach ($iterableTypes as $iterableType) {
240+
$docBlockIterableTypes = [...$iterableTypes, 'non-empty-array', 'non-empty-list'];
241+
if (array_intersect($docBlockIterableTypes, $parts)) {
242+
return true;
243+
}
244+
245+
foreach ($docBlockIterableTypes as $iterableType) {
241246
if ($this->containsTypeArray($parts, $iterableType)) {
242247
return true;
243248
}

‎PhpCollective/Sniffs/Commenting/DocBlockThrowsSniff.php‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ protected function extractExceptions(File $phpcsFile, int $stackPointer): array
153153

154154
// Handle the case where there's no namespace separator, string token, or fully qualified name token
155155
// (e.g., for parenthesis after 'new')
156-
if (!$this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_FULLY_QUALIFIED], $tokens[$contentIndex])) {
156+
if (!$this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED], $tokens[$contentIndex])) {
157157
continue;
158158
}
159159

@@ -248,12 +248,13 @@ protected function extractException(File $phpcsFile, int $contentIndex): array
248248
$fullClass = $tokens[$position]['content'];
249249
++$position;
250250
} else {
251-
while ($this->isGivenKind([T_NS_SEPARATOR, T_STRING], $tokens[$position])) {
251+
while ($this->isGivenKind([T_NS_SEPARATOR, T_STRING, T_NAME_QUALIFIED], $tokens[$position])) {
252252
$fullClass .= $tokens[$position]['content'];
253253
++$position;
254254
}
255255
}
256256

257+
$fullyQualified = str_starts_with($fullClass, '\\');
257258
$class = $fullClass = ltrim($fullClass, '\\');
258259
$lastSeparator = strrpos($class, '\\');
259260
if ($lastSeparator !== false) {
@@ -265,6 +266,7 @@ protected function extractException(File $phpcsFile, int $contentIndex): array
265266
'end' => $position - 1,
266267
'fullClass' => $fullClass,
267268
'class' => $class,
269+
'fullyQualified' => $fullyQualified,
268270
];
269271
}
270272

@@ -283,6 +285,7 @@ protected function compareExceptionsAndAnnotations(
283285
int $docBlockEndIndex,
284286
): void {
285287
$useStatements = $this->getUseStatements($phpcsFile);
288+
$namespace = $this->getNamespaceAt($phpcsFile, $docBlockEndIndex);
286289

287290
foreach ($annotations as $annotation) {
288291
if ($this->isInCode($annotation, $exceptions, $useStatements)) {
@@ -307,7 +310,7 @@ protected function compareExceptionsAndAnnotations(
307310

308311
$exceptionsToAdd = [];
309312
foreach ($exceptions as $exception) {
310-
$exception = $this->normalizeClassName($exception, $useStatements);
313+
$exception = $this->normalizeClassName($exception, $useStatements, $namespace);
311314
if (empty($exception['fullClass'])) {
312315
// We skip for complex scenarios
313316
$phpcsFile->addError('Doc Block @throw annotation missing', $docBlockEndIndex, 'ThrowMissingManual');
@@ -333,19 +336,79 @@ protected function compareExceptionsAndAnnotations(
333336
$this->addAnnotationLines($phpcsFile, $exceptionsToAdd, $docBlockEndIndex);
334337
}
335338

339+
/**
340+
* Namespace the given position is declared in; a file can hold several namespace blocks.
341+
*
342+
* @param \PHP_CodeSniffer\Files\File $phpcsFile
343+
* @param int $index
344+
*
345+
* @return string
346+
*/
347+
protected function getNamespaceAt(File $phpcsFile, int $index): string
348+
{
349+
$tokens = $phpcsFile->getTokens();
350+
351+
$namespaceIndex = $index;
352+
do {
353+
$namespaceIndex = $phpcsFile->findPrevious(T_NAMESPACE, $namespaceIndex - 1);
354+
if ($namespaceIndex === false) {
355+
return '';
356+
}
357+
$nextIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $namespaceIndex + 1, null, true);
358+
// `namespace\Foo` is the relative-name operator, not a declaration
359+
} while ($nextIndex !== false && $tokens[$nextIndex]['code'] === T_NS_SEPARATOR);
360+
361+
if (isset($tokens[$namespaceIndex]['scope_closer']) && $tokens[$namespaceIndex]['scope_closer'] < $index) {
362+
return '';
363+
}
364+
365+
$endIndex = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], $namespaceIndex + 1);
366+
if ($endIndex === false) {
367+
return '';
368+
}
369+
370+
$namespace = '';
371+
for ($i = $namespaceIndex + 1; $i < $endIndex; $i++) {
372+
if (!in_array($tokens[$i]['code'], Tokens::$emptyTokens, true)) {
373+
$namespace .= $tokens[$i]['content'];
374+
}
375+
}
376+
377+
return $namespace;
378+
}
379+
336380
/**
337381
* @param array<string, mixed> $exception
338382
* @param array<string, array<string, mixed>> $useStatements
383+
* @param string $namespace Namespace of the file, to resolve names that are not imported.
339384
*
340385
* @return array<string, mixed> Exception
341386
*/
342-
protected function normalizeClassName(array $exception, array $useStatements): array
387+
protected function normalizeClassName(array $exception, array $useStatements, string $namespace = ''): array
343388
{
389+
if (!empty($exception['fullyQualified']) || empty($exception['fullClass'])) {
390+
return $exception;
391+
}
392+
393+
/** @var string $fullClass */
394+
$fullClass = $exception['fullClass'];
395+
$segments = explode('\\', $fullClass);
396+
$first = array_shift($segments);
397+
344398
foreach ($useStatements as $useStatement) {
345-
if ($useStatement['alias'] === $exception['class'] || $useStatement['shortName'] === $exception['class']) {
399+
if ($useStatement['alias'] !== $first && $useStatement['shortName'] !== $first) {
400+
continue;
401+
}
402+
if (!$segments) {
346403
$exception['class'] = $useStatement['shortName'];
347-
$exception['fullClass'] = $useStatement['fullName'];
348404
}
405+
$exception['fullClass'] = implode('\\', [$useStatement['fullName'], ...$segments]);
406+
407+
return $exception;
408+
}
409+
410+
if ($namespace !== '') {
411+
$exception['fullClass'] = $namespace . '\\' . $fullClass;
349412
}
350413

351414
return $exception;

‎PhpCollective/Sniffs/PHP/PreferCastOverFunctionSniff.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,15 @@ public function process(File $phpcsFile, $stackPtr): void
6060

6161
$closingBraceIndex = $tokens[$openingBraceIndex]['parenthesis_closer'];
6262

63+
// First-class callable syntax `strval(...)` is not a call and has no cast equivalent
64+
$firstArgumentIndex = $phpcsFile->findNext(T_WHITESPACE, $openingBraceIndex + 1, $closingBraceIndex, true);
65+
if ($firstArgumentIndex !== false && $tokens[$firstArgumentIndex]['code'] === T_ELLIPSIS) {
66+
$afterEllipsisIndex = $phpcsFile->findNext(T_WHITESPACE, $firstArgumentIndex + 1, $closingBraceIndex, true);
67+
if ($afterEllipsisIndex === false) {
68+
return;
69+
}
70+
}
71+
6372
// We must ignore when commas are encountered
6473
if ($this->contains($phpcsFile, 'T_COMMA', $openingBraceIndex + 1, $closingBraceIndex - 1)) {
6574
return;

‎tests/PhpCollective/Sniffs/Commenting/DocBlockThrowsSniffTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ class DocBlockThrowsSniffTest extends TestCase
1717
*/
1818
public function testDocBlockThrowsSniffer(): void
1919
{
20-
$this->assertSnifferFindsErrors(new DocBlockThrowsSniff(), 6);
20+
$this->assertSnifferFindsErrors(new DocBlockThrowsSniff(), 9);
2121
}
2222

2323
/**
2424
* @return void
2525
*/
2626
public function testDocBlockThrowsFixer(): void
2727
{
28-
$this->assertSnifferCanFixErrors(new DocBlockThrowsSniff(), 6);
28+
$this->assertSnifferCanFixErrors(new DocBlockThrowsSniff(), 9);
2929
}
3030
}

‎tests/_data/DocBlockParamAllowDefaultValue/after.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,17 @@ public function arrayShape(array $array = null): void
9999
public function partialDocBlock(array $row, int $accountId, array &$errors): void
100100
{
101101
}
102+
103+
/**
104+
* Narrowed array types already cover the `array` typehint.
105+
*
106+
* @param non-empty-array<array{int, string}> $block
107+
* @param non-empty-list<string> $names
108+
* @param non-empty-array $rows
109+
* @param list $items
110+
* @return void
111+
*/
112+
public function narrowedArrays(array $block, array $names, array $rows, array $items): void
113+
{
114+
}
102115
}

‎tests/_data/DocBlockParamAllowDefaultValue/before.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,17 @@ public function arrayShape(array $array = null): void
9999
public function partialDocBlock(array $row, int $accountId, array &$errors): void
100100
{
101101
}
102+
103+
/**
104+
* Narrowed array types already cover the `array` typehint.
105+
*
106+
* @param non-empty-array<array{int, string}> $block
107+
* @param non-empty-list<string> $names
108+
* @param non-empty-array $rows
109+
* @param list $items
110+
* @return void
111+
*/
112+
public function narrowedArrays(array $block, array $names, array $rows, array $items): void
113+
{
114+
}
102115
}

‎tests/_data/DocBlockThrows/after.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,27 @@ public function staticCall(): void
7171

7272
new Parser();
7373
}
74+
75+
/**
76+
* @throws \PhpCollective\LocalException
77+
* @return void
78+
*/
79+
public function sameNamespace(): void
80+
{
81+
throw new LocalException();
82+
}
83+
84+
/**
85+
* @throws \Foo\Baz\Exception\DeeperException
86+
* @throws \PhpCollective\Sub\NestedException
87+
* @return void
88+
*/
89+
public function relativeNames(): void
90+
{
91+
if ($this->something()) {
92+
throw new Sub\NestedException();
93+
}
94+
95+
throw new SomeAliasedException\DeeperException();
96+
}
7497
}

‎tests/_data/DocBlockThrows/before.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,24 @@ public function staticCall(): void
6565

6666
new Parser();
6767
}
68+
69+
/**
70+
* @return void
71+
*/
72+
public function sameNamespace(): void
73+
{
74+
throw new LocalException();
75+
}
76+
77+
/**
78+
* @return void
79+
*/
80+
public function relativeNames(): void
81+
{
82+
if ($this->something()) {
83+
throw new Sub\NestedException();
84+
}
85+
86+
throw new SomeAliasedException\DeeperException();
87+
}
6888
}

‎tests/_data/PreferCastOverFunction/after.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,15 @@ public function namespacedFunctionForms($value): array
1616

1717
return [$plain, $fullyQualified, $qualified, $relative];
1818
}
19+
20+
/**
21+
* First-class callable syntax must stay as is.
22+
*/
23+
public function firstClassCallable(array $values): array
24+
{
25+
$strings = array_map(strval(...), $values);
26+
$ints = array_map(\intval( ... ), $values);
27+
28+
return [$strings, $ints];
29+
}
1930
}

‎tests/_data/PreferCastOverFunction/before.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,15 @@ public function namespacedFunctionForms($value): array
1616

1717
return [$plain, $fullyQualified, $qualified, $relative];
1818
}
19+
20+
/**
21+
* First-class callable syntax must stay as is.
22+
*/
23+
public function firstClassCallable(array $values): array
24+
{
25+
$strings = array_map(strval(...), $values);
26+
$ints = array_map(\intval( ... ), $values);
27+
28+
return [$strings, $ints];
29+
}
1930
}

0 commit comments

Comments
 (0)