Skip to content

Commit

Permalink
bug PHP-CS-Fixer#5006 PhpdocToParamTypeFixer - fix for breaking PHP s…
Browse files Browse the repository at this point in the history
…yntax for type having reserved name (kubawerlos)

This PR was squashed before being merged into the 2.16 branch (closes PHP-CS-Fixer#5006).

Discussion
----------

PhpdocToParamTypeFixer - fix for breaking PHP syntax for type having reserved name

Twin PR to PHP-CS-Fixer#4963

I wonder if it would be useful to have some class e.g. `SyntaxChecker` with (static?) method `isValid`, which will handle cache internally and all needed will be to call it like this:
```php
if (!SyntaxChecker::isValid(sprintf('<?php function f(%s $x) {}', $returnType))) {
    continue;
}
```

Currently, this will have 2 uses (here and `PhpdocToReturnTypeFixer`, maybe also `AbstractPsrAutoloadingFixer`, not mandatory), but I imagine quite soon we will have `PhpdocToPropertyTypeFixer`.

Commits
-------

76dde6f PhpdocToParamTypeFixer - fix for breaking PHP syntax for type having reserved name
  • Loading branch information
SpacePossum committed Jul 10, 2020
2 parents 9380e8c + 76dde6f commit 7405b4d
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 29 deletions.
40 changes: 40 additions & 0 deletions src/AbstractPhpdocToTypeDeclarationFixer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <[email protected]>
* Dariusz Rumiński <[email protected]>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer;

use PhpCsFixer\Tokenizer\Tokens;

/**
* @internal
*/
abstract class AbstractPhpdocToTypeDeclarationFixer extends AbstractFixer
{
/**
* @var array<string, bool>
*/
private static $syntaxValidationCache = [];

final protected function isValidSyntax($code)
{
if (!isset(self::$syntaxValidationCache[$code])) {
try {
Tokens::fromCode($code);
self::$syntaxValidationCache[$code] = true;
} catch (\ParseError $e) {
self::$syntaxValidationCache[$code] = false;
}
}

return self::$syntaxValidationCache[$code];
}
}
8 changes: 6 additions & 2 deletions src/Fixer/FunctionNotation/PhpdocToParamTypeFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace PhpCsFixer\Fixer\FunctionNotation;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\AbstractPhpdocToTypeDeclarationFixer;
use PhpCsFixer\DocBlock\Annotation;
use PhpCsFixer\DocBlock\DocBlock;
use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
Expand All @@ -29,7 +29,7 @@
/**
* @author Jan Gantzert <[email protected]>
*/
final class PhpdocToParamTypeFixer extends AbstractFixer implements ConfigurationDefinitionFixerInterface
final class PhpdocToParamTypeFixer extends AbstractPhpdocToTypeDeclarationFixer implements ConfigurationDefinitionFixerInterface
{
/** @internal */
const CLASS_REGEX = '/^\\\\?[a-zA-Z_\\x7f-\\xff](?:\\\\?[a-zA-Z0-9_\\x7f-\\xff]+)*(?<array>\[\])*$/';
Expand Down Expand Up @@ -258,6 +258,10 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
continue;
}

if (!$this->isValidSyntax(sprintf('<?php function f(%s $x) {}', $paramType))) {
continue;
}

$this->fixFunctionDefinition(
$paramType,
$tokens,
Expand Down
30 changes: 3 additions & 27 deletions src/Fixer/FunctionNotation/PhpdocToReturnTypeFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace PhpCsFixer\Fixer\FunctionNotation;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\AbstractPhpdocToTypeDeclarationFixer;
use PhpCsFixer\DocBlock\Annotation;
use PhpCsFixer\DocBlock\DocBlock;
use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
Expand All @@ -29,7 +29,7 @@
/**
* @author Filippo Tessarotto <[email protected]>
*/
final class PhpdocToReturnTypeFixer extends AbstractFixer implements ConfigurationDefinitionFixerInterface
final class PhpdocToReturnTypeFixer extends AbstractPhpdocToTypeDeclarationFixer implements ConfigurationDefinitionFixerInterface
{
/**
* @var array<int, array<int, int|string>>
Expand Down Expand Up @@ -75,11 +75,6 @@ final class PhpdocToReturnTypeFixer extends AbstractFixer implements Configurati
*/
private $classRegex = '/^\\\\?[a-zA-Z_\\x7f-\\xff](?:\\\\?[a-zA-Z0-9_\\x7f-\\xff]+)*(?<array>\[\])*$/';

/**
* @var array<string, bool>
*/
private $returnTypeCache = [];

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -266,7 +261,7 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
continue;
}

if (!$this->isValidType($returnType)) {
if (!$this->isValidSyntax(sprintf('<?php function f():%s {}', $returnType))) {
continue;
}

Expand Down Expand Up @@ -356,23 +351,4 @@ private function findReturnAnnotations(Tokens $tokens, $index)

return $doc->getAnnotationsOfType('return');
}

/**
* @param string $returnType
*
* @return bool
*/
private function isValidType($returnType)
{
if (!\array_key_exists($returnType, $this->returnTypeCache)) {
try {
Tokens::fromCode(sprintf('<?php function f():%s {}', $returnType));
$this->returnTypeCache[$returnType] = true;
} catch (\ParseError $e) {
$this->returnTypeCache[$returnType] = false;
}
}

return $this->returnTypeCache[$returnType];
}
}
7 changes: 7 additions & 0 deletions tests/Fixer/FunctionNotation/PhpdocToParamTypeFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ function my_foo() {}
function my_foo2($bar) {}
',
],
'invalid - phpdoc param with keyword' => [
'<?php
/** @param Break $foo */ function foo_break($foo) {}
/** @param __CLASS__ $foo */ function foo_class($foo) {}
/** @param I\Want\To\Break\Free $foo */ function foo_queen($foo) {}
',
],
'non-root class with single int param' => [
'<?php /** @param int $bar */ function my_foo(int $bar) {}',
'<?php /** @param int $bar */ function my_foo($bar) {}',
Expand Down

0 comments on commit 7405b4d

Please sign in to comment.