Remove weakly_canonical and replace with Luau's normalisePath

CHANGELOG.md

@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Added
+
+- Auto-import requires will now show the require path in the details section rather than just "Auto-import". This will
+  help to disambiguate between multiple modules with the same name but at different
+  paths. ([#593](https://github.com/JohnnyMorganz/luau-lsp/issues/593))
+
 ### Changed
 
 - The server no longer computes `relatedDocuments` in a `textDocument/diagnostic` request unnecessarily if the client
@@ -24,6 +30,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
     0.24s) ([#749](https://github.com/JohnnyMorganz/luau-lsp/issues/749))
   - These improvements heavily depend on the amount of code you have matching ignore globs. Workspace diagnostics
     improvements depends on the performance of Luau typechecking.
+- Optimised the resolution of relative string requires by remove filesystem
+  calls ([#856](https://github.com/JohnnyMorganz/luau-lsp/issues/856))
 
 ### Fixed
 

CMakeLists.txt

@@ -76,6 +76,7 @@ target_sources(Luau.LanguageServer.Test PRIVATE
         tests/Fixture.cpp
         tests/TempDir.cpp
         tests/Autocomplete.test.cpp
+        tests/AutoImports.test.cpp
         tests/MagicFunctions.test.cpp
         tests/Documentation.test.cpp
         tests/TextDocument.test.cpp

src/Utils.cpp

@@ -214,3 +214,137 @@ void replaceAll(std::string& str, const std::string& from, const std::string& to
         start_pos += to.length();
     }
 }
+
+bool isAbsolutePath(std::string_view path)
+{
+#ifdef _WIN32
+    // Must either begin with "X:/", "X:\", "/", or "\", where X is a drive letter
+    return (path.size() >= 3 && isalpha(path[0]) && path[1] == ':' && (path[2] == '/' || path[2] == '\\')) ||
+           (path.size() >= 1 && (path[0] == '/' || path[0] == '\\'));
+#else
+    // Must begin with '/'
+    return path.size() >= 1 && path[0] == '/';
+#endif
+}
+
+// Returns the normal/canonical form of a path (e.g. "../subfolder/../module.luau" -> "../module.luau")
+std::string normalizePath(std::string_view path)
+{
+    return resolvePath(path, "");
+}
+
+// Takes a path that is relative to the file at baseFilePath and returns the path explicitly rebased onto baseFilePath.
+// For absolute paths, baseFilePath will be ignored, and this function will resolve the path to a canonical path:
+// (e.g. "/Users/.././Users/johndoe" -> "/Users/johndoe").
+std::string resolvePath(std::string_view path, std::string_view baseFilePath)
+{
+    std::vector<std::string_view> pathComponents;
+    std::vector<std::string_view> baseFilePathComponents;
+
+    // Dependent on whether the final resolved path is absolute or relative
+    // - if relative (when path and baseFilePath are both relative), resolvedPathPrefix remains empty
+    // - if absolute (if either path or baseFilePath are absolute), resolvedPathPrefix is "C:\", "/", etc.
+    std::string resolvedPathPrefix;
+    bool isResolvedPathRelative = false;
+
+    if (isAbsolutePath(path))
+    {
+        // path is absolute, we use path's prefix and ignore baseFilePath
+        size_t afterPrefix = path.find_first_of("\\/") + 1;
+        resolvedPathPrefix = path.substr(0, afterPrefix);
+        pathComponents = splitPath(path.substr(afterPrefix));
+    }
+    else
+    {
+        size_t afterPrefix = baseFilePath.find_first_of("\\/") + 1;
+        baseFilePathComponents = splitPath(baseFilePath.substr(afterPrefix));
+        if (isAbsolutePath(baseFilePath))
+        {
+            // path is relative and baseFilePath is absolute, we use baseFilePath's prefix
+            resolvedPathPrefix = baseFilePath.substr(0, afterPrefix);
+        }
+        else
+        {
+            // path and baseFilePath are both relative, we do not set a prefix (resolved path will be relative)
+            isResolvedPathRelative = true;
+        }
+        pathComponents = splitPath(path);
+    }
+
+    // Remove filename from components
+    if (!baseFilePathComponents.empty())
+        baseFilePathComponents.pop_back();
+
+    // Resolve the path by applying pathComponents to baseFilePathComponents
+    int numPrependedParents = 0;
+    for (std::string_view component : pathComponents)
+    {
+        if (component == "..")
+        {
+            if (baseFilePathComponents.empty())
+            {
+                if (isResolvedPathRelative)
+                    numPrependedParents++; // "../" will later be added to the beginning of the resolved path
+            }
+            else if (baseFilePathComponents.back() != "..")
+            {
+                baseFilePathComponents.pop_back(); // Resolve cases like "folder/subfolder/../../file" to "file"
+            }
+        }
+        else if (component != "." && !component.empty())
+        {
+            baseFilePathComponents.push_back(component);
+        }
+    }
+
+    // Create resolved path prefix for relative paths
+    if (isResolvedPathRelative)
+    {
+        if (numPrependedParents > 0)
+        {
+            resolvedPathPrefix.reserve(numPrependedParents * 3);
+            for (int i = 0; i < numPrependedParents; i++)
+            {
+                resolvedPathPrefix += "../";
+            }
+        }
+        else
+        {
+            resolvedPathPrefix = "./";
+        }
+    }
+
+    // Join baseFilePathComponents to form the resolved path
+    std::string resolvedPath = resolvedPathPrefix;
+    for (auto iter = baseFilePathComponents.begin(); iter != baseFilePathComponents.end(); ++iter)
+    {
+        if (iter != baseFilePathComponents.begin())
+            resolvedPath += "/";
+
+        resolvedPath += *iter;
+    }
+    if (resolvedPath.size() > resolvedPathPrefix.size() && resolvedPath.back() == '/')
+    {
+        // Remove trailing '/' if present
+        resolvedPath.pop_back();
+    }
+    return resolvedPath;
+}
+
+std::vector<std::string_view> splitPath(std::string_view path)
+{
+    std::vector<std::string_view> components;
+
+    size_t pos = 0;
+    size_t nextPos = path.find_first_of("\\/", pos);
+
+    while (nextPos != std::string::npos)
+    {
+        components.push_back(path.substr(pos, nextPos - pos));
+        pos = nextPos + 1;
+        nextPos = path.find_first_of("\\/", pos);
+    }
+    components.push_back(path.substr(pos));
+
+    return components;
+}

src/Workspace.cpp

@@ -198,11 +198,11 @@ bool WorkspaceFolder::isIgnoredFileForAutoImports(const std::filesystem::path& p
 bool WorkspaceFolder::isDefinitionFile(const std::filesystem::path& path, const std::optional<ClientConfiguration>& givenConfig)
 {
     auto config = givenConfig ? *givenConfig : client->getConfiguration(rootUri);
-    auto canonicalised = std::filesystem::weakly_canonical(path);
+    auto canonicalised = normalizePath(path.generic_string());
 
     for (auto& file : config.types.definitionFiles)
     {
-        if (std::filesystem::weakly_canonical(resolvePath(file)) == canonicalised)
+        if (normalizePath(resolvePath(file).generic_string()) == canonicalised)
         {
             return true;
         }

src/include/LSP/Utils.hpp

@@ -46,3 +46,9 @@ inline bool contains(const std::map<K, V>& map, const K& value)
 {
     return map.find(value) != map.end();
 }
+
+// TODO: taken from FileUtils, use that instead?
+bool isAbsolutePath(std::string_view path);
+std::string normalizePath(std::string_view path);
+std::string resolvePath(std::string_view relativePath, std::string_view baseFilePath);
+std::vector<std::string_view> splitPath(std::string_view path);

src/platform/LSPPlatform.cpp

@@ -142,10 +142,10 @@ std::optional<Luau::ModuleInfo> LSPPlatform::resolveStringRequire(const Luau::Mo
         }
     }
 
-    std::error_code ec;
-    filePath = std::filesystem::weakly_canonical(filePath, ec);
+    filePath = normalizePath((fileResolver->rootUri.fsPath() / filePath).generic_string());
 
     // Handle "init.luau" files in a directory
+    std::error_code ec;
     if (std::filesystem::is_directory(filePath, ec))
     {
         filePath /= "init";

src/platform/roblox/RobloxCompletion.cpp

@@ -80,8 +80,8 @@ static lsp::TextEdit createRequireTextEdit(const std::string& name, const std::s
     return {range, importText};
 }
 
-static lsp::CompletionItem createSuggestRequire(
-    const std::string& name, const std::vector<lsp::TextEdit>& textEdits, const char* sortText, const std::string& path)
+static lsp::CompletionItem createSuggestRequire(const std::string& name, const std::vector<lsp::TextEdit>& textEdits, const char* sortText,
+    const std::string& path, const std::string& requirePath)
 {
     std::string documentation;
     for (const auto& edit : textEdits)
@@ -90,7 +90,7 @@ static lsp::CompletionItem createSuggestRequire(
     lsp::CompletionItem item;
     item.label = name;
     item.kind = lsp::CompletionItemKind::Module;
-    item.detail = "Auto-import";
+    item.detail = requirePath;
     item.documentation = {lsp::MarkupKind::Markdown, codeBlock("luau", documentation) + "\n\n" + path};
     item.insertText = name;
     item.sortText = sortText;
@@ -433,7 +433,8 @@ void RobloxPlatform::handleSuggestImports(const TextDocument& textDocument, cons
 
             textEdits.emplace_back(createRequireTextEdit(node->name, require, lineNumber, prependNewline));
 
-            items.emplace_back(createSuggestRequire(name, textEdits, isRelative ? SortText::AutoImports : SortText::AutoImportsAbsolute, path));
+            items.emplace_back(
+                createSuggestRequire(name, textEdits, isRelative ? SortText::AutoImports : SortText::AutoImportsAbsolute, path, require));
         }
     }
 }

src/platform/roblox/RobloxSourcemap.cpp

@@ -505,16 +505,12 @@ std::optional<SourceNodePtr> RobloxPlatform::getSourceNodeFromVirtualPath(const
 
 std::optional<SourceNodePtr> RobloxPlatform::getSourceNodeFromRealPath(const std::string& name) const
 {
-    std::error_code ec;
-    auto canonicalName = std::filesystem::weakly_canonical(name, ec);
-    if (ec.value() != 0)
-        canonicalName = name;
+    auto canonicalName = normalizePath(name);
     // URI-ify the file path so that its normalised (in particular, the drive letter)
-    canonicalName = Uri::file(canonicalName).fsPath();
-    auto strName = canonicalName.generic_string();
-    if (realPathsToSourceNodes.find(strName) == realPathsToSourceNodes.end())
+    canonicalName = Uri::file(canonicalName).fsPath().generic_string();
+    if (realPathsToSourceNodes.find(canonicalName) == realPathsToSourceNodes.end())
         return std::nullopt;
-    return realPathsToSourceNodes.at(strName);
+    return realPathsToSourceNodes.at(canonicalName);
 }
 
 Luau::ModuleName RobloxPlatform::getVirtualPathFromSourceNode(const SourceNodePtr& sourceNode)
@@ -529,10 +525,7 @@ std::optional<std::filesystem::path> RobloxPlatform::getRealPathFromSourceNode(c
     // TODO: make sure this is correct once we make sourcemap.json generic
     if (auto filePath = sourceNode->getScriptFilePath())
     {
-        std::error_code ec;
-        auto canonicalName = std::filesystem::weakly_canonical(fileResolver->rootUri.fsPath() / *filePath, ec);
-        if (ec.value() != 0)
-            canonicalName = *filePath;
+        auto canonicalName = normalizePath((fileResolver->rootUri.fsPath() / *filePath).generic_string());
         // URI-ify the file path so that its normalised (in particular, the drive letter)
         return Uri::file(canonicalName).fsPath();
     }