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
JohnnyMorganz · Aug 10, 2024 · 85cc799 · 85cc799
1 parent c04866b
commit 85cc799
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 2 deletions.
diff --git 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
@@ -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
@@ -477,6 +477,8 @@ std::optional<SourceNodePtr> 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
@@ -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