From 57457750d3f5c20834c1695e776bc21c909af10f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 1 Aug 2023 21:03:54 +0200 Subject: [PATCH] Fix parsing Doctrine strings --- phpstan.neon | 1 + .../ConstExpr/DoctrineConstExprStringNode.php | 42 +++++ src/Lexer/Lexer.php | 29 +-- src/Parser/ConstExprParser.php | 68 +++++++ src/Parser/PhpDocParser.php | 17 +- tests/PHPStan/Parser/Doctrine/ApiResource.php | 34 ++++ tests/PHPStan/Parser/PhpDocParserTest.php | 167 ++++++++++++++++-- 7 files changed, 320 insertions(+), 38 deletions(-) create mode 100644 src/Ast/ConstExpr/DoctrineConstExprStringNode.php create mode 100644 tests/PHPStan/Parser/Doctrine/ApiResource.php diff --git a/phpstan.neon b/phpstan.neon index 0c336514..58ab0c1a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,7 @@ parameters: - tests excludePaths: - tests/PHPStan/*/data/* + - tests/PHPStan/Parser/Doctrine/ApiResource.php level: 8 ignoreErrors: - '#^Dynamic call to static method PHPUnit\\Framework\\Assert#' diff --git a/src/Ast/ConstExpr/DoctrineConstExprStringNode.php b/src/Ast/ConstExpr/DoctrineConstExprStringNode.php new file mode 100644 index 00000000..a503937b --- /dev/null +++ b/src/Ast/ConstExpr/DoctrineConstExprStringNode.php @@ -0,0 +1,42 @@ +value = $value; + } + + public function __toString(): string + { + return self::escape($this->value); + } + + public static function unescape(string $value): string + { + // from https://github.com/doctrine/annotations/blob/a9ec7af212302a75d1f92fa65d3abfbd16245a2a/lib/Doctrine/Common/Annotations/DocLexer.php#L103-L107 + return str_replace('""', '"', substr($value, 1, strlen($value) - 2)); + } + + private static function escape(string $value): string + { + // from https://github.com/phpstan/phpdoc-parser/issues/205#issuecomment-1662323656 + return sprintf('"%s"', str_replace('"', '""', $value)); + } + +} diff --git a/src/Lexer/Lexer.php b/src/Lexer/Lexer.php index 86ae8239..32539faf 100644 --- a/src/Lexer/Lexer.php +++ b/src/Lexer/Lexer.php @@ -35,19 +35,20 @@ class Lexer public const TOKEN_INTEGER = 20; public const TOKEN_SINGLE_QUOTED_STRING = 21; public const TOKEN_DOUBLE_QUOTED_STRING = 22; - public const TOKEN_IDENTIFIER = 23; - public const TOKEN_THIS_VARIABLE = 24; - public const TOKEN_VARIABLE = 25; - public const TOKEN_HORIZONTAL_WS = 26; - public const TOKEN_PHPDOC_EOL = 27; - public const TOKEN_OTHER = 28; - public const TOKEN_END = 29; - public const TOKEN_COLON = 30; - public const TOKEN_WILDCARD = 31; - public const TOKEN_OPEN_CURLY_BRACKET = 32; - public const TOKEN_CLOSE_CURLY_BRACKET = 33; - public const TOKEN_NEGATED = 34; - public const TOKEN_ARROW = 35; + public const TOKEN_DOCTRINE_ANNOTATION_STRING = 23; + public const TOKEN_IDENTIFIER = 24; + public const TOKEN_THIS_VARIABLE = 25; + public const TOKEN_VARIABLE = 26; + public const TOKEN_HORIZONTAL_WS = 27; + public const TOKEN_PHPDOC_EOL = 28; + public const TOKEN_OTHER = 29; + public const TOKEN_END = 30; + public const TOKEN_COLON = 31; + public const TOKEN_WILDCARD = 32; + public const TOKEN_OPEN_CURLY_BRACKET = 33; + public const TOKEN_CLOSE_CURLY_BRACKET = 34; + public const TOKEN_NEGATED = 35; + public const TOKEN_ARROW = 36; public const TOKEN_LABELS = [ self::TOKEN_REFERENCE => '\'&\'', @@ -79,6 +80,7 @@ class Lexer self::TOKEN_INTEGER => 'TOKEN_INTEGER', self::TOKEN_SINGLE_QUOTED_STRING => 'TOKEN_SINGLE_QUOTED_STRING', self::TOKEN_DOUBLE_QUOTED_STRING => 'TOKEN_DOUBLE_QUOTED_STRING', + self::TOKEN_DOCTRINE_ANNOTATION_STRING => 'TOKEN_DOCTRINE_ANNOTATION_STRING', self::TOKEN_IDENTIFIER => 'type', self::TOKEN_THIS_VARIABLE => '\'$this\'', self::TOKEN_VARIABLE => 'variable', @@ -180,6 +182,7 @@ private function generateRegexp(): string if ($this->parseDoctrineAnnotations) { $patterns[self::TOKEN_DOCTRINE_TAG] = '@[a-z_\\\\][a-z0-9_\:\\\\]*[a-z_][a-z0-9_]*'; + $patterns[self::TOKEN_DOCTRINE_ANNOTATION_STRING] = '"(?:""|[^"])*+"'; } // anything but TOKEN_CLOSE_PHPDOC or TOKEN_HORIZONTAL_WS or TOKEN_EOL diff --git a/src/Parser/ConstExprParser.php b/src/Parser/ConstExprParser.php index f97c260c..f273103e 100644 --- a/src/Parser/ConstExprParser.php +++ b/src/Parser/ConstExprParser.php @@ -23,6 +23,9 @@ class ConstExprParser /** @var bool */ private $useIndexAttributes; + /** @var bool */ + private $parseDoctrineStrings; + /** * @param array{lines?: bool, indexes?: bool} $usedAttributes */ @@ -36,6 +39,24 @@ public function __construct( $this->quoteAwareConstExprString = $quoteAwareConstExprString; $this->useLinesAttributes = $usedAttributes['lines'] ?? false; $this->useIndexAttributes = $usedAttributes['indexes'] ?? false; + $this->parseDoctrineStrings = false; + } + + /** + * @internal + */ + public function toDoctrine(): self + { + $self = new self( + $this->unescapeStrings, + $this->quoteAwareConstExprString, + [ + 'lines' => $this->useLinesAttributes, + 'indexes' => $this->useIndexAttributes, + ] + ); + $self->parseDoctrineStrings = true; + return $self; } public function parse(TokenIterator $tokens, bool $trimStrings = false): Ast\ConstExpr\ConstExprNode @@ -66,7 +87,41 @@ public function parse(TokenIterator $tokens, bool $trimStrings = false): Ast\Con ); } + if ($this->parseDoctrineStrings && $tokens->isCurrentTokenType(Lexer::TOKEN_DOCTRINE_ANNOTATION_STRING)) { + $value = $tokens->currentTokenValue(); + $tokens->next(); + + return $this->enrichWithAttributes( + $tokens, + new Ast\ConstExpr\DoctrineConstExprStringNode(Ast\ConstExpr\DoctrineConstExprStringNode::unescape($value)), + $startLine, + $startIndex + ); + } + if ($tokens->isCurrentTokenType(Lexer::TOKEN_SINGLE_QUOTED_STRING, Lexer::TOKEN_DOUBLE_QUOTED_STRING)) { + if ($this->parseDoctrineStrings) { + if ($tokens->isCurrentTokenType(Lexer::TOKEN_SINGLE_QUOTED_STRING)) { + throw new ParserException( + $tokens->currentTokenValue(), + $tokens->currentTokenType(), + $tokens->currentTokenOffset(), + Lexer::TOKEN_DOUBLE_QUOTED_STRING, + null, + $tokens->currentTokenLine() + ); + } + + $value = $tokens->currentTokenValue(); + $tokens->next(); + + return $this->enrichWithAttributes( + $tokens, + $this->parseDoctrineString($value, $tokens), + $startLine, + $startIndex + ); + } $value = $tokens->currentTokenValue(); $type = $tokens->currentTokenType(); if ($trimStrings) { @@ -214,6 +269,19 @@ private function parseArray(TokenIterator $tokens, int $endToken, int $startInde } + public function parseDoctrineString(string $value, TokenIterator $tokens): Ast\ConstExpr\DoctrineConstExprStringNode + { + $text = $value; + + while ($tokens->isCurrentTokenType(Lexer::TOKEN_DOUBLE_QUOTED_STRING, Lexer::TOKEN_DOCTRINE_ANNOTATION_STRING)) { + $text .= $tokens->currentTokenValue(); + $tokens->next(); + } + + return new Ast\ConstExpr\DoctrineConstExprStringNode(Ast\ConstExpr\DoctrineConstExprStringNode::unescape($text)); + } + + private function parseArrayItem(TokenIterator $tokens): Ast\ConstExpr\ConstExprArrayItemNode { $startLine = $tokens->currentTokenLine(); diff --git a/src/Parser/PhpDocParser.php b/src/Parser/PhpDocParser.php index 7b56e3bd..ff5f18f6 100644 --- a/src/Parser/PhpDocParser.php +++ b/src/Parser/PhpDocParser.php @@ -35,6 +35,9 @@ class PhpDocParser /** @var ConstExprParser */ private $constantExprParser; + /** @var ConstExprParser */ + private $doctrineConstantExprParser; + /** @var bool */ private $requireWhitespaceBeforeDescription; @@ -68,6 +71,7 @@ public function __construct( { $this->typeParser = $typeParser; $this->constantExprParser = $constantExprParser; + $this->doctrineConstantExprParser = $constantExprParser->toDoctrine(); $this->requireWhitespaceBeforeDescription = $requireWhitespaceBeforeDescription; $this->preserveTypeAliasesWithInvalidTypes = $preserveTypeAliasesWithInvalidTypes; $this->parseDoctrineAnnotations = $parseDoctrineAnnotations; @@ -688,7 +692,7 @@ private function parseDoctrineArgumentValue(TokenIterator $tokens) ); try { - $constExpr = $this->constantExprParser->parse($tokens, true); + $constExpr = $this->doctrineConstantExprParser->parse($tokens, true); if ($constExpr instanceof Ast\ConstExpr\ConstExprArrayNode) { throw $exception; } @@ -750,14 +754,15 @@ private function parseDoctrineArrayKey(TokenIterator $tokens) $key = new Ast\ConstExpr\ConstExprIntegerNode(str_replace('_', '', $tokens->currentTokenValue())); $tokens->next(); - } elseif ($tokens->isCurrentTokenType(Lexer::TOKEN_SINGLE_QUOTED_STRING)) { - $key = new Ast\ConstExpr\QuoteAwareConstExprStringNode(StringUnescaper::unescapeString($tokens->currentTokenValue()), Ast\ConstExpr\QuoteAwareConstExprStringNode::SINGLE_QUOTED); + } elseif ($tokens->isCurrentTokenType(Lexer::TOKEN_DOCTRINE_ANNOTATION_STRING)) { + $key = new Ast\ConstExpr\DoctrineConstExprStringNode(Ast\ConstExpr\DoctrineConstExprStringNode::unescape($tokens->currentTokenValue())); + $tokens->next(); } elseif ($tokens->isCurrentTokenType(Lexer::TOKEN_DOUBLE_QUOTED_STRING)) { - $key = new Ast\ConstExpr\QuoteAwareConstExprStringNode(StringUnescaper::unescapeString($tokens->currentTokenValue()), Ast\ConstExpr\QuoteAwareConstExprStringNode::DOUBLE_QUOTED); - + $value = $tokens->currentTokenValue(); $tokens->next(); + $key = $this->doctrineConstantExprParser->parseDoctrineString($value, $tokens); } else { $currentTokenValue = $tokens->currentTokenValue(); @@ -786,7 +791,7 @@ private function parseDoctrineArrayKey(TokenIterator $tokens) } $tokens->rollback(); - $constExpr = $this->constantExprParser->parse($tokens, true); + $constExpr = $this->doctrineConstantExprParser->parse($tokens, true); if (!$constExpr instanceof Ast\ConstExpr\ConstFetchNode) { throw new ParserException( $tokens->currentTokenValue(), diff --git a/tests/PHPStan/Parser/Doctrine/ApiResource.php b/tests/PHPStan/Parser/Doctrine/ApiResource.php new file mode 100644 index 00000000..21a958c5 --- /dev/null +++ b/tests/PHPStan/Parser/Doctrine/ApiResource.php @@ -0,0 +1,34 @@ + + * + * @Annotation + * @Target({"CLASS"}) + */ +final class ApiResource +{ + + /** @var string */ + public $shortName; + + /** @var string */ + public $description; + + /** @var string */ + public $iri; + + /** @var array */ + public $itemOperations; + + /** @var array */ + public $collectionOperations; + + /** @var array */ + public $attributes = []; + +} diff --git a/tests/PHPStan/Parser/PhpDocParserTest.php b/tests/PHPStan/Parser/PhpDocParserTest.php index ebf8e4a5..3139aec0 100644 --- a/tests/PHPStan/Parser/PhpDocParserTest.php +++ b/tests/PHPStan/Parser/PhpDocParserTest.php @@ -10,7 +10,7 @@ use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprIntegerNode; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstFetchNode; -use PHPStan\PhpDocParser\Ast\ConstExpr\QuoteAwareConstExprStringNode; +use PHPStan\PhpDocParser\Ast\ConstExpr\DoctrineConstExprStringNode; use PHPStan\PhpDocParser\Ast\Node; use PHPStan\PhpDocParser\Ast\NodeTraverser; use PHPStan\PhpDocParser\Ast\PhpDoc\AssertTagMethodValueNode; @@ -5445,8 +5445,8 @@ public function provideTagsWithBackslash(): Iterator new PhpDocTagNode( '@ORM\Mapping\JoinColumn', new DoctrineTagValueNode(new DoctrineAnnotation('@ORM\Mapping\JoinColumn', [ - new DoctrineArgument(new IdentifierTypeNode('name'), new ConstExprStringNode('column_id')), - new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new ConstExprStringNode('id')), + new DoctrineArgument(new IdentifierTypeNode('name'), new DoctrineConstExprStringNode('column_id')), + new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new DoctrineConstExprStringNode('id')), ]), '') ), ]), @@ -6041,6 +6041,141 @@ public function provideDoctrineData(): Iterator null, [new Doctrine\X()], ]; + + $apiResource = new Doctrine\ApiResource(); + $apiResource->itemOperations = [ + 'get' => [ + 'security' => 'is_granted(' . PHP_EOL . + "constant('REDACTED')," . PHP_EOL . + 'object' . PHP_EOL . ')', + 'normalization_context' => [ + 'groups' => ['Redacted:read'], + ], + ], + ]; + yield [ + 'Regression test for issue #207', + '/**' . PHP_EOL . + ' * @ApiResource(' . PHP_EOL . + ' * itemOperations={' . PHP_EOL . + ' * "get"={' . PHP_EOL . + ' * "security"="is_granted(' . PHP_EOL . + "constant('REDACTED')," . PHP_EOL . + 'object' . PHP_EOL . + ')",' . PHP_EOL . + ' * "normalization_context"={"groups"={"Redacted:read"}}' . PHP_EOL . + ' * }' . PHP_EOL . + ' * }' . PHP_EOL . + ' * )' . PHP_EOL . + ' */', + new PhpDocNode([ + new PhpDocTagNode('@ApiResource', new DoctrineTagValueNode( + new DoctrineAnnotation('@ApiResource', [ + new DoctrineArgument(new IdentifierTypeNode('itemOperations'), new DoctrineArray([ + new DoctrineArrayItem( + new DoctrineConstExprStringNode('get'), + new DoctrineArray([ + new DoctrineArrayItem( + new DoctrineConstExprStringNode('security'), + new DoctrineConstExprStringNode('is_granted(' . PHP_EOL . + "constant('REDACTED')," . PHP_EOL . + 'object' . PHP_EOL . + ')') + ), + new DoctrineArrayItem( + new DoctrineConstExprStringNode('normalization_context'), + new DoctrineArray([ + new DoctrineArrayItem( + new DoctrineConstExprStringNode('groups'), + new DoctrineArray([ + new DoctrineArrayItem(null, new DoctrineConstExprStringNode('Redacted:read')), + ]) + ), + ]) + ), + ]) + ), + ])), + ]), + '' + )), + ]), + null, + null, + [$apiResource], + ]; + + $xWithString = new Doctrine\X(); + $xWithString->a = '"bar"'; + yield [ + 'Escaped strings', + '/** @X(a="""bar""") */', + new PhpDocNode([ + new PhpDocTagNode('@X', new DoctrineTagValueNode( + new DoctrineAnnotation('@X', [ + new DoctrineArgument(new IdentifierTypeNode('a'), new DoctrineConstExprStringNode($xWithString->a)), + ]), + '' + )), + ]), + null, + null, + [$xWithString], + ]; + + $xWithString2 = new Doctrine\X(); + $xWithString2->a = 'Allowed choices are "bar" or "baz".'; + yield [ + 'Escaped strings 2', + '/** @X(a="Allowed choices are ""bar"" or ""baz"".") */', + new PhpDocNode([ + new PhpDocTagNode('@X', new DoctrineTagValueNode( + new DoctrineAnnotation('@X', [ + new DoctrineArgument(new IdentifierTypeNode('a'), new DoctrineConstExprStringNode($xWithString2->a)), + ]), + '' + )), + ]), + null, + null, + [$xWithString2], + ]; + + $xWithString3 = new Doctrine\X(); + $xWithString3->a = 'In PHP, "" is an empty string'; + yield [ + 'Escaped strings 3', + '/** @X(a="In PHP, """" is an empty string") */', + new PhpDocNode([ + new PhpDocTagNode('@X', new DoctrineTagValueNode( + new DoctrineAnnotation('@X', [ + new DoctrineArgument(new IdentifierTypeNode('a'), new DoctrineConstExprStringNode($xWithString3->a)), + ]), + '' + )), + ]), + null, + null, + [$xWithString3], + ]; + + $xWithString4 = new Doctrine\X(); + $xWithString4->a = '"May the Force be with you," he said.'; + yield [ + 'Escaped strings 4', + '/** @X(a="""May the Force be with you,"" he said.") */', + new PhpDocNode([ + new PhpDocTagNode('@X', new DoctrineTagValueNode( + new DoctrineAnnotation('@X', [ + new DoctrineArgument(new IdentifierTypeNode('a'), new DoctrineConstExprStringNode($xWithString4->a)), + ]), + '' + )), + ]), + null, + null, + [$xWithString4], + ]; } public function provideDoctrineWithoutDoctrineCheckData(): Iterator @@ -6051,7 +6186,7 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new PhpDocNode([ new PhpDocTagNode('@DummyAnnotation', new DoctrineTagValueNode( new DoctrineAnnotation('@DummyAnnotation', [ - new DoctrineArgument(new IdentifierTypeNode('dummyValue'), new ConstExprStringNode('hello')), + new DoctrineArgument(new IdentifierTypeNode('dummyValue'), new DoctrineConstExprStringNode('hello')), ]), '' )), @@ -6069,17 +6204,17 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new PhpDocNode([ new PhpDocTagNode('@DummyJoinTable', new DoctrineTagValueNode( new DoctrineAnnotation('@DummyJoinTable', [ - new DoctrineArgument(new IdentifierTypeNode('name'), new ConstExprStringNode('join_table')), + new DoctrineArgument(new IdentifierTypeNode('name'), new DoctrineConstExprStringNode('join_table')), new DoctrineArgument(new IdentifierTypeNode('joinColumns'), new DoctrineArray([ new DoctrineArrayItem(null, new DoctrineAnnotation('@DummyJoinColumn', [ - new DoctrineArgument(new IdentifierTypeNode('name'), new ConstExprStringNode('col1')), - new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new ConstExprStringNode('col2')), + new DoctrineArgument(new IdentifierTypeNode('name'), new DoctrineConstExprStringNode('col1')), + new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new DoctrineConstExprStringNode('col2')), ])), ])), new DoctrineArgument(new IdentifierTypeNode('inverseJoinColumns'), new DoctrineArray([ new DoctrineArrayItem(null, new DoctrineAnnotation('@DummyJoinColumn', [ - new DoctrineArgument(new IdentifierTypeNode('name'), new ConstExprStringNode('col3')), - new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new ConstExprStringNode('col4')), + new DoctrineArgument(new IdentifierTypeNode('name'), new DoctrineConstExprStringNode('col3')), + new DoctrineArgument(new IdentifierTypeNode('referencedColumnName'), new DoctrineConstExprStringNode('col4')), ])), ])), ]), @@ -6107,7 +6242,7 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new PhpDocNode([ new PhpDocTagNode('@DummyAnnotation', new DoctrineTagValueNode( new DoctrineAnnotation('@DummyAnnotation', [ - new DoctrineArgument(new IdentifierTypeNode('dummyValue'), new ConstExprStringNode('bar')), + new DoctrineArgument(new IdentifierTypeNode('dummyValue'), new DoctrineConstExprStringNode('bar')), ]), '' )), @@ -6124,7 +6259,7 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new PhpDocTagNode('@DummyId', new GenericTagValueNode('')), new PhpDocTagNode('@DummyColumn', new DoctrineTagValueNode( new DoctrineAnnotation('@DummyColumn', [ - new DoctrineArgument(new IdentifierTypeNode('type'), new ConstExprStringNode('integer')), + new DoctrineArgument(new IdentifierTypeNode('type'), new DoctrineConstExprStringNode('integer')), ]), '' )), @@ -6164,7 +6299,7 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new DoctrineArrayItem(null, new ConstExprIntegerNode('1')), new DoctrineArrayItem(null, new ConstExprIntegerNode('2')), new DoctrineArrayItem(null, new DoctrineArray([ - new DoctrineArrayItem(new QuoteAwareConstExprStringNode('key', QuoteAwareConstExprStringNode::DOUBLE_QUOTED), new DoctrineAnnotation( + new DoctrineArrayItem(new DoctrineConstExprStringNode('key'), new DoctrineAnnotation( '@Name', [] )), @@ -6229,7 +6364,7 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator new PhpDocTagNode('@Name', new DoctrineTagValueNode( new DoctrineAnnotation('@Name', [ new DoctrineArgument(null, new DoctrineArray([ - new DoctrineArrayItem(new QuoteAwareConstExprStringNode('foo', QuoteAwareConstExprStringNode::DOUBLE_QUOTED), new ConstExprStringNode('bar')), + new DoctrineArrayItem(new DoctrineConstExprStringNode('foo'), new DoctrineConstExprStringNode('bar')), ])), ]), '' @@ -6237,12 +6372,6 @@ public function provideDoctrineWithoutDoctrineCheckData(): Iterator ]), ]; - //yield [ - // 'Escaped strings', - // '/** @Doctrine\Tests\Common\Annotations\Name(foo="""bar""") */', - // new PhpDocNode([]), - //]; - yield [ 'More tags on the same line with description inbetween, second Doctrine one cannot have parse error', '/** @X test @Z(test= */',