Switch to using document URI to store managed files (#288)

* Switch to using document URI to store managed files

We also have to normalise document URI to lowercase on windows/mac to deal with case insensitive file systems

We leave "getTextDocumentFromModuleName" for simplicity

* Fix tests

* Update changelog

Author: JohnnyMorganz

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Fixed
+
+- Changed internal representation of documents to reduce the likelihood of Request Failed for "No managed text document"
+
 ## [1.16.2] - 2023-02-01
 
 ### Fixed

src/DocumentationParser.cpp

@@ -380,7 +380,7 @@ std::vector<std::string> WorkspaceFolder::getComments(const Luau::ModuleName& mo
         return {};
 
     // Get relevant text document
-    auto textDocument = fileResolver.getTextDocument(moduleName);
+    auto textDocument = fileResolver.getTextDocumentFromModuleName(moduleName);
     bool tempDocument = false;
     if (!textDocument)
     {

src/LanguageServer.cpp

@@ -476,16 +476,8 @@ void LanguageServer::recomputeDiagnostics(WorkspaceFolderPtr& workspace, const C
         // Recompute diagnostics for all currently opened files
         else
         {
-            for (const auto& [file, document] : workspace->fileResolver.managedFiles)
-            {
-                auto filePath = workspace->fileResolver.resolveToRealPath(file);
-                if (filePath)
-                {
-
-                    auto uri = Uri::file(*filePath);
-                    this->pushDiagnostics(workspace, uri, document.version());
-                }
-            }
+            for (const auto& [_, document] : workspace->fileResolver.managedFiles)
+                this->pushDiagnostics(workspace, document.uri(), document.version());
         }
     }
     else

src/Workspace.cpp

@@ -9,54 +9,41 @@
 
 void WorkspaceFolder::openTextDocument(const lsp::DocumentUri& uri, const lsp::DidOpenTextDocumentParams& params)
 {
-    auto moduleName = fileResolver.getModuleName(uri);
+    auto normalisedUri = fileResolver.normalisedUriString(uri);
+
     fileResolver.managedFiles.emplace(
-        std::make_pair(moduleName, TextDocument(uri, params.textDocument.languageId, params.textDocument.version, params.textDocument.text)));
+        std::make_pair(normalisedUri, TextDocument(uri, params.textDocument.languageId, params.textDocument.version, params.textDocument.text)));
+
     // Mark the file as dirty as we don't know what changes were made to it
+    auto moduleName = fileResolver.getModuleName(uri);
     frontend.markDirty(moduleName);
 }
 
 void WorkspaceFolder::updateTextDocument(
     const lsp::DocumentUri& uri, const lsp::DidChangeTextDocumentParams& params, std::vector<Luau::ModuleName>* markedDirty)
 {
-    auto moduleName = fileResolver.getModuleName(uri);
+    auto normalisedUri = fileResolver.normalisedUriString(uri);
 
-    if (!contains(fileResolver.managedFiles, moduleName))
+    if (!contains(fileResolver.managedFiles, normalisedUri))
     {
-        // Check if we have the original file URI stored (https://github.com/JohnnyMorganz/luau-lsp/issues/26)
-        // TODO: can be potentially removed when server generates sourcemap
-        auto fsPath = uri.fsPath().generic_string();
-        if (fsPath != moduleName && contains(fileResolver.managedFiles, fsPath))
-        {
-            // Change the managed file key to use the new modulename
-            auto nh = fileResolver.managedFiles.extract(fsPath);
-            nh.key() = moduleName;
-            fileResolver.managedFiles.insert(std::move(nh));
-        }
-        else
-        {
-            client->sendLogMessage(lsp::MessageType::Error, "Text Document not loaded locally: " + uri.toString());
-            return;
-        }
+        client->sendLogMessage(lsp::MessageType::Error, "Text Document not loaded locally: " + uri.toString());
+        return;
     }
-    auto& textDocument = fileResolver.managedFiles.at(moduleName);
+    auto& textDocument = fileResolver.managedFiles.at(normalisedUri);
     textDocument.update(params.contentChanges, params.textDocument.version);
 
     // Mark the module dirty for the typechecker
+    auto moduleName = fileResolver.getModuleName(uri);
     frontend.markDirty(moduleName, markedDirty);
 }
 
 void WorkspaceFolder::closeTextDocument(const lsp::DocumentUri& uri)
 {
-    auto config = client->getConfiguration(rootUri);
-    auto moduleName = fileResolver.getModuleName(uri);
-    fileResolver.managedFiles.erase(moduleName);
-
-    // Clear out base uri fsPath as well, in case we managed it like that
-    // TODO: can be potentially removed when server generates sourcemap
-    fileResolver.managedFiles.erase(uri.fsPath().generic_string());
+    fileResolver.managedFiles.erase(fileResolver.normalisedUriString(uri));
 
     // Mark the module as dirty as we no longer track its changes
+    auto config = client->getConfiguration(rootUri);
+    auto moduleName = fileResolver.getModuleName(uri);
     frontend.markDirty(moduleName);
 
     // Refresh workspace diagnostics to clear diagnostics on ignored files
@@ -137,29 +124,6 @@ bool WorkspaceFolder::updateSourceMap()
             frontend.typeChecker, instanceTypes, fileResolver, /* TODO - expressiveTypes: */ config.diagnostics.strictDatamodelTypes);
         types::registerInstanceTypes(frontend.typeCheckerForAutocomplete, instanceTypes, fileResolver, /* TODO - expressiveTypes: */ true);
 
-        // Update managed file paths as they may be converted to virtual
-        // Check if we have the original file URIs stored (https://github.com/JohnnyMorganz/luau-lsp/issues/26)
-        std::vector<std::pair<Luau::ModuleName, TextDocument>> movedFiles;
-        for (auto it = fileResolver.managedFiles.begin(); it != fileResolver.managedFiles.end();)
-        {
-            if (!fileResolver.isVirtualPath(it->first))
-            {
-                if (auto virtualPath = fileResolver.resolveToVirtualPath(it->first); virtualPath && virtualPath != it->first)
-                {
-                    // Store the new ModuleName pairing into a vector and remove the old key
-                    movedFiles.emplace_back(std::make_pair(*virtualPath, it->second));
-                    it = fileResolver.managedFiles.erase(it);
-                    continue; // Ensure we continue so we don't increment iterator and skip next element
-                }
-            }
-
-            it++;
-        }
-
-        // Add any new pairings back into the map
-        for (auto& pair : movedFiles)
-            fileResolver.managedFiles.emplace(pair);
-
         return true;
     }
     else

src/WorkspaceFileResolver.cpp

@@ -16,21 +16,33 @@ Luau::ModuleName WorkspaceFileResolver::getModuleName(const Uri& name)
 
     return fsPath;
 }
-const TextDocument* WorkspaceFileResolver::getTextDocument(const Luau::ModuleName& name) const
+
+const std::string WorkspaceFileResolver::normalisedUriString(const lsp::DocumentUri& uri) const
+{
+    auto uriString = uri.toString();
+
+// As windows/macOS is case insensitive, we lowercase the URI string for simplicitly and to handle
+// normalisation issues
+#if defined(_WIN32) || defined(__APPLE__)
+    uriString = toLower(uriString);
+#endif
+
+    return uriString;
+}
+
+const TextDocument* WorkspaceFileResolver::getTextDocument(const lsp::DocumentUri& uri) const
 {
-    auto it = managedFiles.find(name);
+    auto it = managedFiles.find(normalisedUriString(uri));
     if (it != managedFiles.end())
         return &it->second;
 
-    // HACK: attempting to solve "No managed text document"
-    // Check to see if we have the file stored using the URI instead
-    // https://github.com/JohnnyMorganz/luau-lsp/issues/26
-    if (auto fsPath = resolveToRealPath(name))
-    {
-        it = managedFiles.find(fsPath->generic_string());
-        if (it != managedFiles.end())
-            return &it->second;
-    }
+    return nullptr;
+}
+
+const TextDocument* WorkspaceFileResolver::getTextDocumentFromModuleName(const Luau::ModuleName& name) const
+{
+    if (auto filePath = resolveToRealPath(name))
+        return getTextDocument(Uri::file(*filePath));
 
     return nullptr;
 }
@@ -124,7 +136,7 @@ std::optional<Luau::SourceCode> WorkspaceFileResolver::readSource(const Luau::Mo
         sourceType = sourceCodeTypeFromPath(realFileName);
     }
 
-    if (auto textDocument = getTextDocument(name))
+    if (auto textDocument = getTextDocumentFromModuleName(name))
     {
         source = textDocument->getText();
     }

src/include/LSP/WorkspaceFileResolver.hpp

@@ -28,7 +28,7 @@ struct WorkspaceFileResolver
     PluginNodePtr pluginInfo;
 
     // Currently opened files where content is managed by client
-    mutable std::unordered_map<Luau::ModuleName, TextDocument> managedFiles;
+    mutable std::unordered_map</* DocumentUri */ std::string, TextDocument> managedFiles;
     mutable std::unordered_map<std::string, Luau::Config> configCache;
     // Errors found when loading .luaurc files - only used for the CLI
     mutable std::vector<std::pair<std::filesystem::path, std::string>> configErrors;
@@ -42,8 +42,12 @@ struct WorkspaceFileResolver
     WorkspaceFileResolver(const Luau::Config defaultConfig)
         : defaultConfig(defaultConfig){};
 
+    // Handle normalisation to simplify lookup
+    const std::string normalisedUriString(const lsp::DocumentUri& uri) const;
+
     /// The file is managed by the client, so FS will be out of date
-    const TextDocument* getTextDocument(const Luau::ModuleName& name) const;
+    const TextDocument* getTextDocument(const lsp::DocumentUri& uri) const;
+    const TextDocument* getTextDocumentFromModuleName(const Luau::ModuleName& name) const;
 
     /// The name points to a virtual path (i.e., game/ or ProjectRoot/)
     bool isVirtualPath(const Luau::ModuleName& name) const

src/operations/ColorProvider.cpp

@@ -199,9 +199,9 @@ lsp::DocumentColorResult WorkspaceFolder::documentColor(const lsp::DocumentColor
         return {};
 
     auto moduleName = fileResolver.getModuleName(params.textDocument.uri);
-    auto textDocument = fileResolver.getTextDocument(moduleName);
+    auto textDocument = fileResolver.getTextDocument(params.textDocument.uri);
     if (!textDocument)
-        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + moduleName);
+        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + params.textDocument.uri.toString());
 
     // Run the type checker to ensure we are up to date
     if (frontend.isDirty(moduleName))

src/operations/Completion.cpp

@@ -24,7 +24,7 @@ static constexpr const char* Keywords = "7";
 void WorkspaceFolder::endAutocompletion(const lsp::CompletionParams& params)
 {
     auto moduleName = fileResolver.getModuleName(params.textDocument.uri);
-    auto document = fileResolver.getTextDocument(moduleName);
+    auto document = fileResolver.getTextDocument(params.textDocument.uri);
     if (!document)
         return;
     auto position = document->convertPosition(params.position);
@@ -306,9 +306,9 @@ std::vector<lsp::CompletionItem> WorkspaceFolder::completion(const lsp::Completi
     }
 
     auto moduleName = fileResolver.getModuleName(params.textDocument.uri);
-    auto textDocument = fileResolver.getTextDocument(moduleName);
+    auto textDocument = fileResolver.getTextDocument(params.textDocument.uri);
     if (!textDocument)
-        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + moduleName);
+        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + params.textDocument.uri.toString());
 
     auto position = textDocument->convertPosition(params.position);
     auto result = Luau::autocomplete(frontend, moduleName, position,

src/operations/Diagnostics.cpp

@@ -16,7 +16,7 @@ lsp::DocumentDiagnosticReport WorkspaceFolder::documentDiagnostics(const lsp::Do
     std::unordered_map<std::string /* lsp::DocumentUri */, std::vector<lsp::Diagnostic>> relatedDiagnostics;
 
     auto moduleName = fileResolver.getModuleName(params.textDocument.uri);
-    auto textDocument = fileResolver.getTextDocument(moduleName);
+    auto textDocument = fileResolver.getTextDocument(params.textDocument.uri);
     if (!textDocument)
         return report; // Bail early with empty report - file was likely closed
 
@@ -47,7 +47,7 @@ lsp::DocumentDiagnosticReport WorkspaceFolder::documentDiagnostics(const lsp::Do
             auto fileName = fileResolver.resolveToRealPath(error.moduleName);
             if (!fileName || isIgnoredFile(*fileName, config))
                 continue;
-            auto diagnostic = createTypeErrorDiagnostic(error, &fileResolver, fileResolver.getTextDocument(error.moduleName));
+            auto diagnostic = createTypeErrorDiagnostic(error, &fileResolver, fileResolver.getTextDocumentFromModuleName(error.moduleName));
             auto uri = Uri::file(*fileName);
             auto& currentDiagnostics = relatedDiagnostics[uri.toString()];
             currentDiagnostics.emplace_back(diagnostic);
@@ -105,7 +105,7 @@ lsp::WorkspaceDiagnosticReport WorkspaceFolder::workspaceDiagnostics(const lsp::
     for (auto uri : files)
     {
         auto moduleName = fileResolver.getModuleName(uri);
-        auto document = fileResolver.getTextDocument(moduleName);
+        auto document = fileResolver.getTextDocument(uri);
 
         lsp::WorkspaceDocumentDiagnosticReport documentReport;
         documentReport.uri = uri;

src/operations/DocumentSymbol.cpp

@@ -139,9 +139,9 @@ struct DocumentSymbolsVisitor : public Luau::AstVisitor
 std::optional<std::vector<lsp::DocumentSymbol>> WorkspaceFolder::documentSymbol(const lsp::DocumentSymbolParams& params)
 {
     auto moduleName = fileResolver.getModuleName(params.textDocument.uri);
-    auto textDocument = fileResolver.getTextDocument(moduleName);
+    auto textDocument = fileResolver.getTextDocument(params.textDocument.uri);
     if (!textDocument)
-        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + moduleName);
+        throw JsonRpcException(lsp::ErrorCode::RequestFailed, "No managed text document for " + params.textDocument.uri.toString());
 
     // Run the type checker to ensure we are up to date
     if (frontend.isDirty(moduleName))