Repository: microsoft/tolerant-php-parser (php)

Issue:
Quoted string parsing doesn't work as expected
As requested in this [PR](https://github.com/microsoft/tolerant-php-parser/pull/396#discussion_r1063150728)

Currently when parsing

```
<?php "'hello world'"';
```

Results in:

```
hello world
```

and not

```
'hello world'
```

same for single and double quotes.

Pull request description:
Phpstan level 4
This PR:

- adds support for PHPStan Level 4
- fixes string literal parsing and adds a test for it  (fixes #397)


Patch:
diff --git a/composer.json b/composer.json
index 1583949..6f5e3e8 100644
--- a/composer.json
+++ b/composer.json
@@ -18,5 +18,8 @@
     ],
     "autoload": {
         "psr-4": { "Microsoft\\PhpParser\\": ["src/"] }
+    },
+    "autoload-dev": {
+        "psr-4": { "Microsoft\\PhpParser\\Tests\\Unit\\": ["tests/unit/"] }
     }
 }
diff --git a/phpstan.neon b/phpstan.neon
index ee6614c..5776c4b 100644
--- a/phpstan.neon
+++ b/phpstan.neon
@@ -1,5 +1,5 @@
 parameters:
-    level: 3
+    level: 4
     paths:
         - src/
     ignoreErrors:
diff --git a/src/DiagnosticsProvider.php b/src/DiagnosticsProvider.php
index 1630679..80cc386 100644
--- a/src/DiagnosticsProvider.php
+++ b/src/DiagnosticsProvider.php
@@ -51,6 +51,8 @@ class DiagnosticsProvider {
         if ($node instanceof Node) {
             return $node->getDiagnosticForNode();
         }
+
+        /** @phpstan-ignore-next-line because it says "unreachable" statement */
         return null;
     }
 
diff --git a/src/Node.php b/src/Node.php
index f235636..0f1e2e6 100644
--- a/src/Node.php
+++ b/src/Node.php
@@ -437,10 +437,7 @@ abstract class Node implements \JsonSerializable {
     public function getDescendantNodeAtPosition(int $pos) {
         foreach ($this->getChildNodes() as $child) {
             if ($child->containsPosition($pos)) {
-                $node = $child->getDescendantNodeAtPosition($pos);
-                if (!is_null($node)) {
-                    return $node;
-                }
+                return $child->getDescendantNodeAtPosition($pos);
             }
         }
 
@@ -590,6 +587,7 @@ abstract class Node implements \JsonSerializable {
             ? $this
             : $this->getFirstAncestor(NamespaceDefinition::class, SourceFileNode::class);
 
+        /** @phpstan-ignore-next-line result is always false -- not sure this can happen */
         if ($namespaceDefinition instanceof NamespaceDefinition && !($namespaceDefinition->parent instanceof SourceFileNode)) {
             $namespaceDefinition = $namespaceDefinition->getFirstAncestor(SourceFileNode::class);
         }
diff --git a/src/Node/Expression/ArgumentExpression.php b/src/Node/Expression/ArgumentExpression.php
index 06ddbaf..87ccc9a 100644
--- a/src/Node/Expression/ArgumentExpression.php
+++ b/src/Node/Expression/ArgumentExpression.php
@@ -6,6 +6,7 @@
 
 namespace Microsoft\PhpParser\Node\Expression;
 
+use Microsoft\PhpParser\MissingToken;
 use Microsoft\PhpParser\Node\Expression;
 use Microsoft\PhpParser\Token;
 
@@ -19,7 +20,7 @@ class ArgumentExpression extends Expression {
     /** @var Token|null */
     public $dotDotDotToken;
 
-    /** @var Expression|null null for first-class callable syntax */
+    /** @var Expression|MissingToken|null for first-class callable syntax */
     public $expression;
 
     const CHILD_NAMES = [
diff --git a/src/Node/InterfaceBaseClause.php b/src/Node/InterfaceBaseClause.php
index 4bca0e4..7b6ed9c 100644
--- a/src/Node/InterfaceBaseClause.php
+++ b/src/Node/InterfaceBaseClause.php
@@ -10,7 +10,7 @@ use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Token;
 
 class InterfaceBaseClause extends Node {
-    /** @var Token */
+    /** @var Token|null */
     public $extendsKeyword;
 
     /** @var DelimitedList\QualifiedNameList */
diff --git a/src/Node/NamespaceUseGroupClause.php b/src/Node/NamespaceUseGroupClause.php
index 950195e..4fc4c2b 100644
--- a/src/Node/NamespaceUseGroupClause.php
+++ b/src/Node/NamespaceUseGroupClause.php
@@ -11,7 +11,7 @@ use Microsoft\PhpParser\Token;
 
 class NamespaceUseGroupClause extends Node {
 
-    /** @var Token */
+    /** @var Token|null */
     public $functionOrConst;
     /** @var QualifiedName */
     public $namespaceName;
diff --git a/src/Node/QualifiedName.php b/src/Node/QualifiedName.php
index 6955dc0..8a71f1d 100644
--- a/src/Node/QualifiedName.php
+++ b/src/Node/QualifiedName.php
@@ -20,9 +20,9 @@ use Microsoft\PhpParser\TokenKind;
 class QualifiedName extends Node implements NamespacedNameInterface {
     use NamespacedNameTrait;
 
-    /** @var Token */
+    /** @var Token|null */
     public $globalSpecifier; // \_opt
-    /** @var RelativeSpecifier */
+    /** @var RelativeSpecifier|null */
     public $relativeSpecifier; // namespace\
     /** @var array */
     public $nameParts;
@@ -86,9 +86,7 @@ class QualifiedName extends Node implements NamespacedNameInterface {
             $this->parent instanceof Node\NamespaceUseClause ||
             $this->parent instanceof Node\NamespaceUseGroupClause ||
             $this->parent->parent instanceof Node\TraitUseClause ||
-            $this->parent instanceof Node\TraitSelectOrAliasClause ||
-            ($this->parent instanceof TraitSelectOrAliasClause &&
-            ($this->parent->asOrInsteadOfKeyword == null || $this->parent->asOrInsteadOfKeyword->kind === TokenKind::AsKeyword))
+            $this->parent instanceof Node\TraitSelectOrAliasClause
         ) {
             return null;
         }
@@ -157,10 +155,8 @@ class QualifiedName extends Node implements NamespacedNameInterface {
 
     /**
      * @param ResolvedName[] $importTable
-     * @param bool $isCaseSensitive
-     * @return string|null
      */
-    private function tryResolveFromImportTable($importTable, bool $isCaseSensitive = false) {
+    private function tryResolveFromImportTable($importTable, bool $isCaseSensitive = false): ?ResolvedName {
         $content = $this->getFileContents();
         $index = $this->nameParts[0]->getText($content);
 //        if (!$isCaseSensitive) {
diff --git a/src/Node/RelativeSpecifier.php b/src/Node/RelativeSpecifier.php
index 8c301ed..5e31fbd 100644
--- a/src/Node/RelativeSpecifier.php
+++ b/src/Node/RelativeSpecifier.php
@@ -13,7 +13,7 @@ class RelativeSpecifier extends Node {
     /** @var Token */
     public $namespaceKeyword;
 
-    /** @var Token */
+    /** @var Token|null */
     public $backslash;
 
     const CHILD_NAMES = [
diff --git a/src/Node/Statement/NamespaceUseDeclaration.php b/src/Node/Statement/NamespaceUseDeclaration.php
index bc12700..3cec324 100644
--- a/src/Node/Statement/NamespaceUseDeclaration.php
+++ b/src/Node/Statement/NamespaceUseDeclaration.php
@@ -18,7 +18,7 @@ class NamespaceUseDeclaration extends StatementNode {
     public $useKeyword;
     /** @var Token */
     public $functionOrConst;
-    /** @var DelimitedList\NamespaceUseClauseList */
+    /** @var DelimitedList\NamespaceUseClauseList|null */
     public $useClauses;
     /** @var Token */
     public $semicolon;
diff --git a/src/Node/StringLiteral.php b/src/Node/StringLiteral.php
index 00018e0..c2d45e3 100644
--- a/src/Node/StringLiteral.php
+++ b/src/Node/StringLiteral.php
@@ -10,7 +10,7 @@ use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Token;
 
 class StringLiteral extends Expression {
-    /** @var Token */
+    /** @var Token|null */
     public $startQuote;
 
     /** @var Token[]|Node[]|Token */
@@ -25,8 +25,7 @@ class StringLiteral extends Expression {
         'endQuote',
     ];
 
-    public function getStringContentsText() {
-        // TODO add tests
+    public function getStringContentsText(): string {
         $stringContents = "";
         if (isset($this->startQuote)) {
             foreach ($this->children as $child) {
@@ -34,8 +33,21 @@ class StringLiteral extends Expression {
                 $stringContents .= $child->getFullText($contents);
             }
         } else {
-            // TODO ensure string consistency (all strings should have start / end quote)
-            $stringContents = trim($this->children->getText($this->getFileContents()), '"\'');
+            $children = $this->children;
+            if ($children instanceof Token) {
+                $value = (string)$children->getText($this->getFileContents());
+                $startQuote = substr($value, 0, 1);
+
+                if ($startQuote === '\'') {
+                    return rtrim(substr($value, 1), '\'');
+                }
+
+                if ($startQuote === '"') {
+                    return rtrim(substr($value, 1), '"');
+                }
+            }
+
+            return '';
         }
         return $stringContents;
     }
diff --git a/src/Parser.php b/src/Parser.php
index c5a4af2..e3d6a92 100644
--- a/src/Parser.php
+++ b/src/Parser.php
@@ -1404,7 +1404,7 @@ class Parser {
         return $expression;
     }
 
-    private function parseStringLiteralExpression2($parentNode) {
+    private function parseStringLiteralExpression2($parentNode): StringLiteral {
         // TODO validate input token
         $expression = new StringLiteral();
         $expression->parent = $parentNode;
@@ -1416,6 +1416,11 @@ class Parser {
                 case TokenKind::DollarOpenBraceToken:
                 case TokenKind::OpenBraceDollarToken:
                     $expression->children[] = $this->eat(TokenKind::DollarOpenBraceToken, TokenKind::OpenBraceDollarToken);
+                    /** 
+                     * @phpstan-ignore-next-line "Strict comparison using
+                     * === between 403|404 and 408 will always evaluate to
+                     * false" is wrong because those tokens were eaten above
+                     */
                     if ($this->getCurrentToken()->kind === TokenKind::StringVarname) {
                         $expression->children[] = $this->parseComplexDollarTemplateStringExpression($expression);
                     } else {
@@ -1654,7 +1659,7 @@ class Parser {
         do {
             if ($isElementStartFn($token)) {
                 $node->addElement($parseElementFn($node));
-            } elseif (!$allowEmptyElements || ($allowEmptyElements && !$this->checkAnyToken($delimiter))) {
+            } elseif (!$allowEmptyElements || !$this->checkAnyToken($delimiter)) {
                 break;
             }
 
@@ -1764,7 +1769,7 @@ class Parser {
         };
     }
 
-    private function parseRelativeSpecifier($parentNode) {
+    private function parseRelativeSpecifier($parentNode): ?RelativeSpecifier {
         $node = new RelativeSpecifier();
         $node->parent = $parentNode;
         $node->namespaceKeyword = $this->eatOptional1(TokenKind::NamespaceKeyword);
@@ -2113,7 +2118,7 @@ class Parser {
     /**
      * @param int $precedence
      * @param Node $parentNode
-     * @return Expression
+     * @return Expression|MissingToken
      */
     private function parseBinaryExpressionOrHigher($precedence, $parentNode) {
         $leftOperand = $this->parseUnaryExpressionOrHigher($parentNode);
@@ -2214,6 +2219,7 @@ class Parser {
                     }
                     break;
                 case TokenKind::QuestionToken:
+                    /** @phpstan-ignore-next-line This seems impossible, questionToken is always set AFAICS but ignoring to be safe */
                     if ($parentNode instanceof TernaryExpression && !isset($parentNode->questionToken)) {
                         // Workaround to parse "a ? b : c ? d : e" as "(a ? b : c) ? d : e"
                         break 2;
@@ -3571,7 +3577,7 @@ class Parser {
         return $namespaceUseDeclaration;
     }
 
-    private function parseNamespaceUseClauseList($parentNode) {
+    private function parseNamespaceUseClauseList($parentNode): ?DelimitedList\NamespaceUseClauseList {
         return $this->parseDelimitedList(
             DelimitedList\NamespaceUseClauseList::class,
             TokenKind::CommaToken,
@@ -3601,7 +3607,7 @@ class Parser {
         );
     }
 
-    private function parseNamespaceUseGroupClauseList($parentNode) {
+    private function parseNamespaceUseGroupClauseList($parentNode): ?DelimitedList\NamespaceUseGroupClauseList {
         return $this->parseDelimitedList(
             DelimitedList\NamespaceUseGroupClauseList::class,
             TokenKind::CommaToken,


Test patch:
diff --git a/tests/unit/Node/StringLiteralTest.php b/tests/unit/Node/StringLiteralTest.php
new file mode 100644
index 0000000..9aa1184
--- /dev/null
+++ b/tests/unit/Node/StringLiteralTest.php
@@ -0,0 +1,56 @@
+<?php
+
+namespace Microsoft\PhpParser\Tests\Unit\Node;
+
+use Generator;
+use Microsoft\PhpParser\Node\StringLiteral;
+use Microsoft\PhpParser\Parser;
+use PHPUnit\Framework\TestCase;
+
+class StringLiteralTest extends TestCase
+{
+    /**
+     * @dataProvider provideGetStringContentsText
+     */
+    public function testGetStringContentsText(string $source, string $expected): void
+    {
+        $stringLiteral = (new Parser())->parseSourceFile($source)->getFirstDescendantNode(StringLiteral::class);
+        self::assertInstanceOf(StringLiteral::class, $stringLiteral);
+        self::assertEquals($expected, $stringLiteral->getStringContentsText());
+    }
+
+    /**
+     * @return Generator<string,array{string,string}>
+     */
+    public function provideGetStringContentsText(): Generator
+    {
+        yield 'empty string double quotes' => [
+            '<?php "";',
+            '',
+        ];
+        yield 'empty string single quotes' => [
+            "<?php '';",
+            '',
+        ];
+        yield 'string double quotes' => [
+            '<?php "hello world";',
+            'hello world',
+        ];
+        yield 'string single quotes' => [
+            "<?php 'hello world';",
+            'hello world',
+        ];
+        yield 'string that starts and ends with double quotes' => [
+            '<?php \'"hello world"\'',
+            '"hello world"',
+        ];
+        yield 'string that starts and ends with single quotes' => [
+            '<?php "\'hello world\'"',
+            '\'hello world\'',
+        ];
+        yield 'backtick string' => [
+            '<?php `hello world`',
+            'hello world',
+        ];
+    }
+}