Use PHPDoc @param string ...$classNames

TysonAndre · dantleech · commit ce5acf3186ef · 2023-01-05T23:50:26.000Z
The `string` is the type of each argument.
The `...` indicates that it is an array of that type, like
`function (string ...$classNames)` also would indicate.

Update code to PHPStan Level 3

Added ReturnTypeWillChange

Fix return type for Expression

Add type hints for returned variables

Add generic parameter for delimted list

Fixing type hints

Ignore co-variance error

Remove TODO - it already must always be a Node

Ignore error

Add missing types

Update phpstan to level 3

Add phpstan to dev reqs

No need to install phpstan independently

Do not override unary expression operand

It seems to me that the operand can be any expression

Added token to operand types -- should this be MissingToken?

Include tokens in return types

ExpressionStatement =&gt; EchoStatement

It seems this is always an EchoStatement not an ExpressionStatement

unaryExpressionOrHigher can return a ThrowExpression

Remove overridden property

Add Token to union

Remove QualifiedName and rename local variable

Remove unused import

Add return types

Remove trailing whitespace

Add MissingToken type

Ignore "should not happen" statement

Bump to level 4

Remove !is_null conditional branch - it always returns Node

NamespaceUseDeclaration#useClauses is technically nullable

NamespaceUseGroupCluase#functionOrConst can be NULL

Add TODO

Removed redundant condition (check)

Add return type

Add retutn type, remove inaccurate docblock

Fix bug with get string literal text, as it returned the quotes

Ignore assumed false-positive from PHPStan

If allowEmptyElements if `false`...

... then it MUST be true in the right hand side of ||

backslash can be NULL

BinaryExpresionOrHigher can return MIssingToken

Ignoring error to be safe (as commented) and more type hints

Set level to 4

Add explanation
diff --git 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
@@ -1,5 +1,5 @@
 parameters:
-    level: 3
+    level: 4
     paths:
         - src/
     ignoreErrors:
diff --git a/src/DiagnosticsProvider.php b/src/DiagnosticsProvider.php
@@ -51,6 +51,8 @@ public static function checkDiagnostics($node) {
         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
@@ -437,10 +437,7 @@ public function getLastChild() {
     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 @@ public function getNamespaceDefinition() {
             ? $this
             : $this->getFirstAncestor(NamespaceDefinition::class, SourceFileNode::class);
 
+        /** @phpstan-ignore-next-line TODO: can this happen? test with framework test cases */
         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
@@ -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
@@ -10,7 +10,7 @@
 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
@@ -11,7 +11,7 @@
 
 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
@@ -20,9 +20,9 @@
 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 @@ public function getResolvedName($namespaceDefinition = null) {
             $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 @@ public function getLastNamePart() {
 
     /**
      * @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
@@ -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
@@ -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
@@ -10,7 +10,7 @@
 use Microsoft\PhpParser\Token;
 
 class StringLiteral extends Expression {
-    /** @var Token */
+    /** @var Token|null */
     public $startQuote;
 
     /** @var Token[]|Node[]|Token */
@@ -25,17 +25,29 @@ 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) {
                 $contents = $this->getFileContents();
                 $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
@@ -1404,7 +1404,7 @@ private function parseStringLiteralExpression($parentNode) {
         return $expression;
     }
 
-    private function parseStringLiteralExpression2($parentNode) {
+    private function parseStringLiteralExpression2($parentNode): StringLiteral {
         // TODO validate input token
         $expression = new StringLiteral();
         $expression->parent = $parentNode;
@@ -1416,6 +1416,7 @@ private function parseStringLiteralExpression2($parentNode) {
                 case TokenKind::DollarOpenBraceToken:
                 case TokenKind::OpenBraceDollarToken:
                     $expression->children[] = $this->eat(TokenKind::DollarOpenBraceToken, TokenKind::OpenBraceDollarToken);
+                    /** @phpstan-ignore-next-line getCurrentToken is not pure, but PHPStan thinks its DollarOpenBrace or OpenBraceDollarToken */
                     if ($this->getCurrentToken()->kind === TokenKind::StringVarname) {
                         $expression->children[] = $this->parseComplexDollarTemplateStringExpression($expression);
                     } else {
@@ -1654,7 +1655,7 @@ private function parseDelimitedList($className, $delimiter, $isElementStartFn, $
         do {
             if ($isElementStartFn($token)) {
                 $node->addElement($parseElementFn($node));
-            } elseif (!$allowEmptyElements || ($allowEmptyElements && !$this->checkAnyToken($delimiter))) {
+            } elseif (!$allowEmptyElements || !$this->checkAnyToken($delimiter)) {
                 break;
             }
 
@@ -1764,7 +1765,7 @@ function ($parentNode) {
         };
     }
 
-    private function parseRelativeSpecifier($parentNode) {
+    private function parseRelativeSpecifier($parentNode): ?RelativeSpecifier {
         $node = new RelativeSpecifier();
         $node->parent = $parentNode;
         $node->namespaceKeyword = $this->eatOptional1(TokenKind::NamespaceKeyword);
@@ -2113,7 +2114,7 @@ private function parseUnaryExpressionOrHigher($parentNode) {
     /**
      * @param int $precedence
      * @param Node $parentNode
-     * @return Expression
+     * @return Expression|MissingToken
      */
     private function parseBinaryExpressionOrHigher($precedence, $parentNode) {
         $leftOperand = $this->parseUnaryExpressionOrHigher($parentNode);
@@ -2214,6 +2215,7 @@ private function parseBinaryExpressionOrHigher($precedence, $parentNode) {
                     }
                     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 +3573,7 @@ private function parseNamespaceUseDeclaration($parentNode) {
         return $namespaceUseDeclaration;
     }
 
-    private function parseNamespaceUseClauseList($parentNode) {
+    private function parseNamespaceUseClauseList($parentNode): ?DelimitedList\NamespaceUseClauseList {
         return $this->parseDelimitedList(
             DelimitedList\NamespaceUseClauseList::class,
             TokenKind::CommaToken,
@@ -3601,7 +3603,7 @@ function ($parentNode) {
         );
     }
 
-    private function parseNamespaceUseGroupClauseList($parentNode) {
+    private function parseNamespaceUseGroupClauseList($parentNode): ?DelimitedList\NamespaceUseGroupClauseList {
         return $this->parseDelimitedList(
             DelimitedList\NamespaceUseGroupClauseList::class,
             TokenKind::CommaToken,
diff --git a/tests/unit/Node/StringLiteralTest.php 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',
+        ];
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -18,5 +18,8 @@`
`18`	`18`	`],`
`19`	`19`	`"autoload": {`
`20`	`20`	`"psr-4": { "Microsoft\\PhpParser\\": ["src/"] }`
	`21`	`+ },`
	`22`	`+ "autoload-dev": {`
	`23`	`+ "psr-4": { "Microsoft\\PhpParser\\Tests\\Unit\\": ["tests/unit/"] }`
`21`	`24`	`}`
`22`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,8 @@ public static function checkDiagnostics($node) {`
`51`	`51`	`if ($node instanceof Node) {`
`52`	`52`	`return $node->getDiagnosticForNode();`
`53`	`53`	`}`
	`54`	`+`
	`55`	`+ /** @phpstan-ignore-next-line because it says "unreachable" statement */`
`54`	`56`	`return null;`
`55`	`57`	`}`
`56`	`58`
Original file line number	Diff line number	Diff line change
`@@ -437,10 +437,7 @@ public function getLastChild() {`
`437`	`437`	`public function getDescendantNodeAtPosition(int $pos) {`
`438`	`438`	`foreach ($this->getChildNodes() as $child) {`
`439`	`439`	`if ($child->containsPosition($pos)) {`
`440`		`- $node = $child->getDescendantNodeAtPosition($pos);`
`441`		`- if (!is_null($node)) {`
`442`		`- return $node;`
`443`		`- }`
	`440`	`+ return $child->getDescendantNodeAtPosition($pos);`
`444`	`441`	`}`
`445`	`442`	`}`
`446`	`443`
`@@ -590,6 +587,7 @@ public function getNamespaceDefinition() {`
`590`	`587`	`? $this`
`591`	`588`	`: $this->getFirstAncestor(NamespaceDefinition::class, SourceFileNode::class);`
`592`	`589`
	`590`	`+ /** @phpstan-ignore-next-line TODO: can this happen? test with framework test cases */`
`593`	`591`	`if ($namespaceDefinition instanceof NamespaceDefinition && !($namespaceDefinition->parent instanceof SourceFileNode)) {`
`594`	`592`	`$namespaceDefinition = $namespaceDefinition->getFirstAncestor(SourceFileNode::class);`
`595`	`593`	`}`