From 85cc799b36ecf468971519ad81d306b06e7c36df Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Sat, 10 Aug 2024 16:31:55 +0200 Subject: [PATCH] Fix module names no longer resolving to virtual paths And add tests for it. Also fix an exception not getting thrown / caught properly Fixes #734 Fixes #735 --- CHANGELOG.md | 4 ++ src/Workspace.cpp | 4 +- src/platform/roblox/RobloxSourcemap.cpp | 2 + tests/Sourcemap.test.cpp | 56 +++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c950ea..567ee9fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed + +- Fixed a regression in 1.32.2 breaking resolution of virtual paths from real paths, particularly around `script` and relative usages of it. ([#734](https://github.com/JohnnyMorganz/luau-lsp/issues/734), [#735](https://github.com/JohnnyMorganz/luau-lsp/issues/735)) + ## [1.32.2] - 2024-08-10 ### Changed diff --git a/src/Workspace.cpp b/src/Workspace.cpp index 0770695e..2c2a757d 100644 --- a/src/Workspace.cpp +++ b/src/Workspace.cpp @@ -140,7 +140,7 @@ bool WorkspaceFolder::isIgnoredFile(const std::filesystem::path& path, const std // We want to test globs against a relative path to workspace, since that's what makes most sense auto relativeFsPath = path.lexically_relative(rootUri.fsPath()); if (relativeFsPath == std::filesystem::path()) - throw new JsonRpcException(lsp::ErrorCode::InternalError, "isIgnoredFile failed: relative path is default-constructed"); + throw JsonRpcException(lsp::ErrorCode::InternalError, "isIgnoredFile failed: relative path is default-constructed"); auto relativePathString = relativeFsPath.generic_string(); // HACK: we convert to generic string so we get '/' separators auto config = givenConfig ? *givenConfig : client->getConfiguration(rootUri); @@ -160,7 +160,7 @@ bool WorkspaceFolder::isIgnoredFileForAutoImports(const std::filesystem::path& p // We want to test globs against a relative path to workspace, since that's what makes most sense auto relativeFsPath = path.lexically_relative(rootUri.fsPath()); if (relativeFsPath == std::filesystem::path()) - throw new JsonRpcException(lsp::ErrorCode::InternalError, "isIgnoredFileForAutoImports failed: relative path is default-constructed"); + throw JsonRpcException(lsp::ErrorCode::InternalError, "isIgnoredFileForAutoImports failed: relative path is default-constructed"); auto relativePathString = relativeFsPath.generic_string(); // HACK: we convert to generic string so we get '/' separators auto config = givenConfig ? *givenConfig : client->getConfiguration(rootUri); diff --git a/src/platform/roblox/RobloxSourcemap.cpp b/src/platform/roblox/RobloxSourcemap.cpp index e382584c..15a326b2 100644 --- a/src/platform/roblox/RobloxSourcemap.cpp +++ b/src/platform/roblox/RobloxSourcemap.cpp @@ -477,6 +477,8 @@ std::optional RobloxPlatform::getSourceNodeFromRealPath(const std auto canonicalName = std::filesystem::weakly_canonical(name, ec); if (ec.value() != 0) canonicalName = name; + // URI-ify the file path so that its normalised (in particular, the drive letter) + canonicalName = Uri::parse(Uri::file(canonicalName).toString()).fsPath(); auto strName = canonicalName.generic_string(); if (realPathsToSourceNodes.find(strName) == realPathsToSourceNodes.end()) return std::nullopt; diff --git a/tests/Sourcemap.test.cpp b/tests/Sourcemap.test.cpp index 4fc8809a..640ed707 100644 --- a/tests/Sourcemap.test.cpp +++ b/tests/Sourcemap.test.cpp @@ -374,6 +374,62 @@ TEST_CASE_FIXTURE(Fixture, "relative_and_absolute_types_are_consistent") CHECK((absoluteTy == relativeTy)); } +TEST_CASE_FIXTURE(Fixture, "get_virtual_module_name_from_real_path") +{ +#ifdef _WIN32 + workspace.rootUri = Uri::parse("file:///c%3A/Users/Development/project"); + workspace.fileResolver.rootUri = Uri::parse("file:///c%3A/Users/Development/project"); + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [{"name": "MainScript", "className": "ModuleScript", "filePaths": ["Foo\\Test.luau"]}] + } + )"); +#else + workspace.rootUri = Uri::parse("/home/project"); + workspace.fileResolver.rootUri = Uri::parse("/home/project"); + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [{"name": "MainScript", "className": "ModuleScript", "filePaths": ["Foo/Test.luau"]}] + } + )"); +#endif + + auto uri = Uri::file(workspace.rootUri.fsPath() / "Foo" / "Test.luau"); + + CHECK_EQ(workspace.fileResolver.getModuleName(uri), "game/MainScript"); +} + +TEST_CASE_FIXTURE(Fixture, "get_real_path_from_virtual_name") +{ +#ifdef _WIN32 + workspace.rootUri = Uri::parse("file:///c%3A/Users/Development/project"); + workspace.fileResolver.rootUri = Uri::parse("file:///c%3A/Users/Development/project"); + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [{"name": "MainScript", "className": "ModuleScript", "filePaths": ["Foo\\Test.luau"]}] + } + )"); +#else + workspace.rootUri = Uri::parse("/home/project"); + workspace.fileResolver.rootUri = Uri::parse("/home/project"); + loadSourcemap(R"( + { + "name": "Game", + "className": "DataModel", + "children": [{"name": "MainScript", "className": "ModuleScript", "filePaths": ["Foo/Test.luau"]}] + } + )"); +#endif + + CHECK_EQ(workspace.platform->resolveToRealPath("game/MainScript"), workspace.rootUri.fsPath() / "Foo" / "Test.luau"); +} + TEST_CASE_FIXTURE(Fixture, "sourcemap_path_is_normalised_to_match_root_uri_subchild_with_lower_case_drive_letter") { #ifdef _WIN32