From 7f95b76cf86b8a12517b5908acfb6d816e71a45f Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 9 Oct 2016 14:40:15 +0200 Subject: [PATCH] Refactor DefinitionResolver Move logic to PhpDocument::getDefininedFqn() for reusability Fix DefinitionResolverTest --- src/NodeVisitors/DefinitionCollector.php | 50 +------------- src/PhpDocument.php | 69 ++++++++++++++++--- .../NodeVisitors/DefinitionCollectorTest.php | 9 ++- 3 files changed, 71 insertions(+), 57 deletions(-) diff --git a/src/NodeVisitors/DefinitionCollector.php b/src/NodeVisitors/DefinitionCollector.php index 8ad80fd..54ce0e5 100644 --- a/src/NodeVisitors/DefinitionCollector.php +++ b/src/NodeVisitors/DefinitionCollector.php @@ -13,14 +13,6 @@ class DefinitionCollector extends NodeVisitorAbstract { /** * Map from fully qualified name (FQN) to Node - * Examples of fully qualified names: - * - testFunction() - * - TestNamespace\TestClass - * - TestNamespace\TestClass::TEST_CONSTANT - * - TestNamespace\TestClass::staticTestProperty - * - TestNamespace\TestClass::testProperty - * - TestNamespace\TestClass::staticTestMethod() - * - TestNamespace\TestClass::testMethod() * * @var Node[] */ @@ -28,45 +20,9 @@ class DefinitionCollector extends NodeVisitorAbstract public function enterNode(Node $node) { - if ($node instanceof Node\Stmt\ClassLike && isset($node->name)) { - // Class, interface or trait declaration - $this->definitions[(string)$node->namespacedName] = $node; - } else if ($node instanceof Node\Stmt\Function_) { - // Function: use functioName() as the name - $name = (string)$node->namespacedName . '()'; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Stmt\ClassMethod) { - // Class method: use ClassName::methodName() as name - $class = $node->getAttribute('parentNode'); - if (!isset($class->name)) { - // Ignore anonymous classes - return; - } - $name = (string)$class->namespacedName . '::' . (string)$node->name . '()'; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Stmt\PropertyProperty) { - // Property: use ClassName::propertyName as name - $class = $node->getAttribute('parentNode')->getAttribute('parentNode'); - if (!isset($class->name)) { - // Ignore anonymous classes - return; - } - $name = (string)$class->namespacedName . '::' . (string)$node->name; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Const_) { - $parent = $node->getAttribute('parentNode'); - if ($parent instanceof Node\Stmt\Const_) { - // Basic constant: use CONSTANT_NAME as name - $name = (string)$node->namespacedName; - } else if ($parent instanceof Node\Stmt\ClassConst) { - // Class constant: use ClassName::CONSTANT_NAME as name - $class = $parent->getAttribute('parentNode'); - if (!isset($class->name) || $class->name instanceof Node\Expr) { - return; - } - $name = (string)$class->namespacedName . '::' . $node->name; - } - $this->definitions[$name] = $node; + $fqn = $node->getAttribute('ownerDocument')->getDefinedFqn($node); + if ($fqn !== null) { + $this->definitions[$fqn] = $node; } } } diff --git a/src/PhpDocument.php b/src/PhpDocument.php index 12bae1b..d4edb1d 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -57,14 +57,6 @@ class PhpDocument /** * Map from fully qualified name (FQN) to Node - * Examples of fully qualified names: - * - testFunction() - * - TestNamespace\TestClass - * - TestNamespace\TestClass::TEST_CONSTANT - * - TestNamespace\TestClass::staticTestProperty - * - TestNamespace\TestClass::testProperty - * - TestNamespace\TestClass::staticTestMethod() - * - TestNamespace\TestClass::testMethod() * * @var Node[] */ @@ -299,6 +291,67 @@ class PhpDocument return isset($this->definitions[$fqn]); } + /** + * Returns the fully qualified name (FQN) that is defined by a node + * Examples of FQNs: + * - testFunction() + * - TestNamespace\TestClass + * - TestNamespace\TestClass::TEST_CONSTANT + * - TestNamespace\TestClass::staticTestProperty + * - TestNamespace\TestClass::testProperty + * - TestNamespace\TestClass::staticTestMethod() + * - TestNamespace\TestClass::testMethod() + * + * @param Node $node + * @return string|null + */ + public function getDefinedFqn(Node $node) + { + if ($node instanceof Node\Name) { + $nameNode = $node; + $node = $node->getAttribute('parentNode'); + } + // Only the class node should count as the definition, not the name node + // Anonymous classes don't count as a definition + if ($node instanceof Node\Stmt\ClassLike && !isset($nameNode) && isset($node->name)) { + // Class, interface or trait declaration + return (string)$node->namespacedName; + } else if ($node instanceof Node\Stmt\Function_) { + // Function: use functionName() as the name + return (string)$node->namespacedName . '()'; + } else if ($node instanceof Node\Stmt\ClassMethod) { + // Class method: use ClassName::methodName() as name + $class = $node->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return null; + } + return (string)$class->namespacedName . '::' . (string)$node->name . '()'; + } else if ($node instanceof Node\Stmt\PropertyProperty) { + // Property: use ClassName::propertyName as name + $class = $node->getAttribute('parentNode')->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return null; + } + return (string)$class->namespacedName . '::' . (string)$node->name; + } else if ($node instanceof Node\Const_) { + $parent = $node->getAttribute('parentNode'); + if ($parent instanceof Node\Stmt\Const_) { + // Basic constant: use CONSTANT_NAME as name + return (string)$node->namespacedName; + } + if ($parent instanceof Node\Stmt\ClassConst) { + // Class constant: use ClassName::CONSTANT_NAME as name + $class = $parent->getAttribute('parentNode'); + if (!isset($class->name) || $class->name instanceof Node\Expr) { + return null; + } + return (string)$class->namespacedName . '::' . $node->name; + } + } + } + /** * Returns the FQN that is referenced by a node * diff --git a/tests/NodeVisitors/DefinitionCollectorTest.php b/tests/NodeVisitors/DefinitionCollectorTest.php index d22bd79..2f18bec 100644 --- a/tests/NodeVisitors/DefinitionCollectorTest.php +++ b/tests/NodeVisitors/DefinitionCollectorTest.php @@ -6,18 +6,23 @@ namespace LanguageServer\Tests\Server\TextDocument; use PHPUnit\Framework\TestCase; use PhpParser\{ParserFactory, NodeTraverser, Node}; use PhpParser\NodeVisitor\NameResolver; +use LanguageServer\{LanguageClient, Project, PhpDocument}; +use LanguageServer\Tests\MockProtocolStream; use LanguageServer\NodeVisitors\{ReferencesAdder, DefinitionCollector}; class DefinitionCollectorTest extends TestCase { public function test() { + $client = new LanguageClient(new MockProtocolStream()); + $project = new Project($client); + $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); + $document = new PhpDocument('whatever', $project, $client, $parser); $traverser = new NodeTraverser; $traverser->addVisitor(new NameResolver); - $traverser->addVisitor(new ReferencesAdder); + $traverser->addVisitor(new ReferencesAdder($document)); $definitionCollector = new DefinitionCollector; $traverser->addVisitor($definitionCollector); - $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); $stmts = $parser->parse(file_get_contents(__DIR__ . '/../../fixtures/symbols.php')); $traverser->traverse($stmts); $defs = $definitionCollector->definitions;