From 8ad59a914a8847616a6d25f2092defff3ce52397 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Tue, 11 Oct 2016 00:53:59 +0200 Subject: [PATCH] Change Project map to FQN => URI Removes PhpDocument::load(), isLoaded(), unload() --- src/LanguageServer.php | 2 +- src/PhpDocument.php | 69 +++--------------- src/Project.php | 71 ++++++++++++++++--- src/Server/TextDocument.php | 4 +- src/Server/Workspace.php | 7 +- tests/NodeVisitor/DefinitionCollectorTest.php | 11 +-- tests/PhpDocumentTest.php | 7 +- tests/ProjectTest.php | 11 +-- tests/Server/TextDocument/DefinitionTest.php | 6 +- tests/Server/TextDocument/DidChangeTest.php | 3 +- tests/Server/TextDocument/DidCloseTest.php | 6 +- .../TextDocument/DocumentSymbolTest.php | 2 +- tests/Server/Workspace/SymbolTest.php | 4 +- 13 files changed, 99 insertions(+), 104 deletions(-) diff --git a/src/LanguageServer.php b/src/LanguageServer.php index f23d271..c9fd73b 100644 --- a/src/LanguageServer.php +++ b/src/LanguageServer.php @@ -153,7 +153,7 @@ class LanguageServer extends \AdvancedJsonRpc\Dispatcher $shortName = substr($file, strlen($rootPath) + 1); $this->client->window->logMessage(MessageType::INFO, "Parsing file $fileNum/$numTotalFiles: $shortName."); - $this->project->getDocument($uri)->updateContent(file_get_contents($file), false); + $this->project->loadDocument($uri); Loop\setTimeout($processFile, 0); } else { diff --git a/src/PhpDocument.php b/src/PhpDocument.php index 4dd7929..342d959 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -77,45 +77,13 @@ class PhpDocument * @param LanguageClient $client The LanguageClient instance (to report errors etc) * @param Parser $parser The PHPParser instance */ - public function __construct(string $uri, Project $project, LanguageClient $client, Parser $parser) + public function __construct(string $uri, string $content, Project $project, LanguageClient $client, Parser $parser) { $this->uri = $uri; $this->project = $project; $this->client = $client; $this->parser = $parser; - } - - /** - * Returns true if the content of this document is being held in memory - * - * @return bool - */ - public function isLoaded() - { - return isset($this->content); - } - - /** - * Loads the content from disk and saves statements and definitions in memory - * - * @return void - */ - public function load() - { - $this->updateContent(file_get_contents(uriToPath($this->getUri())), true); - } - - /** - * Unloads the content, statements and definitions from memory - * - * @return void - */ - public function unload() - { - unset($this->content); - unset($this->statements); - unset($this->definitions); - unset($this->references); + $this->updateContent($content); } /** @@ -126,12 +94,9 @@ class PhpDocument * @param bool $keepInMemory Wether to keep content, statements and definitions in memory or only update project definitions * @return void */ - public function updateContent(string $content, bool $keepInMemory = true) + public function updateContent(string $content) { - $keepInMemory = $keepInMemory || $this->isLoaded(); - if ($keepInMemory) { - $this->content = $content; - } + $this->content = $content; $stmts = null; $errors = []; try { @@ -180,13 +145,11 @@ class PhpDocument // Register this document on the project for all the symbols defined in it foreach ($definitionCollector->definitions as $fqn => $node) { - $this->project->addDefinitionDocument($fqn, $this); + $this->project->addDefinitionDocument($fqn, $this->uri); } - if ($keepInMemory) { - $this->statements = $stmts; - $this->definitions = $definitionCollector->definitions; - } + $this->statements = $stmts; + $this->definitions = $definitionCollector->definitions; } } @@ -211,9 +174,6 @@ class PhpDocument */ public function getContent() { - if (!isset($this->content)) { - throw new Exception('Content is not loaded'); - } return $this->content; } @@ -224,9 +184,6 @@ class PhpDocument */ public function &getStatements() { - if (!isset($this->statements)) { - $this->parse($this->getContent()); - } return $this->statements; } @@ -251,7 +208,7 @@ class PhpDocument $traverser = new NodeTraverser; $finder = new NodeAtPositionFinder($position); $traverser->addVisitor($finder); - $traverser->traverse($this->getStatements()); + $traverser->traverse($this->statements); return $finder->node; } @@ -263,7 +220,7 @@ class PhpDocument */ public function getDefinitionByFqn(string $fqn) { - return $this->getDefinitions()[$fqn] ?? null; + return $this->definitions[$fqn] ?? null; } /** @@ -274,9 +231,6 @@ class PhpDocument */ public function &getDefinitions() { - if (!isset($this->definitions)) { - throw new Exception('Definitions of this document are not loaded'); - } return $this->definitions; } @@ -288,7 +242,7 @@ class PhpDocument */ public function isDefined(string $fqn): bool { - return isset($this->getDefinitions()[$fqn]); + return isset($this->definitions[$fqn]); } /** @@ -481,9 +435,6 @@ class PhpDocument return null; } $document = $this->project->getDefinitionDocument($fqn); - if (!$document->isLoaded()) { - $document->load(); - } if (!isset($document)) { return null; } diff --git a/src/Project.php b/src/Project.php index 41319ed..bed2676 100644 --- a/src/Project.php +++ b/src/Project.php @@ -18,10 +18,9 @@ class Project private $documents = []; /** - * An associative array [string => PhpDocument] - * that maps fully qualified symbol names to loaded PhpDocuments + * An associative array that maps fully qualified symbol names to document URIs * - * @var PhpDocument[] + * @var string[] */ private $definitions = []; @@ -48,7 +47,8 @@ class Project } /** - * Returns the document indicated by uri. Instantiates a new document if none exists. + * Returns the document indicated by uri. + * If the document is not open, tries to read it from disk, but the document is not added the list of open documents. * * @param string $uri * @return LanguageServer\PhpDocument @@ -56,9 +56,58 @@ class Project public function getDocument(string $uri) { if (!isset($this->documents[$uri])) { - $this->documents[$uri] = new PhpDocument($uri, $this, $this->client, $this->parser); + return $this->loadDocument($uri); + } else { + return $this->documents[$uri]; } - return $this->documents[$uri]; + } + + /** + * Reads a document from disk. + * The document is NOT added to the list of open documents, but definitions are registered. + * + * @param string $uri + * @return LanguageServer\PhpDocument + */ + public function loadDocument(string $uri) + { + $content = file_get_contents(uriToPath($uri)); + if (isset($this->documents[$uri])) { + $document = $this->documents[$uri]; + $document->updateContent($content); + } else { + $document = new PhpDocument($uri, $content, $this, $this->client, $this->parser); + } + return $document; + } + + /** + * Ensures a document is loaded and added to the list of open documents. + * + * @param string $uri + * @param string $content + * @return void + */ + public function openDocument(string $uri, string $content) + { + if (isset($this->documents[$uri])) { + $document = $this->documents[$uri]; + $document->updateContent($content); + } else { + $document = new PhpDocument($uri, $content, $this, $this->client, $this->parser); + $this->documents[$uri] = $document; + } + return $document; + } + + public function closeDocument(string $uri) + { + unset($this->documents[$uri]); + } + + public function isDocumentOpen(string $uri) + { + return isset($this->documents[$uri]); } /** @@ -67,9 +116,9 @@ class Project * @param string $fqn The fully qualified name of the symbol * @return void */ - public function addDefinitionDocument(string $fqn, PhpDocument $document) + public function addDefinitionDocument(string $fqn, string $uri) { - $this->definitions[$fqn] = $document; + $this->definitions[$fqn] = $uri; } /** @@ -80,12 +129,12 @@ class Project */ public function getDefinitionDocument(string $fqn) { - return $this->definitions[$fqn] ?? null; + return isset($this->definitions[$fqn]) ? $this->getDocument($this->definitions[$fqn]) : null; } /** - * Returns an associative array [string => PhpDocument] - * that maps fully qualified symbol names to loaded PhpDocuments + * Returns an associative array [string => string] that maps fully qualified symbol names + * to URIs of the document where the symbol is defined * * @return PhpDocument[] */ diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 9c8799a..69a4021 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -67,7 +67,7 @@ class TextDocument */ public function didOpen(TextDocumentItem $textDocument) { - $this->project->getDocument($textDocument->uri)->updateContent($textDocument->text); + $this->project->openDocument($textDocument->uri, $textDocument->text); } /** @@ -92,7 +92,7 @@ class TextDocument */ public function didClose(TextDocumentIdentifier $textDocument) { - $this->project->getDocument($textDocument->uri)->unload(); + $this->project->closeDocument($textDocument->uri); } /** diff --git a/src/Server/Workspace.php b/src/Server/Workspace.php index 5186008..dafa79e 100644 --- a/src/Server/Workspace.php +++ b/src/Server/Workspace.php @@ -54,12 +54,9 @@ class Workspace public function symbol(string $query): array { $symbols = []; - foreach ($this->project->getDefinitionDocuments() as $fqn => $document) { + foreach ($this->project->getDefinitionDocuments() as $fqn => $uri) { if ($query === '' || stripos($fqn, $query) !== false) { - if (!$document->isLoaded()) { - $document->load(); - } - $symbols[] = SymbolInformation::fromNode($document->getDefinitionByFqn($fqn), $fqn); + $symbols[] = SymbolInformation::fromNode($this->project->getDocument($uri)->getDefinitionByFqn($fqn), $fqn); } } return $symbols; diff --git a/tests/NodeVisitor/DefinitionCollectorTest.php b/tests/NodeVisitor/DefinitionCollectorTest.php index 554ba2e..10ec058 100644 --- a/tests/NodeVisitor/DefinitionCollectorTest.php +++ b/tests/NodeVisitor/DefinitionCollectorTest.php @@ -9,6 +9,7 @@ use PhpParser\NodeVisitor\NameResolver; use LanguageServer\{LanguageClient, Project, PhpDocument}; use LanguageServer\Tests\MockProtocolStream; use LanguageServer\NodeVisitor\{ReferencesAdder, DefinitionCollector}; +use function LanguageServer\pathToUri; class DefinitionCollectorTest extends TestCase { @@ -17,13 +18,14 @@ class DefinitionCollectorTest extends TestCase $client = new LanguageClient(new MockProtocolStream()); $project = new Project($client); $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); - $document = new PhpDocument('symbols', $project, $client, $parser); + $uri = pathToUri(realpath(__DIR__ . '/../../fixtures/symbols.php')); + $document = $project->loadDocument($uri); $traverser = new NodeTraverser; $traverser->addVisitor(new NameResolver); $traverser->addVisitor(new ReferencesAdder($document)); $definitionCollector = new DefinitionCollector; $traverser->addVisitor($definitionCollector); - $stmts = $parser->parse(file_get_contents(__DIR__ . '/../../fixtures/symbols.php')); + $stmts = $parser->parse(file_get_contents($uri)); $traverser->traverse($stmts); $defs = $definitionCollector->definitions; $this->assertEquals([ @@ -55,13 +57,14 @@ class DefinitionCollectorTest extends TestCase $client = new LanguageClient(new MockProtocolStream()); $project = new Project($client); $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); - $document = new PhpDocument('references', $project, $client, $parser); + $uri = pathToUri(realpath(__DIR__ . '/../../fixtures/references.php')); + $document = $project->loadDocument($uri); $traverser = new NodeTraverser; $traverser->addVisitor(new NameResolver); $traverser->addVisitor(new ReferencesAdder($document)); $definitionCollector = new DefinitionCollector; $traverser->addVisitor($definitionCollector); - $stmts = $parser->parse(file_get_contents(__DIR__ . '/../../fixtures/references.php')); + $stmts = $parser->parse(file_get_contents($uri)); $traverser->traverse($stmts); $defs = $definitionCollector->definitions; $this->assertEquals(['TestNamespace\\whatever()'], array_keys($defs)); diff --git a/tests/PhpDocumentTest.php b/tests/PhpDocumentTest.php index f6ab98a..c14a6b3 100644 --- a/tests/PhpDocumentTest.php +++ b/tests/PhpDocumentTest.php @@ -24,17 +24,14 @@ class PhpDocumentTest extends TestCase public function testParsesVariableVariables() { - $document = $this->project->getDocument('whatever'); - - $document->updateContent("project->openDocument('whatever', "assertEquals([], $document->getDefinitions()); } public function testGetNodeAtPosition() { - $document = $this->project->getDocument('whatever'); - $document->updateContent("project->openDocument('whatever', "getNodeAtPosition(new Position(1, 13)); $this->assertInstanceOf(Node\Name\FullyQualified::class, $node); $this->assertEquals('SomeClass', (string)$node); diff --git a/tests/ProjectTest.php b/tests/ProjectTest.php index 97c49f9..affbe2c 100644 --- a/tests/ProjectTest.php +++ b/tests/ProjectTest.php @@ -8,6 +8,7 @@ use LanguageServer\Tests\MockProtocolStream; use LanguageServer\{Server, Client, LanguageClient, Project, PhpDocument}; use LanguageServer\Protocol\{TextDocumentItem, TextDocumentIdentifier, SymbolKind, DiagnosticSeverity, FormattingOptions}; use AdvancedJsonRpc\{Request as RequestBody, Response as ResponseBody}; +use function LanguageServer\pathToUri; class ProjectTest extends TestCase { @@ -21,18 +22,18 @@ class ProjectTest extends TestCase $this->project = new Project(new LanguageClient(new MockProtocolStream())); } - public function testGetDocumentCreatesNewDocument() + public function testGetDocumentLoadsDocument() { - $document = $this->project->getDocument('file:///document1.php'); + $document = $this->project->getDocument(pathToUri(__FILE__)); $this->assertNotNull($document); $this->assertInstanceOf(PhpDocument::class, $document); } - public function testGetDocumentCreatesDocumentOnce() + public function testGetDocumentReturnsOpenedInstance() { - $document1 = $this->project->getDocument('file:///document1.php'); - $document2 = $this->project->getDocument('file:///document1.php'); + $document1 = $this->project->openDocument(pathToUri(__FILE__), file_get_contents(__FILE__)); + $document2 = $this->project->getDocument(pathToUri(__FILE__)); $this->assertSame($document1, $document2); } diff --git a/tests/Server/TextDocument/DefinitionTest.php b/tests/Server/TextDocument/DefinitionTest.php index 948fb35..7af6168 100644 --- a/tests/Server/TextDocument/DefinitionTest.php +++ b/tests/Server/TextDocument/DefinitionTest.php @@ -20,9 +20,9 @@ class DefinitionTest extends TestCase $client = new LanguageClient(new MockProtocolStream()); $project = new Project($client); $this->textDocument = new Server\TextDocument($project, $client); - $project->getDocument('references')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/references.php')); - $project->getDocument('symbols')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); - $project->getDocument('use')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/use.php')); + $project->openDocument('references', file_get_contents(__DIR__ . '/../../../fixtures/references.php')); + $project->openDocument('symbols', file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); + $project->openDocument('use', file_get_contents(__DIR__ . '/../../../fixtures/use.php')); } public function testDefinitionForClassLike() diff --git a/tests/Server/TextDocument/DidChangeTest.php b/tests/Server/TextDocument/DidChangeTest.php index 081978f..291e9d8 100644 --- a/tests/Server/TextDocument/DidChangeTest.php +++ b/tests/Server/TextDocument/DidChangeTest.php @@ -22,8 +22,7 @@ class DidChangeTest extends TestCase $client = new LanguageClient(new MockProtocolStream()); $project = new Project($client); $textDocument = new Server\TextDocument($project, $client); - $phpDocument = $project->getDocument('whatever'); - $phpDocument->updateContent("openDocument('whatever', "getDocument('whatever'); - $phpDocument->updateContent('hello world'); + $phpDocument = $project->openDocument('whatever', 'hello world'); $textDocumentItem = new TextDocumentItem(); $textDocumentItem->uri = 'whatever'; @@ -28,7 +27,6 @@ class DidCloseTest extends TestCase $textDocument->didClose(new TextDocumentIdentifier($textDocumentItem->uri)); - $this->expectException(Exception::class); - $phpDocument->getContent(); + $this->assertFalse($project->isDocumentOpen($textDocumentItem->uri)); } } diff --git a/tests/Server/TextDocument/DocumentSymbolTest.php b/tests/Server/TextDocument/DocumentSymbolTest.php index e5edfe7..601fbf9 100644 --- a/tests/Server/TextDocument/DocumentSymbolTest.php +++ b/tests/Server/TextDocument/DocumentSymbolTest.php @@ -20,7 +20,7 @@ class DocumentSymbolTest extends TestCase $client = new LanguageClient(new MockProtocolStream()); $project = new Project($client); $this->textDocument = new Server\TextDocument($project, $client); - $project->getDocument('symbols')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); + $project->openDocument('symbols', file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); } public function test() diff --git a/tests/Server/Workspace/SymbolTest.php b/tests/Server/Workspace/SymbolTest.php index f238207..fe90cf4 100644 --- a/tests/Server/Workspace/SymbolTest.php +++ b/tests/Server/Workspace/SymbolTest.php @@ -34,8 +34,8 @@ class SymbolTest extends TestCase $this->workspace = new Server\Workspace($project, $client); $this->symbolsUri = pathToUri(realpath(__DIR__ . '/../../../fixtures/symbols.php')); $this->referencesUri = pathToUri(realpath(__DIR__ . '/../../../fixtures/references.php')); - $project->getDocument($this->symbolsUri)->updateContent(file_get_contents($this->symbolsUri), false); - $project->getDocument($this->referencesUri)->updateContent(file_get_contents($this->referencesUri), false); + $project->loadDocument($this->symbolsUri); + $project->loadDocument($this->referencesUri); } public function testEmptyQueryReturnsAllSymbols()