diff --git a/CHANGELOG.md b/CHANGELOG.md index ec08f02c..2c5e9b7d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 9b68c1bd..c018c045 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/src/Utils.cpp b/src/Utils.cpp index ff0c5c9d..d36ba9a1 100644 --- a/src/Utils.cpp +++ b/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 pathComponents; + std::vector 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 splitPath(std::string_view path) +{ + std::vector 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; +} \ No newline at end of file diff --git a/src/Workspace.cpp b/src/Workspace.cpp index eaf95dd7..fb78769d 100644 --- a/src/Workspace.cpp +++ b/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& 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; } diff --git a/src/include/LSP/Utils.hpp b/src/include/LSP/Utils.hpp index 8e137190..1239f7d1 100644 --- a/src/include/LSP/Utils.hpp +++ b/src/include/LSP/Utils.hpp @@ -46,3 +46,9 @@ inline bool contains(const std::map& 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 splitPath(std::string_view path); diff --git a/src/platform/LSPPlatform.cpp b/src/platform/LSPPlatform.cpp index 22fdf1e7..46287ac3 100644 --- a/src/platform/LSPPlatform.cpp +++ b/src/platform/LSPPlatform.cpp @@ -142,10 +142,10 @@ std::optional 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"; diff --git a/src/platform/roblox/RobloxCompletion.cpp b/src/platform/roblox/RobloxCompletion.cpp index 791098f5..50282cfe 100644 --- a/src/platform/roblox/RobloxCompletion.cpp +++ b/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& textEdits, const char* sortText, const std::string& path) +static lsp::CompletionItem createSuggestRequire(const std::string& name, const std::vector& 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)); } } } diff --git a/src/platform/roblox/RobloxSourcemap.cpp b/src/platform/roblox/RobloxSourcemap.cpp index f8d7004f..2d4777dd 100644 --- a/src/platform/roblox/RobloxSourcemap.cpp +++ b/src/platform/roblox/RobloxSourcemap.cpp @@ -505,16 +505,12 @@ std::optional RobloxPlatform::getSourceNodeFromVirtualPath(const std::optional 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 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(); } diff --git a/tests/AutoImports.test.cpp b/tests/AutoImports.test.cpp new file mode 100644 index 00000000..a14e7164 --- /dev/null +++ b/tests/AutoImports.test.cpp @@ -0,0 +1,663 @@ +#include "doctest.h" +#include "Fixture.h" +#include "TempDir.h" +#include "Platform/RobloxPlatform.hpp" +#include "LSP/IostreamHelpers.hpp" + +static std::optional getItem(const std::vector& items, const std::string& label) +{ + for (const auto& item : items) + if (item.label == label) + return item; + + return std::nullopt; +} + +static std::vector filterAutoImports(const std::vector& items, const std::string& moduleName = "") +{ + std::vector results; + + for (const auto& item : items) + { + if (item.kind == lsp::CompletionItemKind::Module) + if (moduleName.empty() || item.label == moduleName) + results.emplace_back(item); + } + + return results; +} + +TEST_SUITE_BEGIN("AutoImports"); + +TEST_CASE_FIXTURE(Fixture, "services_show_up_in_auto_import") +{ + client->globalConfig.completion.imports.enabled = true; + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + auto serviceImport = getItem(result, "ReplicatedStorage"); + REQUIRE(serviceImport); + + CHECK_EQ(serviceImport->label, "ReplicatedStorage"); + CHECK_EQ(serviceImport->insertText, "ReplicatedStorage"); + CHECK_EQ(serviceImport->detail, "Auto-import"); + CHECK_EQ(serviceImport->kind, lsp::CompletionItemKind::Class); + REQUIRE_EQ(serviceImport->additionalTextEdits.size(), 1); + CHECK_EQ(serviceImport->additionalTextEdits[0].newText, "local ReplicatedStorage = game:GetService(\"ReplicatedStorage\")\n"); + CHECK_EQ(serviceImport->additionalTextEdits[0].range, lsp::Range{{0, 0}, {0, 0}}); + + CHECK(getItem(result, "ServerScriptService")); + CHECK(getItem(result, "Workspace")); +} + +TEST_CASE_FIXTURE(Fixture, "services_do_not_show_up_in_autocomplete_if_imports_is_disabled") +{ + client->globalConfig.completion.imports.enabled = false; + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + CHECK_EQ(getItem(result, "ReplicatedStorage"), std::nullopt); +} + +TEST_CASE_FIXTURE(Fixture, "services_do_not_show_up_in_autocomplete_if_suggest_services_is_disabled") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.suggestServices = false; + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + CHECK_EQ(getItem(result, "ReplicatedStorage"), std::nullopt); +} + +TEST_CASE_FIXTURE(Fixture, "service_does_not_show_up_in_autocomplete_if_already_exists") +{ + client->globalConfig.completion.imports.enabled = true; + auto [source, marker] = sourceWithMarker(R"( + local ReplicatedStorage = game:GetService("ReplicatedStorage") + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + // item will exist as variable import, but it shouldn't be an auto import + auto item = getItem(result, "ReplicatedStorage"); + CHECK_NE(item->detail, "Auto-import"); + CHECK_EQ(item->additionalTextEdits.size(), 0); +} + +TEST_CASE_FIXTURE(Fixture, "service_auto_imports_are_inserted_alphabetically") +{ + client->globalConfig.completion.imports.enabled = true; + auto [source, marker] = sourceWithMarker(R"( + local ServerScriptService = game:GetService("ServerScriptService") + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + auto beforeImport = getItem(result, "ReplicatedStorage"); + REQUIRE_EQ(beforeImport->additionalTextEdits.size(), 1); + CHECK_EQ(beforeImport->additionalTextEdits[0].range, lsp::Range{{1, 0}, {1, 0}}); + + auto afterImport = getItem(result, "Workspace"); + REQUIRE_EQ(afterImport->additionalTextEdits.size(), 1); + CHECK_EQ(afterImport->additionalTextEdits[0].range, lsp::Range{{2, 0}, {2, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "service_auto_imports_are_inserted_after_hot_comments") +{ + client->globalConfig.completion.imports.enabled = true; + auto [source, marker] = sourceWithMarker(R"( + --!strict + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + auto import = getItem(result, "ReplicatedStorage"); + REQUIRE(import); + REQUIRE_EQ(import->additionalTextEdits.size(), 1); + CHECK_EQ(import->additionalTextEdits[0].range, lsp::Range{{2, 0}, {2, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "module_script_shows_up_in_auto_imports") +{ + client->globalConfig.completion.imports.enabled = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "Module"); + + REQUIRE_EQ(imports.size(), 1); + CHECK_EQ(imports[0].label, "Module"); + CHECK_EQ(imports[0].insertText, "Module"); + CHECK_EQ(imports[0].kind, lsp::CompletionItemKind::Module); + + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 2); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local ReplicatedStorage = game:GetService(\"ReplicatedStorage\")\n"); + CHECK_EQ(imports[0].additionalTextEdits[0].range, lsp::Range{{0, 0}, {0, 0}}); + CHECK_EQ(imports[0].additionalTextEdits[1].newText, "local Module = require(ReplicatedStorage.Folder.Module)\n"); + CHECK_EQ(imports[0].additionalTextEdits[1].range, lsp::Range{{0, 0}, {0, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "module_script_does_not_show_up_in_autocomplete_if_imports_is_disabled") +{ + client->globalConfig.completion.imports.enabled = false; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + CHECK_EQ(filterAutoImports(result, "Module").size(), 0); +} + +TEST_CASE_FIXTURE(Fixture, "module_script_does_not_show_up_in_autocomplete_if_require_imports_is_disabled") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.suggestRequires = false; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + CHECK_EQ(filterAutoImports(result, "Module").size(), 0); +} + +TEST_CASE_FIXTURE(Fixture, "auto_import_reuses_service_if_already_defined") +{ + client->globalConfig.completion.imports.enabled = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + local ReplicatedStorage = game:GetService("ReplicatedStorage") + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "Module"); + + REQUIRE_EQ(imports.size(), 1); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 1); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local Module = require(ReplicatedStorage.Folder.Module)\n"); + CHECK_EQ(imports[0].additionalTextEdits[0].range, lsp::Range{{2, 0}, {2, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "auto_import_separates_new_service_and_require_with_line") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.separateGroupsWithLine = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "Module"); + + REQUIRE_EQ(imports.size(), 1); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 2); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local ReplicatedStorage = game:GetService(\"ReplicatedStorage\")\n\n"); + CHECK_EQ(imports[0].additionalTextEdits[0].range, lsp::Range{{0, 0}, {0, 0}}); + CHECK_EQ(imports[0].additionalTextEdits[1].newText, "local Module = require(ReplicatedStorage.Folder.Module)\n"); + CHECK_EQ(imports[0].additionalTextEdits[1].range, lsp::Range{{0, 0}, {0, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "auto_import_separates_existing_service_and_new_require_with_line") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.separateGroupsWithLine = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + local ReplicatedStorage = game:GetService("ReplicatedStorage") + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "Module"); + + REQUIRE_EQ(imports.size(), 1); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 1); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "\nlocal Module = require(ReplicatedStorage.Folder.Module)\n"); + CHECK_EQ(imports[0].additionalTextEdits[0].range, lsp::Range{{2, 0}, {2, 0}}); +} + +TEST_CASE_FIXTURE(Fixture, "auto_imports_will_prefer_relative_over_absolute_import_for_sibling_modules") +{ + client->globalConfig.completion.imports.enabled = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Module", + "className": "ModuleScript", + "filePaths": ["source.luau"] + }, + { + "name": "SiblingModule", + "className": "ModuleScript" + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("source.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "SiblingModule"); + + REQUIRE_EQ(imports.size(), 1); + CHECK_EQ(imports[0].label, "SiblingModule"); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 1); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local SiblingModule = require(script.Parent.SiblingModule)\n"); +} + +TEST_CASE_FIXTURE(Fixture, "auto_imports_will_force_absolute_import_depending_on_setting") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.requireStyle = ImportRequireStyle::AlwaysAbsolute; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Module", + "className": "ModuleScript", + "filePaths": ["source.luau"] + }, + { + "name": "SiblingModule", + "className": "ModuleScript" + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("source.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "SiblingModule"); + + REQUIRE_EQ(imports.size(), 1); + CHECK_EQ(imports[0].label, "SiblingModule"); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 2); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local ReplicatedStorage = game:GetService(\"ReplicatedStorage\")\n"); + CHECK_EQ(imports[0].additionalTextEdits[1].newText, "local SiblingModule = require(ReplicatedStorage.SiblingModule)\n"); +} + +TEST_CASE_FIXTURE(Fixture, "auto_imports_will_prefer_absolute_import_over_relative_import_for_further_away_modules") +{ + client->globalConfig.completion.imports.enabled = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Module", + "className": "ModuleScript", + "filePaths": ["source.luau"] + }, + { + "name": "Folder", + "className": "Folder", + "children": [ + { "name": "OtherModule", "className": "ModuleScript" } + ] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("source.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "OtherModule"); + + REQUIRE_EQ(imports.size(), 1); + CHECK_EQ(imports[0].label, "OtherModule"); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 2); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local ReplicatedStorage = game:GetService(\"ReplicatedStorage\")\n"); + CHECK_EQ(imports[0].additionalTextEdits[1].newText, "local OtherModule = require(ReplicatedStorage.Folder.OtherModule)\n"); +} + +TEST_CASE_FIXTURE(Fixture, "auto_imports_will_force_relative_import_depending_on_setting") +{ + client->globalConfig.completion.imports.enabled = true; + client->globalConfig.completion.imports.requireStyle = ImportRequireStyle::AlwaysRelative; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Module", + "className": "ModuleScript", + "filePaths": ["source.luau"] + }, + { + "name": "Folder", + "className": "Folder", + "children": [ + { "name": "OtherModule", "className": "ModuleScript" } + ] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("source.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "OtherModule"); + + REQUIRE_EQ(imports.size(), 1); + CHECK_EQ(imports[0].label, "OtherModule"); + REQUIRE_EQ(imports[0].additionalTextEdits.size(), 1); + CHECK_EQ(imports[0].additionalTextEdits[0].newText, "local OtherModule = require(script.Parent.Folder.OtherModule)\n"); +} + +TEST_CASE_FIXTURE(Fixture, "auto_imports_of_modules_show_path_name") +{ + client->globalConfig.completion.imports.enabled = true; + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [ + { + "name": "ReplicatedStorage", + "className": "ReplicatedStorage", + "children": [ + { + "name": "Folder1", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + }, + { + "name": "Folder2", + "className": "Folder", + "children": [{ "name": "Module", "className": "ModuleScript" }] + } + ] + } + ] + } + )"); + + auto [source, marker] = sourceWithMarker(R"( + | + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + auto imports = filterAutoImports(result, "Module"); + + REQUIRE_EQ(imports.size(), 2); + CHECK_EQ(imports[1].detail, "ReplicatedStorage.Folder1.Module"); + CHECK_EQ(imports[0].detail, "ReplicatedStorage.Folder2.Module"); +} + +TEST_SUITE_END(); diff --git a/tests/WorkspaceFileResolver.test.cpp b/tests/WorkspaceFileResolver.test.cpp index 7df092f3..35829a72 100644 --- a/tests/WorkspaceFileResolver.test.cpp +++ b/tests/WorkspaceFileResolver.test.cpp @@ -4,6 +4,7 @@ #include "Platform/RobloxPlatform.hpp" #include "Luau/Ast.h" #include "Luau/FileResolver.h" +#include "TempDir.h" TEST_SUITE_BEGIN("WorkspaceFileResolverTests"); @@ -238,12 +239,8 @@ TEST_CASE_FIXTURE(Fixture, "resolve_alias_supports_tilde_expansion") TEST_CASE_FIXTURE(Fixture, "string require doesn't add file extension if already exists") { - WorkspaceFileResolver fileResolver; - LSPPlatform platform{&fileResolver}; - fileResolver.platform = &platform; - Luau::ModuleInfo baseContext{}; - auto resolved = platform.resolveStringRequire(&baseContext, "Module.luau"); + auto resolved = workspace.platform->resolveStringRequire(&baseContext, "Module.luau"); REQUIRE(resolved.has_value()); CHECK(endsWith(resolved->name, "/Module.luau")); @@ -251,12 +248,8 @@ TEST_CASE_FIXTURE(Fixture, "string require doesn't add file extension if already TEST_CASE_FIXTURE(Fixture, "string require doesn't replace a non-luau/lua extension") { - WorkspaceFileResolver fileResolver; - LSPPlatform platform{&fileResolver}; - fileResolver.platform = &platform; - Luau::ModuleInfo baseContext{}; - auto resolved = platform.resolveStringRequire(&baseContext, "Module.mod"); + auto resolved = workspace.platform->resolveStringRequire(&baseContext, "Module.mod"); REQUIRE(resolved.has_value()); CHECK(endsWith(resolved->name, "/Module.mod.lua")); @@ -272,4 +265,51 @@ TEST_CASE_FIXTURE(Fixture, "string_require_resolves_relative_to_file") CHECK_EQ(Luau::toString(requireType(getModule(moduleName), "other")), "number"); } +TEST_CASE_FIXTURE(Fixture, "string_require_supports_relative_file_aliases") +{ + client->globalConfig.require.fileAliases = { + {"@file", "path/to/source.luau"}, + }; + + Luau::ModuleInfo baseContext{}; + auto resolved = workspace.platform->resolveStringRequire(&baseContext, "@file"); + REQUIRE(resolved); + CHECK_EQ(resolved->name, workspace.rootUri.fsPath() / "path/to/source.luau"); +} + +TEST_CASE_FIXTURE(Fixture, "string_require_supports_absolute_file_aliases") +{ +#ifdef _WIN32 + auto basePath = "C:/Users/test/folder"; +#else + auto basePath = "/home/folder"; +#endif + + auto fullPath = (std::filesystem::path(basePath) / "path/to/source.luau").generic_string(); + + client->globalConfig.require.fileAliases = { + {"@file", fullPath}, + }; + + Luau::ModuleInfo baseContext{}; + auto resolved = workspace.platform->resolveStringRequire(&baseContext, "@file"); + REQUIRE(resolved); + CHECK_EQ(resolved->name, fullPath); +} + +TEST_CASE_FIXTURE(Fixture, "string_require_supports_absolute_file_aliases_with_tilde_expansion") +{ + client->globalConfig.require.fileAliases = { + {"@file", "~/path/to/source.luau"}, + }; + + auto home = getHomeDirectory(); + REQUIRE(home); + + Luau::ModuleInfo baseContext{}; + auto resolved = workspace.platform->resolveStringRequire(&baseContext, "@file"); + REQUIRE(resolved); + CHECK_EQ(resolved->name, home.value() / "path/to/source.luau"); +} + TEST_SUITE_END(); diff --git a/tests/testdata/standard_definitions.d.luau b/tests/testdata/standard_definitions.d.luau index f7b1a192..62a43c20 100644 --- a/tests/testdata/standard_definitions.d.luau +++ b/tests/testdata/standard_definitions.d.luau @@ -1,4 +1,4 @@ ---#METADATA#{"CREATABLE_INSTANCES":["TextLabel","Part"], "SERVICES": ["ReplicatedStorage"]} +--#METADATA#{"CREATABLE_INSTANCES":["TextLabel","Part"], "SERVICES": ["ReplicatedStorage","ServerScriptService","Workspace"]} declare class Enum function GetEnumItems(self): { any } @@ -82,6 +82,12 @@ end declare class ReplicatedStorage extends Instance end +declare class ServerScriptService extends Instance +end + +declare class Workspace extends Instance +end + declare class ServiceProvider extends Instance function GetService(self, className: string): Instance end