From cdf8fc36e1fffcdd4f510288d50c37ed1bcde9dc Mon Sep 17 00:00:00 2001
From: Sara Itani <sitani@microsoft.com>
Date: Sun, 5 Mar 2017 16:34:01 -0800
Subject: [PATCH] add getNodeAtPosition, fix definition collector tests

---
 src/ParserResourceFactory.php                 | 13 ++-
 src/PhpDocument.php                           | 24 +++---
 tests/NodeVisitor/DefinitionCollectorTest.php | 85 ++++++++-----------
 tests/PhpDocumentTest.php                     | 10 ++-
 4 files changed, 67 insertions(+), 65 deletions(-)

diff --git a/src/ParserResourceFactory.php b/src/ParserResourceFactory.php
index 8aac4a4..05cb146 100644
--- a/src/ParserResourceFactory.php
+++ b/src/ParserResourceFactory.php
@@ -8,7 +8,7 @@ use LanguageServer\Index\ReadableIndex;
 class ParserResourceFactory {
     const PARSER_KIND = ParserKind::TOLERANT_PHP_PARSER;
     
-    public function getParser() {
+    public static function getParser() {
         if (self::PARSER_KIND === ParserKind::PHP_PARSER) {
             return new Parser;
         } else {
@@ -16,11 +16,20 @@ class ParserResourceFactory {
         }
     }
 
-    public function getDefinitionResolver(ReadableIndex $index) {
+    public static function getDefinitionResolver(ReadableIndex $index) {
         if (self::PARSER_KIND === ParserKind::PHP_PARSER) {
             return new DefinitionResolver($index);
         } else {
             return new TolerantDefinitionResolver($index);
         }
     }
+
+    public static function getTreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri)
+    {
+        if (self::PARSER_KIND === ParserKind::PHP_PARSER) {
+            return new TreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri);
+        } else {
+            return new TolerantTreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri);
+        }
+    }
 }
diff --git a/src/PhpDocument.php b/src/PhpDocument.php
index d653341..534f88f 100644
--- a/src/PhpDocument.php
+++ b/src/PhpDocument.php
@@ -64,7 +64,7 @@ class PhpDocument
     /**
      * The AST of the document
      *
-     * @var Node[]
+     * @var Node[] | Tolerant\Node
      */
     private $stmts;
 
@@ -161,11 +161,7 @@ class PhpDocument
         $this->definitions = null;
         $this->definitionNodes = null;
 
-        $treeAnalyzerClass = $this->parser instanceof Parser 
-            ? TreeAnalyzer::class
-            : TolerantTreeAnalyzer::class;
-            
-        $treeAnalyzer = new $treeAnalyzerClass($this->parser, $content, $this->docBlockFactory, $this->definitionResolver, $this->uri);
+        $treeAnalyzer = ParserResourceFactory::getTreeAnalyzer($this->parser, $content, $this->docBlockFactory, $this->definitionResolver, $this->uri);
 
         $this->diagnostics = $treeAnalyzer->getDiagnostics();
 
@@ -251,11 +247,17 @@ class PhpDocument
         if ($this->stmts === null) {
             return null;
         }
-        $traverser = new NodeTraverser;
-        $finder = new NodeAtPositionFinder($position);
-        $traverser->addVisitor($finder);
-        $traverser->traverse($this->stmts);
-        return $finder->node;
+
+        if (\is_array($this->stmts)) {
+            $traverser = new NodeTraverser;
+            $finder = new NodeAtPositionFinder($position);
+            $traverser->addVisitor($finder);
+            $traverser->traverse($this->stmts);
+            return $finder->node;
+        } else {
+            $offset = $position->toOffset($this->stmts->getFileContents());
+            return $this->stmts->getDescendantNodeAtPosition($offset);
+        }
     }
 
     /**
diff --git a/tests/NodeVisitor/DefinitionCollectorTest.php b/tests/NodeVisitor/DefinitionCollectorTest.php
index 16c4935..d53f43b 100644
--- a/tests/NodeVisitor/DefinitionCollectorTest.php
+++ b/tests/NodeVisitor/DefinitionCollectorTest.php
@@ -4,17 +4,12 @@ declare(strict_types = 1);
 namespace LanguageServer\Tests\Server\TextDocument;
 
 use PHPUnit\Framework\TestCase;
-use PhpParser\{NodeTraverser, Node};
-use PhpParser\NodeVisitor\NameResolver;
+use PhpParser\{Node};
 use phpDocumentor\Reflection\DocBlockFactory;
 use LanguageServer\{
-    ParserResourceFactory, LanguageClient, PhpDocument, PhpDocumentLoader, Parser, DefinitionResolver
+    ParserResourceFactory
 };
-use LanguageServer\ContentRetriever\FileSystemContentRetriever;
-use LanguageServer\Protocol\ClientCapabilities;
-use LanguageServer\Index\{ProjectIndex, Index, DependenciesIndex};
-use LanguageServer\Tests\MockProtocolStream;
-use LanguageServer\NodeVisitor\{ReferencesAdder, DefinitionCollector};
+use LanguageServer\Index\{Index};
 use function LanguageServer\pathToUri;
 use Microsoft\PhpParser as Tolerant;
 
@@ -23,23 +18,7 @@ class DefinitionCollectorTest extends TestCase
     public function testCollectsSymbols()
     {
         $path = realpath(__DIR__ . '/../../fixtures/symbols.php');
-        $uri = pathToUri($path);
-        $parser = ParserResourceFactory::getParser();
-        $docBlockFactory = DocBlockFactory::createInstance();
-        $index = new Index;
-        $definitionResolver = ParserResourceFactory::getDefinitionResolver($index);
-        $content = file_get_contents($path);
-        $document = new PhpDocument($uri, $content, $index, $parser, $docBlockFactory, $definitionResolver);
-        $stmts = $parser->parse($content);
-
-        $traverser = new NodeTraverser;
-        $traverser->addVisitor(new NameResolver);
-        $traverser->addVisitor(new ReferencesAdder($document));
-        $definitionCollector = new DefinitionCollector($definitionResolver);
-        $traverser->addVisitor($definitionCollector);
-        $traverser->traverse($stmts);
-
-        $defNodes = $definitionCollector->nodes;
+        $defNodes = $this->collectDefinitions($path);
 
         $this->assertEquals([
             'TestNamespace',
@@ -55,43 +34,47 @@ class DefinitionCollectorTest extends TestCase
             'TestNamespace\\test_function()',
             'TestNamespace\\ChildClass'
         ], array_keys($defNodes));
-        $this->assertInstanceOf(Node\Const_::class, $defNodes['TestNamespace\\TEST_CONST']);
-        $this->assertInstanceOf(Node\Stmt\Class_::class, $defNodes['TestNamespace\\TestClass']);
-        $this->assertInstanceOf(Node\Const_::class, $defNodes['TestNamespace\\TestClass::TEST_CLASS_CONST']);
-        $this->assertInstanceOf(Node\Stmt\PropertyProperty::class, $defNodes['TestNamespace\\TestClass::$staticTestProperty']);
-        $this->assertInstanceOf(Node\Stmt\PropertyProperty::class, $defNodes['TestNamespace\\TestClass->testProperty']);
-        $this->assertInstanceOf(Node\Stmt\ClassMethod::class, $defNodes['TestNamespace\\TestClass::staticTestMethod()']);
-        $this->assertInstanceOf(Node\Stmt\ClassMethod::class, $defNodes['TestNamespace\\TestClass->testMethod()']);
-        $this->assertInstanceOf(Node\Stmt\Trait_::class, $defNodes['TestNamespace\\TestTrait']);
-        $this->assertInstanceOf(Node\Stmt\Interface_::class, $defNodes['TestNamespace\\TestInterface']);
-        $this->assertInstanceOf(Node\Stmt\Function_::class, $defNodes['TestNamespace\\test_function()']);
-        $this->assertInstanceOf(Node\Stmt\Class_::class, $defNodes['TestNamespace\\ChildClass']);
+
+
+        $this->assertInstanceOf(Tolerant\Node\ConstElement::class, $defNodes['TestNamespace\\TEST_CONST']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\ClassDeclaration::class, $defNodes['TestNamespace\\TestClass']);
+        $this->assertInstanceOf(Tolerant\Node\ConstElement::class, $defNodes['TestNamespace\\TestClass::TEST_CLASS_CONST']);
+        // TODO - should we parse properties more strictly?
+        $this->assertInstanceOf(Tolerant\Node\Expression\Variable::class, $defNodes['TestNamespace\\TestClass::$staticTestProperty']);
+        $this->assertInstanceOf(Tolerant\Node\Expression\Variable::class, $defNodes['TestNamespace\\TestClass->testProperty']);
+        $this->assertInstanceOf(Tolerant\Node\MethodDeclaration::class, $defNodes['TestNamespace\\TestClass::staticTestMethod()']);
+        $this->assertInstanceOf(Tolerant\Node\MethodDeclaration::class, $defNodes['TestNamespace\\TestClass->testMethod()']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\TraitDeclaration::class, $defNodes['TestNamespace\\TestTrait']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\InterfaceDeclaration::class, $defNodes['TestNamespace\\TestInterface']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\FunctionDeclaration::class, $defNodes['TestNamespace\\test_function()']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\ClassDeclaration::class, $defNodes['TestNamespace\\ChildClass']);
     }
 
     public function testDoesNotCollectReferences()
     {
         $path = realpath(__DIR__ . '/../../fixtures/references.php');
+        $defNodes = $this->collectDefinitions($path);
+
+        $this->assertEquals(['TestNamespace', 'TestNamespace\\whatever()'], array_keys($defNodes));
+        $this->assertInstanceOf(Tolerant\Node\Statement\NamespaceDefinition::class, $defNodes['TestNamespace']);
+        $this->assertInstanceOf(Tolerant\Node\Statement\FunctionDeclaration::class, $defNodes['TestNamespace\\whatever()']);
+    }
+
+    /**
+     * @param $path
+     * @return Node
+     */
+    private function collectDefinitions($path):array
+    {
         $uri = pathToUri($path);
         $parser = ParserResourceFactory::getParser();
+
         $docBlockFactory = DocBlockFactory::createInstance();
         $index = new Index;
         $definitionResolver = ParserResourceFactory::getDefinitionResolver($index);
         $content = file_get_contents($path);
-        $document = new PhpDocument($uri, $content, $index, $parser, $docBlockFactory, $definitionResolver);
-        $stmts = $parser->parse($content);
 
-        $traverser = new NodeTraverser;
-        $traverser->addVisitor(new NameResolver);
-        $traverser->addVisitor(new ReferencesAdder($document));
-        $definitionCollector = new DefinitionCollector($definitionResolver);
-        $traverser->addVisitor($definitionCollector);
-        $traverser->traverse($stmts);
-
-        $defNodes = $definitionCollector->nodes;
-
-        $this->assertEquals(['TestNamespace', 'TestNamespace\\whatever()'], array_keys($defNodes));
-        $this->assertInstanceOf(Node\Name::class, $defNodes['TestNamespace']);
-        $this->assertInstanceOf(Node\Stmt\Namespace_::class, $defNodes['TestNamespace']->getAttribute('parentNode'));
-        $this->assertInstanceOf(Node\Stmt\Function_::class, $defNodes['TestNamespace\\whatever()']);
+        $treeAnalyzer = ParserResourceFactory::getTreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri);
+        return $treeAnalyzer->getDefinitionNodes();
     }
 }
diff --git a/tests/PhpDocumentTest.php b/tests/PhpDocumentTest.php
index 94399f0..0dc9a6d 100644
--- a/tests/PhpDocumentTest.php
+++ b/tests/PhpDocumentTest.php
@@ -39,10 +39,18 @@ class PhpDocumentTest extends TestCase
     {
         $document = $this->createDocument('whatever', "<?php\n$\$a = new SomeClass;");
         $node = $document->getNodeAtPosition(new Position(1, 13));
-        $this->assertInstanceOf(Node\Name\FullyQualified::class, $node);
+        $this->assertQualifiedName($node);
         $this->assertEquals('SomeClass', (string)$node);
     }
 
+    private function assertQualifiedName($node) {
+        if ($node instanceof Node) {
+            $this->assertInstanceOf(Node\Name\FullyQualified::class, $node);
+        } else {
+            $this->assertInstanceOf(Tolerant\Node\QualifiedName::class, $node);
+        }
+    }
+
     public function testIsVendored()
     {
         $document = $this->createDocument('file:///dir/vendor/x.php', "<?php\n$\$a = new SomeClass;");