Merge pull request microsoft#396 from phpactor/phpstan-level-4

roblourens · web-flow · commit b443ec89137b · 2023-01-08T14:13:52.000-08:00
Phpstan level 4
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 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
@@ -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,11 @@ private function parseStringLiteralExpression2($parentNode) {
                 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 @@ 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 +1769,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 +2118,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 +2219,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 +3577,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 +3607,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 result is always false -- not sure this can happen */`
`593`	`591`	`if ($namespaceDefinition instanceof NamespaceDefinition && !($namespaceDefinition->parent instanceof SourceFileNode)) {`
`594`	`592`	`$namespaceDefinition = $namespaceDefinition->getFirstAncestor(SourceFileNode::class);`
`595`	`593`	`}`