WIP (Broken) Optimize getDiagnostics

benchmark_get_diagnostics.php

@@ -0,0 +1,25 @@
+<?php
+// Autoload required classes
+require __DIR__ . "/vendor/autoload.php";
+
+use Microsoft\PhpParser\{DiagnosticsProvider, Node, Parser, PositionUtilities};
+
+
+const ITERATIONS = 100;
+// Return and print an AST from string contents
+$main = function() {
+    // Instantiate new parser instance
+    $parser = new Parser();
+    $t0 = microtime(true);
+    $astNode = $parser->parseSourceFile(file_get_contents('src/Parser.php'));
+    $t1 = microtime(true);
+    for ($i = 0; $i < ITERATIONS; $i++) {
+        $diagnostics = DiagnosticsProvider::getDiagnostics($astNode);
+    }
+    $t2 = microtime(true);
+    printf("Average time to generate diagnostics: %.6f\n", ($t2 - $t1) / ITERATIONS);
+    printf("time to parse: %.6f\n", ($t1 - $t0));
+    global $__counts;
+    var_export($__counts);
+};
+$main();

phpunit.xml

@@ -20,21 +20,8 @@
       <file>tests/ParserGrammarTest.php</file>
     </testsuite>
 
-    <testsuite name="validation">
-      <!--
-      Validates against real-world scenarios.
-      * INPUT: All files in `validation/frameworks/<framework-name>/*`
-      * OUTPUT: Failing tests are output to `tests/output/<framework-name>`
-      -->
-      <file>tests/ParserFrameworkValidationTests.php</file>
-    </testsuite>
-
     <testsuite name="api">
-      <file>tests/api/NodeApiTest.php</file>
-      <file>tests/api/getNodeAtPosition.php</file>
-      <file>tests/api/getResolvedName.php</file>
-      <file>tests/api/PositionUtilitiesTest.php</file>
-      <file>tests/api/TextEditTest.php</file>
+
     </testsuite>
 
     <testsuite name="performance">

src/DiagnosticsProvider.php

@@ -10,127 +10,80 @@
 
 class DiagnosticsProvider {
 
-    private static $tokenKindToText;
+    /**
+     * @param int $kind (must be a valid token kind)
+     * @return string
+     */
+    public static function getTextForTokenKind($kind) {
+        static $tokenKindToText;
+        if (!isset($tokenKindToText)) {
+            $tokenKindToText = \array_flip(\array_merge(
+                TokenStringMaps::OPERATORS_AND_PUNCTUATORS,
+                TokenStringMaps::KEYWORDS,
+                TokenStringMaps::RESERVED_WORDS
+            ));
+        }
+        return $tokenKindToText[$kind];
+    }
 
     /**
      * Returns the diagnostic for $node, or null.
-     * @param \Microsoft\PhpParser\Node $node
+     * @param \Microsoft\PhpParser\Node|\Microsoft\PhpParser\Token $node
      * @return Diagnostic|null
      */
     public static function checkDiagnostics($node) {
-        if (!isset(self::$tokenKindToText)) {
-            self::$tokenKindToText = \array_flip(\array_merge(
+        if ($node instanceof Token) {
+            if (\get_class($node) === Token::class) {
+                return null;
+            }
+            return self::checkDiagnosticForUnexpectedToken($node);
+        }
+
+        if ($node instanceof Node) {
+            return $node->getDiagnosticForNode();
+        }
+        return null;
+    }
+
+    /**
+     * @param Token $token
+     * @return Diagnostic|null
+     */
+    private static function checkDiagnosticForUnexpectedToken($token) {
+        static $tokenKindToText;
+        if (!isset($tokenKindToText)) {
+            $tokenKindToText = \array_flip(\array_merge(
                 TokenStringMaps::OPERATORS_AND_PUNCTUATORS,
                 TokenStringMaps::KEYWORDS,
                 TokenStringMaps::RESERVED_WORDS
             ));
         }
-
-        if ($node instanceof SkippedToken) {
+        if ($token instanceof SkippedToken) {
             // TODO - consider also attaching parse context information to skipped tokens
             // this would allow us to provide more helpful error messages that inform users what to do
             // about the problem rather than simply pointing out the mistake.
             return new Diagnostic(
                 DiagnosticKind::Error,
                 "Unexpected '" .
-                (isset(self::$tokenKindToText[$node->kind])
-                    ? self::$tokenKindToText[$node->kind]
-                    : Token::getTokenKindNameFromValue($node->kind)) .
+                (isset($tokenKindToText[$token->kind])
+                    ? $tokenKindToText[$token->kind]
+                    : Token::getTokenKindNameFromValue($token->kind)) .
                 "'",
-                $node->start,
-                $node->getEndPosition() - $node->start
+                $token->start,
+                $token->getEndPosition() - $token->start
             );
-        } elseif ($node instanceof MissingToken) {
+        } elseif ($token instanceof MissingToken) {
             return new Diagnostic(
                 DiagnosticKind::Error,
                 "'" .
-                (isset(self::$tokenKindToText[$node->kind])
-                    ? self::$tokenKindToText[$node->kind]
-                    : Token::getTokenKindNameFromValue($node->kind)) .
+                (isset($tokenKindToText[$token->kind])
+                    ? $tokenKindToText[$token->kind]
+                    : Token::getTokenKindNameFromValue($token->kind)) .
                 "' expected.",
-                $node->start,
-                $node->getEndPosition() - $node->start
+                $token->start,
+                $token->getEndPosition() - $token->start
             );
         }
-
-        if ($node === null || $node instanceof Token) {
-            return null;
-        }
-
-        if ($node instanceof Node) {
-            if ($node instanceof Node\MethodDeclaration) {
-                foreach ($node->modifiers as $modifier) {
-                    if ($modifier->kind === TokenKind::VarKeyword) {
-                        return new Diagnostic(
-                            DiagnosticKind::Error,
-                            "Unexpected modifier '" . self::$tokenKindToText[$modifier->kind] . "'",
-                            $modifier->start,
-                            $modifier->length
-                        );
-                    }
-                }
-            }
-            elseif ($node instanceof Node\Statement\NamespaceUseDeclaration) {
-                if (
-                    $node->useClauses != null
-                    && \count($node->useClauses->children) > 1
-                ) {
-                    foreach ($node->useClauses->children as $useClause) {
-                        if($useClause instanceof Node\NamespaceUseClause && !is_null($useClause->openBrace)) {
-                            return new Diagnostic(
-                                DiagnosticKind::Error,
-                                "; expected.",
-                                $useClause->getEndPosition(),
-                                1
-                            );
-                        }
-                    }
-                }
-            }
-            else if ($node instanceof Node\Statement\BreakOrContinueStatement) {
-                if ($node->breakoutLevel === null) {
-                    return null;
-                }
-
-                $breakoutLevel = $node->breakoutLevel;
-                while ($breakoutLevel instanceof Node\Expression\ParenthesizedExpression) {
-                    $breakoutLevel = $breakoutLevel->expression;
-                }
-
-                if (
-                    $breakoutLevel instanceof Node\NumericLiteral
-                    && $breakoutLevel->children->kind === TokenKind::IntegerLiteralToken
-                ) {
-                    $literalString = $breakoutLevel->getText();
-                    $firstTwoChars = \substr($literalString, 0, 2);
-
-                    if ($firstTwoChars === '0b' || $firstTwoChars === '0B') {
-                        if (\bindec(\substr($literalString, 2)) > 0) {
-                            return null;
-                        }
-                    }
-                    else if (\intval($literalString, 0) > 0) {
-                        return null;
-                    }
-                }
-
-                if ($breakoutLevel instanceof Token) {
-                    $start = $breakoutLevel->getStartPosition();
-                }
-                else {
-                    $start = $breakoutLevel->getStart();
-                }
-                $end = $breakoutLevel->getEndPosition();
-
-                return new Diagnostic(
-                    DiagnosticKind::Error,
-                    "Positive integer literal expected.",
-                    $start,
-                    $end - $start
-                );
-            }
-        }
-        return null;
     }
 
     /**
@@ -141,11 +94,15 @@ public static function checkDiagnostics($node) {
     public static function getDiagnostics(Node $n) : array {
         $diagnostics = [];
 
-        foreach ($n->getDescendantNodesAndTokens() as $node) {
+        /**
+         * @param \Microsoft\PhpParser\Node|\Microsoft\PhpParser\Token $node
+         */
+        $n->walkDescendantNodesAndTokens(function($node) use (&$diagnostics) {
+            // echo "Processing " . get_class($node) . "\n";
             if (($diagnostic = self::checkDiagnostics($node)) !== null) {
                 $diagnostics[] = $diagnostic;
             }
-        }
+        });
 
         return $diagnostics;
     }

src/Node.php

@@ -158,14 +158,62 @@ public function getRoot() : Node {
     public function getDescendantNodesAndTokens(callable $shouldDescendIntoChildrenFn = null) {
         // TODO - write unit tests to prove invariants
         // (concatenating all descendant Tokens should produce document, concatenating all Nodes should produce document)
-        foreach ($this->getChildNodesAndTokens() as $child) {
-            if ($child instanceof Node) {
+        foreach (static::CHILD_NAMES as $name) {
+            $child = $this->$name;
+            // Check possible types of $child, most frequent first
+            if ($child instanceof Token) {
                 yield $child;
-                if ($shouldDescendIntoChildrenFn == null || $shouldDescendIntoChildrenFn($child)) {
-                    yield from $child->getDescendantNodesAndTokens($shouldDescendIntoChildrenFn);
-                }
-            } elseif ($child instanceof Token) {
+            } elseif ($child instanceof Node) {
                 yield $child;
+                if ($shouldDescendIntoChildrenFn === null || $shouldDescendIntoChildrenFn($child)) {
+                   yield from $child->getDescendantNodesAndTokens($shouldDescendIntoChildrenFn);
+                }
+            } elseif (\is_array($child)) {
+                foreach ($child as $childElement) {
+                    if ($childElement instanceof Token) {
+                        yield $childElement;
+                    } elseif ($childElement instanceof Node) {
+                        yield $childElement;
+                        if ($shouldDescendIntoChildrenFn === null || $shouldDescendIntoChildrenFn($childElement)) {
+                           yield from $childElement->getDescendantNodesAndTokens($shouldDescendIntoChildrenFn);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Iterate over all descendant Nodes and Tokens, calling $callback
+     *
+     * @param callable $callback a callback that accepts Node|Token
+     * @param callable|null $shouldDescendIntoChildrenFn
+     * @return void
+     */
+    public function walkDescendantNodesAndTokens(\Closure $callback, callable $shouldDescendIntoChildrenFn = null) {
+        // TODO - write unit tests to prove invariants
+        // (concatenating all descendant Tokens should produce document, concatenating all Nodes should produce document)
+        foreach (static::CHILD_NAMES as $name) {
+            $child = $this->$name;
+            // Check possible types of $child, most frequent first
+            if ($child instanceof Token) {
+                $callback($child);
+            } elseif ($child instanceof Node) {
+                $callback($child);
+                if ($shouldDescendIntoChildrenFn === null || $shouldDescendIntoChildrenFn($child)) {
+                   $child->walkDescendantNodesAndTokens($callback, $shouldDescendIntoChildrenFn);
+                }
+            } elseif (\is_array($child)) {
+                foreach ($child as $childElement) {
+                    if ($childElement instanceof Token) {
+                        $callback($childElement);
+                    } elseif ($childElement instanceof Node) {
+                        $callback($childElement);
+                        if ($shouldDescendIntoChildrenFn === null || $shouldDescendIntoChildrenFn($childElement)) {
+                           $childElement->walkDescendantNodesAndTokens($callback, $shouldDescendIntoChildrenFn);
+                        }
+                    }
+                }
             }
         }
     }
@@ -640,4 +688,13 @@ private function addToImportTable($alias, $functionOrConst, $namespaceNameParts,
         }
         return array($namespaceImportTable, $functionImportTable, $constImportTable);
     }
+
+    /**
+     * This is overridden in subclasses
+     * @return Diagnostic|null - Callers should use DiagnosticsProvider::getDiagnostics instead
+     * @internal
+     */
+    public function getDiagnosticForNode() {
+        return null;
+    }
 }

src/Node/MethodDeclaration.php

@@ -6,6 +6,9 @@
 
 namespace Microsoft\PhpParser\Node;
 
+use Microsoft\PhpParser\Diagnostic;
+use Microsoft\PhpParser\DiagnosticKind;
+use Microsoft\PhpParser\DiagnosticsProvider;
 use Microsoft\PhpParser\FunctionLike;
 use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Token;
@@ -52,4 +55,22 @@ public function isStatic() : bool {
     public function getName() {
         return $this->name->getText($this->getFileContents());
     }
+
+    /**
+     * @return Diagnostic|null - Callers should use DiagnosticsProvider::getDiagnostics instead
+     * @internal
+     * @override
+     */
+    public function getDiagnosticForNode() {
+        foreach ($this->modifiers as $modifier) {
+            if ($modifier->kind === TokenKind::VarKeyword) {
+                return new Diagnostic(
+                    DiagnosticKind::Error,
+                    "Unexpected modifier '" . DiagnosticsProvider::getTextForTokenKind($modifier->kind) . "'",
+                    $modifier->start,
+                    $modifier->length
+                );
+            }
+        }
+    }
 }