From 81418e2ce05d23d246a0218c6bd98f38a3c0c4a2 Mon Sep 17 00:00:00 2001 From: Andrew Nagy Date: Wed, 2 Feb 2022 22:17:54 +0000 Subject: [PATCH] improve file caching logic with versions and refreshes --- src/Psalm/Codebase.php | 8 +- .../LanguageServer/LanguageServer.php | 80 ++++++------------- .../LanguageServer/Server/TextDocument.php | 71 ++++++++++------ src/Psalm/Internal/Provider/FileProvider.php | 33 ++++++-- 4 files changed, 98 insertions(+), 94 deletions(-) diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index fb1058047..e0682bf98 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -1366,12 +1366,12 @@ class Codebase $callables = InternalCallMapHandler::getCallablesFromCallMap($function_symbol); if (!$callables || !$callables[0]->params) { - return null; + throw $exception; } $params = $callables[0]->params; } else { - return null; + throw $exception; } } } @@ -1866,9 +1866,9 @@ class Codebase ); } - public function addTemporaryFileChanges(string $file_path, string $new_content): void + public function addTemporaryFileChanges(string $file_path, string $new_content, ?int $version = null): void { - $this->file_provider->addTemporaryFileChanges($file_path, $new_content); + $this->file_provider->addTemporaryFileChanges($file_path, $new_content, $version ); } public function removeTemporaryFileChanges(string $file_path): void diff --git a/src/Psalm/Internal/LanguageServer/LanguageServer.php b/src/Psalm/Internal/LanguageServer/LanguageServer.php index 78f124195..09ad5965f 100644 --- a/src/Psalm/Internal/LanguageServer/LanguageServer.php +++ b/src/Psalm/Internal/LanguageServer/LanguageServer.php @@ -110,21 +110,6 @@ class LanguageServer extends Dispatcher */ protected $project_analyzer; - /** - * @var array - */ - protected $onsave_paths_to_analyze = []; - - /** - * @var array - */ - protected $onchange_paths_to_analyze = []; - - /** - * @var array> - */ - protected $current_issues = []; - public function __construct( ProtocolReader $reader, ProtocolWriter $writer, @@ -365,42 +350,40 @@ class LanguageServer extends Dispatcher try { $this->client->refreshConfiguration(); } catch(Throwable $e) { - error_log((string) $e); + $this->server->logError((string) $e); } $this->clientStatus('running'); } - public function queueTemporaryFileAnalysis(string $file_path, string $uri, ?int $version=null): void - { - $this->logDebug("queueTemporaryFileAnalysis", ['version' => $version, 'file_path' => $file_path, 'uri' => $uri]); - $this->onchange_paths_to_analyze[$version][$file_path] = $uri; - $this->debounceVersionedAnalysis($version); + public function queueChangeFileAnalysis(string $file_path, string $uri, ?int $version=null) { + $this->doVersionedAnalysis([$file_path => $uri], $version); } - public function queueFileAnalysis(string $file_path, string $uri, ?int $version=null): void - { - //$this->logDebug("queueFileAnalysis", ['version' => $version, 'file_path' => $file_path, 'uri' => $uri]); - $this->onsave_paths_to_analyze[$version][$file_path] = $uri; - $this->debounceVersionedAnalysis($version); + public function queueOpenFileAnalysis(string $file_path, string $uri, ?int $version=null) { + $this->doVersionedAnalysis([$file_path => $uri], $version); } - public function debounceVersionedAnalysis(?int $version=null) { - //$this->logDebug("debounceVersionedAnalysis", ['version' => $version]); - $onchange_paths_to_analyze = $this->onchange_paths_to_analyze[$version] ?? []; - $onsave_paths_to_analyze = $this->onsave_paths_to_analyze[$version] ?? []; - $all_files_to_analyze = $onchange_paths_to_analyze + $onsave_paths_to_analyze; + /** + * Queue Saved File Analysis + * @param string $file_path + * @param string $uri + * @return void + */ + public function queueSaveFileAnalysis(string $file_path, string $uri) { + //Always reanalzye open files because of things changing elsewhere + $opened = array_reduce( + $this->project_analyzer->getCodebase()->file_provider->getOpenFilesPath(), + function (array $opened, string $file_path) { + $opened[$file_path] = $this->pathToUri($file_path); + return $opened; + }, + [$file_path => $this->pathToUri($file_path)]); - $this->doVersionedAnalysis('', [$all_files_to_analyze, $version]); + $this->doVersionedAnalysis($opened); } - public function doVersionedAnalysis(string $watcherId, $data = []):void { - //$this->logDebug("doVersionedAnalysis"); - - [$all_files_to_analyze, $version] = $data; - + public function doVersionedAnalysis($all_files_to_analyze, ?int $version=null):void { try { - - if(empty($all_files_to_analyze)) { $this->logWarning("No versioned analysis to do."); return; @@ -423,12 +406,8 @@ class LanguageServer extends Dispatcher $this->emitVersionedIssues($files,$version); } catch(Throwable $e) { - error_log((string) $e); - } finally { - unset($this->onchange_paths_to_analyze[$version]); - unset($this->onsave_paths_to_analyze[$version]); + $this->server->logError((string) $e); } - } public function emitVersionedIssues(array $files, ?int $version = null): void { @@ -436,9 +415,8 @@ class LanguageServer extends Dispatcher 'files' => array_keys($files), 'version' => $version ]); - $data = IssueBuffer::clear(); - $this->current_issues = $data; + $data = IssueBuffer::clear(); foreach ($files as $file_path => $uri) { //Dont report errors in files we are not watching if (!$this->project_analyzer->getCodebase()->config->isInProjectDirs($file_path)) { @@ -664,14 +642,4 @@ class LanguageServer extends Dispatcher return $filepath; } - - /** - * Get the value of current_issues - * - * @return array> - */ - public function getCurrentIssues(): array - { - return $this->current_issues; - } } diff --git a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php index 4ac708714..d09a6a602 100644 --- a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php +++ b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php @@ -88,23 +88,25 @@ class TextDocument $file_path = LanguageServer::uriToPath($textDocument->uri); + $this->codebase->removeTemporaryFileChanges($file_path); $this->codebase->file_provider->openFile($file_path); + $this->codebase->file_provider->setOpenContents($file_path, $textDocument->text); - $this->server->queueFileAnalysis($file_path, $textDocument->uri, $textDocument->version); + $this->server->queueOpenFileAnalysis($file_path, $textDocument->uri, $textDocument->version); } /** * The document save notification is sent from the client to the server when the document was saved in the client * - * @param TextDocumentItem $textDocument the document that was opened + * @param TextDocumentIdentifier $textDocument the document that was opened * @param string|null $text Optional the content when saved. Depends on the includeText value * when the save notification was requested. */ - public function didSave(TextDocumentItem $textDocument, ?string $text = null): void + public function didSave(TextDocumentIdentifier $textDocument, ?string $text = null): void { $this->server->logDebug( 'textDocument/didSave', - ['version' => $textDocument->version, 'uri' => $textDocument->uri] + ['uri' => (array) $textDocument] ); $file_path = LanguageServer::uriToPath($textDocument->uri); @@ -113,7 +115,7 @@ class TextDocument $this->codebase->removeTemporaryFileChanges($file_path); $this->codebase->file_provider->setOpenContents($file_path, $text); - $this->server->queueFileAnalysis($file_path, $textDocument->uri); + $this->server->queueSaveFileAnalysis($file_path, $textDocument->uri); } /** @@ -147,8 +149,8 @@ class TextDocument } } - $this->codebase->addTemporaryFileChanges($file_path, $new_content); - $this->server->queueTemporaryFileAnalysis($file_path, $textDocument->uri, $textDocument->version); + $this->codebase->addTemporaryFileChanges($file_path, $new_content, $textDocument->version); + $this->server->queueChangeFileAnalysis($file_path, $textDocument->uri, $textDocument->version); } /** @@ -191,12 +193,15 @@ class TextDocument $file_path = LanguageServer::uriToPath($textDocument->uri); + //This currently doesnt work right with out of project files + if (!$this->codebase->config->isInProjectDirs($file_path)) { + return new Success(null); + } + try { $reference_location = $this->codebase->getReferenceAtPosition($file_path, $position); } catch (UnanalyzedFileException $e) { - $this->codebase->file_provider->openFile($file_path); - $this->server->queueFileAnalysis($file_path, $textDocument->uri); - + $this->server->logError((string) $e); return new Success(null); } @@ -239,12 +244,15 @@ class TextDocument $file_path = LanguageServer::uriToPath($textDocument->uri); + //This currently doesnt work right with out of project files + if (!$this->codebase->config->isInProjectDirs($file_path)) { + return new Success(null); + } + try { $reference_location = $this->codebase->getReferenceAtPosition($file_path, $position); } catch (UnanalyzedFileException $e) { - $this->codebase->file_provider->openFile($file_path); - $this->server->queueFileAnalysis($file_path, $textDocument->uri); - + $this->server->logError((string) $e); return new Success(null); } @@ -257,7 +265,7 @@ class TextDocument try { $symbol_information = $this->codebase->getSymbolInformation($file_path, $reference); } catch(UnexpectedValueException $e) { - error_log((string) $e); + $this->server->logError((string) $e); return new Success(null); } @@ -299,24 +307,27 @@ class TextDocument $file_path = LanguageServer::uriToPath($textDocument->uri); + //This currently doesnt work right with out of project files + if (!$this->codebase->config->isInProjectDirs($file_path)) { + return new Success(null); + } + try { $completion_data = $this->codebase->getCompletionDataAtPosition($file_path, $position); } catch (UnanalyzedFileException $e) { - $this->codebase->file_provider->openFile($file_path); - $this->server->queueFileAnalysis($file_path, $textDocument->uri); - + $this->server->logError((string) $e); return new Success(null); } try { $type_context = $this->codebase->getTypeContextAtPosition($file_path, $position); } catch (UnexpectedValueException $e) { - error_log((string) $e); + $this->server->logError((string) $e); return new Success(null); } if (!$completion_data && !$type_context) { - error_log('completion not found at ' . $position->line . ':' . $position->character); + $this->server->logError('completion not found at ' . $position->line . ':' . $position->character); return new Success(null); } @@ -353,23 +364,31 @@ class TextDocument $file_path = LanguageServer::uriToPath($textDocument->uri); + //This currently doesnt work right with out of project files + if (!$this->codebase->config->isInProjectDirs($file_path)) { + return new Success(null); + } + try { $argument_location = $this->codebase->getFunctionArgumentAtPosition($file_path, $position); } catch (UnanalyzedFileException $e) { - $this->codebase->file_provider->openFile($file_path); - $this->server->queueFileAnalysis($file_path, $textDocument->uri); - - return new Success(new SignatureHelp()); + $this->server->logError((string) $e); + return new Success(null); } if ($argument_location === null) { - return new Success(new SignatureHelp()); + return new Success(null); } - $signature_information = $this->codebase->getSignatureInformation($argument_location[0], $file_path); + try { + $signature_information = $this->codebase->getSignatureInformation($argument_location[0], $file_path); + } catch(UnexpectedValueException $e) { + $this->server->logError((string) $e); + return new Success(null); + } if (!$signature_information) { - return new Success(new SignatureHelp()); + return new Success(null); } return new Success( diff --git a/src/Psalm/Internal/Provider/FileProvider.php b/src/Psalm/Internal/Provider/FileProvider.php index 02674c519..ed602ff97 100644 --- a/src/Psalm/Internal/Provider/FileProvider.php +++ b/src/Psalm/Internal/Provider/FileProvider.php @@ -31,11 +31,16 @@ class FileProvider */ protected $open_files = []; + /** + * @var array + */ + protected $open_files_paths = []; + public function getContents(string $file_path, bool $go_to_source = false): string { $file_path_lc = strtolower($file_path); if (!$go_to_source && isset($this->temp_files[$file_path_lc])) { - return $this->temp_files[$file_path_lc]; + return $this->temp_files[$file_path_lc]['content']; } if (isset($this->open_files[$file_path_lc])) { @@ -67,11 +72,11 @@ class FileProvider file_put_contents($file_path, $file_contents); } - public function setOpenContents(string $file_path, ?string $file_contents=null): void + public function setOpenContents(string $file_path, string $file_contents): void { $file_path_lc = strtolower($file_path); if (isset($this->open_files[$file_path_lc])) { - $this->open_files[$file_path_lc] = $file_contents ?? $this->getContents($file_path, true); + $this->open_files[$file_path_lc] = $file_contents; } } @@ -84,9 +89,20 @@ class FileProvider return (int)filemtime($file_path); } - public function addTemporaryFileChanges(string $file_path, string $new_content): void + public function addTemporaryFileChanges(string $file_path, string $new_content, ?int $version = null): void { - $this->temp_files[strtolower($file_path)] = $new_content; + if( + isset($this->temp_files[strtolower($file_path)]) && + !is_null($version) && + !is_null($this->temp_files[strtolower($file_path)]['version']) && + $version < $this->temp_files[strtolower($file_path)]['version'] + ) { + return; + } + $this->temp_files[strtolower($file_path)] = [ + 'version' => $version, + 'content' => $new_content, + ]; } public function removeTemporaryFileChanges(string $file_path): void @@ -94,14 +110,15 @@ class FileProvider unset($this->temp_files[strtolower($file_path)]); } - public function getOpenFiles(): array + public function getOpenFilesPath(): array { - return array_keys($this->open_files); + return $this->open_files_paths; } public function openFile(string $file_path): void { $this->open_files[strtolower($file_path)] = $this->getContents($file_path, true); + $this->open_files_paths[strtolower($file_path)] = $file_path; } public function isOpen(string $file_path): bool @@ -113,7 +130,7 @@ class FileProvider public function closeFile(string $file_path): void { $file_path_lc = strtolower($file_path); - unset($this->temp_files[$file_path_lc], $this->open_files[$file_path_lc]); + unset($this->temp_files[$file_path_lc], $this->open_files[$file_path_lc], $this->open_files_paths[$file_path_lc]); } public function fileExists(string $file_path): bool