Fix use of init files and directory aliases

Fixes #473
JohnnyMorganz · Oct 14, 2023 · 0de2260 · 0de2260
1 parent b2fe09e
commit 0de2260
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Fixed
 
 - Do not add line separator in hover when there is no text documentation
+- Fixed init files not working with directory aliases (e.g. `require("@dir")` or `require("@dir/subdir")`)
 
 ## [1.24.1] - 2023-09-09
 

diff --git a/src/WorkspaceFileResolver.cpp b/src/WorkspaceFileResolver.cpp
@@ -229,8 +229,8 @@ std::filesystem::path WorkspaceFileResolver::getRequireBasePath(std::optional<Lu
 }
 
 // Resolve the string using a directory alias if present
-std::optional<std::filesystem::path> resolveDirectoryAlias(const std::filesystem::path& rootPath,
-    const std::unordered_map<std::string, std::string>& directoryAliases, const std::string& str, bool includeExtension)
+std::optional<std::filesystem::path> resolveDirectoryAlias(
+    const std::filesystem::path& rootPath, const std::unordered_map<std::string, std::string>& directoryAliases, const std::string& str)
 {
     for (const auto& [alias, path] : directoryAliases)
     {
@@ -247,13 +247,6 @@ std::optional<std::filesystem::path> resolveDirectoryAlias(const std::filesystem
             if (!filePath.is_absolute())
                 filePath = rootPath / filePath;
 
-            if (includeExtension && !filePath.has_extension())
-            {
-                if (std::filesystem::exists(filePath.replace_extension(".luau")))
-                    return filePath.replace_extension(".luau");
-                else
-                    return filePath.replace_extension(".lua");
-            }
             return filePath;
         }
     }
@@ -284,6 +277,7 @@ std::optional<Luau::ModuleInfo> WorkspaceFileResolver::resolveStringRequire(cons
     }
 
     std::error_code ec;
+    filePath = std::filesystem::weakly_canonical(filePath, ec);
 
     // Handle "init.luau" files in a directory
     if (std::filesystem::is_directory(filePath, ec))
@@ -294,10 +288,10 @@ std::optional<Luau::ModuleInfo> WorkspaceFileResolver::resolveStringRequire(cons
     // Add file endings
     if (filePath.extension() != ".luau" && filePath.extension() != ".lua")
     {
-        auto fullFilePath = std::filesystem::weakly_canonical(filePath.string() + ".luau", ec);
-        if (ec.value() != 0 || !std::filesystem::exists(fullFilePath))
+        auto fullFilePath = filePath.string() + ".luau";
+        if (!std::filesystem::exists(fullFilePath))
             // fall back to .lua if a module with .luau doesn't exist
-            filePath = std::filesystem::weakly_canonical(filePath.string() + ".lua", ec);
+            filePath = filePath.string() + ".lua";
         else
             filePath = fullFilePath;
     }

diff --git a/src/include/LSP/WorkspaceFileResolver.hpp b/src/include/LSP/WorkspaceFileResolver.hpp
@@ -68,8 +68,8 @@ struct TextDocumentPtr
     }
 };
 
-std::optional<std::filesystem::path> resolveDirectoryAlias(const std::filesystem::path& rootPath,
-    const std::unordered_map<std::string, std::string>& directoryAliases, const std::string& str, bool includeExtension = true);
+std::optional<std::filesystem::path> resolveDirectoryAlias(
+    const std::filesystem::path& rootPath, const std::unordered_map<std::string, std::string>& directoryAliases, const std::string& str);
 
 struct WorkspaceFileResolver
     : Luau::FileResolver

diff --git a/src/operations/Completion.cpp b/src/operations/Completion.cpp
@@ -542,9 +542,8 @@ std::vector<lsp::CompletionItem> WorkspaceFolder::completion(const lsp::Completi
                 }
 
                 // Check if it starts with a directory alias, otherwise resolve with require base path
-                std::filesystem::path currentDirectory =
-                    resolveDirectoryAlias(rootUri.fsPath(), config.require.directoryAliases, contentsString, /* includeExtension = */ false)
-                        .value_or(fileResolver.getRequireBasePath(moduleName).append(contentsString));
+                std::filesystem::path currentDirectory = resolveDirectoryAlias(rootUri.fsPath(), config.require.directoryAliases, contentsString)
+                                                             .value_or(fileResolver.getRequireBasePath(moduleName).append(contentsString));
 
                 try
                 {

diff --git a/tests/WorkspaceFileResolver.test.cpp b/tests/WorkspaceFileResolver.test.cpp
@@ -91,16 +91,20 @@ TEST_CASE_FIXTURE(Fixture, "resolveDirectoryAliases")
         {"@test1/", "C:/Users/test/test1"},
         {"@test2/", "~/test2"},
         {"@relative/", "src/client"},
+        {"@test4", "C:/Users/test/test1"},
     };
 
     auto home = getHomeDirectory();
     REQUIRE(home);
 
-    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test1/foo"), "C:/Users/test/test1/foo.lua");
+    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test1/foo"), "C:/Users/test/test1/foo");
     CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test1/foo.luau"), "C:/Users/test/test1/foo.luau");
-    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test1/", /* includeExtension = */ false), "C:/Users/test/test1");
+    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test1/"), "C:/Users/test/test1");
+    // NOTE: we do not strip `/` from `@test1`, so we use it as `@test4`
+    // for now we don't "fix" this, because our startsWith check is greedy, so we want to allow differentiation between `@foo/` and `@foobar/`
+    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test4"), "C:/Users/test/test1");
 
-    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test2/bar"), home.value() / "test2" / "bar.lua");
+    CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test2/bar"), home.value() / "test2" / "bar");
 
     CHECK_EQ(resolveDirectoryAlias("", directoryAliases, "@test3/bar"), std::nullopt);
 
@@ -130,4 +134,4 @@ TEST_CASE_FIXTURE(Fixture, "string require doesn't replace a non-luau/lua extens
     CHECK_EQ(resolved->name, "/Module.mod.lua");
 }
 
-TEST_SUITE_END();
+TEST_SUITE_END();