Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require a name for definitions files #885

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [Unreleased]

### Changed

- Definitions files must now provide a name for the file in settings and command line: i.e.
`--definitions:@roblox=path/to/globalTypes.d.luau`. This is to support mapping global types back to their original
definitions file for documentation features. Please update your LSP settings (`luau-lsp.types.definitionFiles`) and command line arguments.
Backwards compatibility has been temporarily preserved, with random names generated.

## [1.38.0] - 2024-12-28

### Added
Expand Down
5 changes: 3 additions & 2 deletions editors/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ $ luau-lsp --help

## Configuring Definitions and Documentation

You can add in built-in definitions by passing the `--definitions=PATH` argument.
You can add in built-in definitions by passing the `--definitions:@name=PATH` argument. The `name` should be a unique
reference to the definitions file.
This can be done multiple times:

```sh
$ luau-lsp lsp --definitions=/path/to/globalTypes.d.luau
$ luau-lsp lsp --definitions:@roblox=/path/to/globalTypes.d.luau
```

> NOTE: Definitions file syntax is unstable and undocumented. It may change at any time
Expand Down
8 changes: 4 additions & 4 deletions editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@
"scope": "resource"
},
"luau-lsp.types.definitionFiles": {
"markdownDescription": "A list of paths to definition files to load in to the type checker. Note that definition file syntax is currently unstable and may change at any time",
"type": "array",
"default": [],
"items": {
"markdownDescription": "A mapping of package names to paths of definition files to load in to the type checker. Note that definition file syntax is currently unstable and may change at any time",
"type": "object",
"default": {},
"additionalProperties": {
"type": "string"
},
"scope": "window"
Expand Down
17 changes: 13 additions & 4 deletions editors/code/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,18 @@ const startLanguageServer = async (context: vscode.ExtensionContext) => {
const typesConfig = vscode.workspace.getConfiguration("luau-lsp.types");

// Load extra type definitions
const definitionFiles = typesConfig.get<string[]>("definitionFiles");
let definitionFiles = typesConfig.get<
{ [packageName: string]: string } | string[]
>("definitionFiles");

if (definitionFiles) {
for (let definitionPath of definitionFiles) {
if (Array.isArray(definitionFiles)) {
definitionFiles = Object.fromEntries(
definitionFiles.map((path, index) => ["roblox" + index, path]),
);
}

for (let [packageName, definitionPath] of Object.entries(definitionFiles)) {
definitionPath = utils.resolvePath(definitionPath);
let uri;
if (vscode.workspace.workspaceFolders) {
Expand All @@ -93,10 +102,10 @@ const startLanguageServer = async (context: vscode.ExtensionContext) => {
uri = vscode.Uri.file(definitionPath);
}
if (await utils.exists(uri)) {
addArg(`--definitions=${uri.fsPath}`);
addArg(`--definitions:${packageName}=${uri.fsPath}`);
} else {
vscode.window.showWarningMessage(
`Definitions file at ${definitionPath} does not exist, types will not be provided from this file`,
`Definitions file '${packageName}' at ${definitionPath} does not exist, types will not be provided from this file`,
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions editors/code/src/roblox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,11 +436,11 @@ export const preLanguageServerStart = async (
typesConfig.get<string>("robloxSecurityLevel") ?? "PluginSecurity";
await updateApiInfo(context);
addArg(
`--definitions=${globalTypesUri(context, securityLevel, "Prod").fsPath}`,
`--definitions:@roblox=${globalTypesUri(context, securityLevel, "Prod").fsPath}`,
"Prod",
);
addArg(
`--definitions=${globalTypesUri(context, securityLevel, "Debug").fsPath}`,
`--definitions:@roblox=${globalTypesUri(context, securityLevel, "Debug").fsPath}`,
"Debug",
);
addArg(`--docs=${apiDocsUri(context).fsPath}`);
Expand Down
40 changes: 37 additions & 3 deletions src/AnalyzeCli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,46 @@ std::vector<std::filesystem::path> getFilesToAnalyze(const std::vector<std::stri
return files;
}


std::unordered_map<std::string, std::filesystem::path> processDefinitionsFilePaths(const argparse::ArgumentParser& program)
{
std::unordered_map<std::string, std::filesystem::path> definitionsFiles{};
size_t backwardsCompatibilityNameSuffix = 0;
for (const auto& definition : program.get<std::vector<std::string>>("--definitions"))
{
std::string packageName = definition;
std::filesystem::path filePath = definition;

size_t eqIndex = definition.find('=');
if (eqIndex == std::string::npos)
{
// TODO: Remove Me - backwards compatibility
packageName = "@roblox";
if (backwardsCompatibilityNameSuffix > 0)
packageName += std::to_string(backwardsCompatibilityNameSuffix);
backwardsCompatibilityNameSuffix += 1;
}
else
{
packageName = definition.substr(0, eqIndex);
filePath = definition.substr(eqIndex + 1, definition.length());
}

if (!Luau::startsWith(packageName, "@"))
packageName = "@" + packageName;

definitionsFiles.emplace(packageName, filePath);
}

return definitionsFiles;
}

int startAnalyze(const argparse::ArgumentParser& program)
{
ReportFormat format = ReportFormat::Default;
bool annotate = program.is_used("--annotate");
auto sourcemapPath = program.present<std::filesystem::path>("--sourcemap");
auto definitionsPaths = program.get<std::vector<std::filesystem::path>>("--definitions");
auto definitionsPaths = processDefinitionsFilePaths(program);
auto ignoreGlobPatterns = program.get<std::vector<std::string>>("--ignore");
auto baseLuaurc = program.present<std::filesystem::path>("--base-luaurc");
auto settingsPath = program.present<std::filesystem::path>("--settings");
Expand Down Expand Up @@ -320,7 +354,7 @@ int startAnalyze(const argparse::ArgumentParser& program)
if (!FFlag::LuauSolverV2)
Luau::registerBuiltinGlobals(frontend, frontend.globalsForAutocomplete);

for (auto& definitionsPath : definitionsPaths)
for (const auto& [packageName, definitionsPath] : definitionsPaths)
{
if (!std::filesystem::exists(definitionsPath))
{
Expand All @@ -335,7 +369,7 @@ int startAnalyze(const argparse::ArgumentParser& program)
return 1;
}

auto loadResult = types::registerDefinitions(frontend, frontend.globals, *definitionsContents);
auto loadResult = types::registerDefinitions(frontend, frontend.globals, packageName, *definitionsContents);
if (!loadResult.success)
{
fprintf(stderr, "Failed to load definitions\n");
Expand Down
6 changes: 3 additions & 3 deletions src/LuauExt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ std::optional<nlohmann::json> parseDefinitionsFileMetadata(const std::string& de
return std::nullopt;
}

Luau::LoadDefinitionFileResult registerDefinitions(Luau::Frontend& frontend, Luau::GlobalTypes& globals, const std::string& definitions)
Luau::LoadDefinitionFileResult registerDefinitions(
Luau::Frontend& frontend, Luau::GlobalTypes& globals, const std::string& packageName, const std::string& definitions)
{
// TODO: packageName shouldn't just be "@roblox"
return frontend.loadDefinitionFile(globals, globals.globalScope, definitions, "@roblox", /* captureComments = */ false);
return frontend.loadDefinitionFile(globals, globals.globalScope, definitions, packageName, /* captureComments = */ false);
}

using NameOrExpr = std::variant<std::string, Luau::AstExpr*>;
Expand Down
10 changes: 5 additions & 5 deletions src/Workspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ bool WorkspaceFolder::isDefinitionFile(const std::filesystem::path& path, const
auto config = givenConfig ? *givenConfig : client->getConfiguration(rootUri);
auto canonicalised = std::filesystem::weakly_canonical(path);

for (auto& file : config.types.definitionFiles)
for (const auto& [_, file] : client->definitionsFiles)
{
if (std::filesystem::weakly_canonical(resolvePath(file)) == canonicalised)
{
Expand Down Expand Up @@ -313,10 +313,10 @@ void WorkspaceFolder::registerTypes()
if (client->definitionsFiles.empty())
client->sendLogMessage(lsp::MessageType::Warning, "No definitions file provided by client");

for (const auto& definitionsFile : client->definitionsFiles)
for (const auto& [packageName, definitionsFile] : client->definitionsFiles)
{
auto resolvedFilePath = resolvePath(definitionsFile);
client->sendLogMessage(lsp::MessageType::Info, "Loading definitions file: " + resolvedFilePath.generic_string());
client->sendLogMessage(lsp::MessageType::Info, "Loading definitions file: " + packageName + " - " + resolvedFilePath.generic_string());

auto definitionsContents = readFile(resolvedFilePath);
if (!definitionsContents)
Expand All @@ -334,9 +334,9 @@ void WorkspaceFolder::registerTypes()
client->sendTrace("workspace initialization: parsing definitions file metadata COMPLETED", json(definitionsFileMetadata).dump());

client->sendTrace("workspace initialization: registering types definition");
auto result = types::registerDefinitions(frontend, frontend.globals, *definitionsContents);
auto result = types::registerDefinitions(frontend, frontend.globals, packageName, *definitionsContents);
if (!FFlag::LuauSolverV2)
types::registerDefinitions(frontend, frontend.globalsForAutocomplete, *definitionsContents);
types::registerDefinitions(frontend, frontend.globalsForAutocomplete, packageName, *definitionsContents);
client->sendTrace("workspace initialization: registering types definition COMPLETED");

client->sendTrace("workspace: applying platform mutations on definitions");
Expand Down
1 change: 1 addition & 0 deletions src/include/Analyze/AnalyzeCli.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
#include "argparse/argparse.hpp"

std::vector<std::filesystem::path> getFilesToAnalyze(const std::vector<std::string>& paths, const std::vector<std::string>& ignoreGlobPatterns);
std::unordered_map<std::string, std::filesystem::path> processDefinitionsFilePaths(const argparse::ArgumentParser& program);
int startAnalyze(const argparse::ArgumentParser& program);
2 changes: 1 addition & 1 deletion src/include/LSP/Client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Client : public BaseClient
lsp::ClientCapabilities capabilities;
lsp::TraceValue traceMode = lsp::TraceValue::Off;
/// A registered definitions file passed by the client
std::vector<std::filesystem::path> definitionsFiles{};
std::unordered_map<std::string, std::filesystem::path> definitionsFiles{};
/// A registered documentation file passed by the client
std::vector<std::filesystem::path> documentationFiles{};
/// Parsed documentation database
Expand Down
6 changes: 3 additions & 3 deletions src/include/LSP/ClientConfiguration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ struct ClientTypesConfiguration
/// Whether Roblox-related definitions should be supported
/// DEPRECATED: USE `platform.type` INSTEAD
bool roblox = true;
/// Any definition files to load globally
std::vector<std::filesystem::path> definitionFiles{};
// TODO: commented out due to backwards compatibility. uncomment if needed later once all settings have converted
// std::unordered_map<std::string, std::filesystem::path> definitionFiles{};
};
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ClientTypesConfiguration, roblox, definitionFiles);
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(ClientTypesConfiguration, roblox); // definitionFiles

enum struct InlayHintsParameterNamesConfig
{
Expand Down
3 changes: 2 additions & 1 deletion src/include/LSP/LuauExt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ bool isMetamethod(const Luau::Name& name);

std::optional<nlohmann::json> parseDefinitionsFileMetadata(const std::string& definitions);

Luau::LoadDefinitionFileResult registerDefinitions(Luau::Frontend& frontend, Luau::GlobalTypes& globals, const std::string& definitions);
Luau::LoadDefinitionFileResult registerDefinitions(
Luau::Frontend& frontend, Luau::GlobalTypes& globals, const std::string& packageName, const std::string& definitions);

using NameOrExpr = std::variant<std::string, Luau::AstExpr*>;

Expand Down
14 changes: 7 additions & 7 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ int startLanguageServer(const argparse::ArgumentParser& program)
_setmode(_fileno(stdout), _O_BINARY);
#endif

auto definitionsFiles = program.get<std::vector<std::filesystem::path>>("--definitions");
auto definitionsFiles = processDefinitionsFilePaths(program);
auto documentationFiles = program.get<std::vector<std::filesystem::path>>("--docs");
std::optional<std::filesystem::path> baseLuaurc = program.present<std::filesystem::path>("--base-luaurc");

Expand Down Expand Up @@ -163,6 +163,7 @@ int main(int argc, char** argv)

// Analyze arguments
argparse::ArgumentParser analyze_command("analyze");
analyze_command.set_assign_chars(":=");
analyze_command.add_description("Run luau-analyze type checking and linting");
analyze_command.add_parents(parent_parser);
analyze_command.add_argument("--annotate")
Expand All @@ -187,10 +188,9 @@ int main(int argc, char** argv)
.metavar("PATH");
analyze_command.add_argument("--definitions", "--defs")
.help("A path to a Luau definitions file to load into the global namespace")
.action(file_path_parser)
.default_value<std::vector<std::filesystem::path>>({})
.default_value<std::vector<std::string>>({})
.append()
.metavar("PATH");
.metavar("@NAME=PATH");
analyze_command.add_argument("--ignore")
.help("file glob pattern for ignoring error outputs")
.default_value<std::vector<std::string>>({})
Expand All @@ -206,15 +206,15 @@ int main(int argc, char** argv)

// Language server arguments
argparse::ArgumentParser lsp_command("lsp");
lsp_command.set_assign_chars(":=");
lsp_command.add_description("Start the language server");
lsp_command.add_epilog("This will start up a server which listens to LSP messages on stdin, and responds on stdout");
lsp_command.add_parents(parent_parser);
lsp_command.add_argument("--definitions")
.help("path to a Luau definitions file to load into the global namespace")
.action(file_path_parser)
.default_value<std::vector<std::filesystem::path>>({})
.default_value<std::vector<std::string>>({})
.append()
.metavar("PATH");
.metavar("@NAME=PATH");
lsp_command.add_argument("--docs", "--documentation")
.help("path to a Luau documentation database for loaded definitions")
.action(file_path_parser)
Expand Down
44 changes: 44 additions & 0 deletions tests/AnalyzeCli.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,48 @@ TEST_CASE("getFilesToAnalyze_still_matches_file_if_it_was_explicitly_provided")
CHECK_EQ(getFilesToAnalyze({fileA}, {"a.luau"}), std::vector<std::filesystem::path>{fileA});
}

TEST_CASE("parse_definitions_files_handles_new_syntax")
{
argparse::ArgumentParser program("test");
program.set_assign_chars(":=");
program.add_argument("--definitions", "--defs")
.help("A path to a Luau definitions file to load into the global namespace")
.default_value<std::vector<std::string>>({})
.append()
.metavar("PATH");

std::vector<std::string> arguments{
"", "--definitions:@roblox=example_path.d.luau", "--definitions:@lune=lune.d.luau", "--definitions:no_at_sign=path.d.luau"};
program.parse_args(arguments);

auto definitionsFiles = processDefinitionsFilePaths(program);

CHECK_EQ(definitionsFiles, std::unordered_map<std::string, std::filesystem::path>{
{"@roblox", "example_path.d.luau"},
{"@lune", "lune.d.luau"},
{"@no_at_sign", "path.d.luau"},
});
}

TEST_CASE("parse_definitions_files_handles_legacy_syntax")
{
argparse::ArgumentParser program("test");
program.set_assign_chars(":=");
program.add_argument("--definitions", "--defs")
.help("A path to a Luau definitions file to load into the global namespace")
.default_value<std::vector<std::string>>({})
.append()
.metavar("PATH");

std::vector<std::string> arguments{"", "--definitions=example_path.d.luau", "--definitions=lune.d.luau"};
program.parse_args(arguments);

auto definitionsFiles = processDefinitionsFilePaths(program);

CHECK_EQ(definitionsFiles, std::unordered_map<std::string, std::filesystem::path>{
{"@roblox", "example_path.d.luau"},
{"@roblox1", "lune.d.luau"},
});
}

TEST_SUITE_END();
36 changes: 32 additions & 4 deletions tests/Definitions.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ TEST_CASE("use_platform_metadata_from_first_registered_definitions_file")
auto client = std::make_shared<Client>(Client{});
auto workspace = WorkspaceFolder(client, "$TEST_WORKSPACE", Uri(), std::nullopt);

client->definitionsFiles.emplace_back("./tests/testdata/standard_definitions.d.luau");
client->definitionsFiles.emplace_back("./tests/testdata/extra_definitions_relying_on_mutations.d.luau");
client->definitionsFiles.emplace("@roblox", "./tests/testdata/standard_definitions.d.luau");
client->definitionsFiles.emplace("@roblox1", "./tests/testdata/extra_definitions_relying_on_mutations.d.luau");

workspace.setupWithConfiguration(defaultTestClientConfiguration());

Expand All @@ -28,8 +28,8 @@ TEST_CASE("handles_definitions_files_relying_on_mutations")
auto client = std::make_shared<Client>(Client{});
auto workspace = WorkspaceFolder(client, "$TEST_WORKSPACE", Uri(), std::nullopt);

client->definitionsFiles.emplace_back("./tests/testdata/standard_definitions.d.luau");
client->definitionsFiles.emplace_back("./tests/testdata/extra_definitions_relying_on_mutations.d.luau");
client->definitionsFiles.emplace("@roblox", "./tests/testdata/standard_definitions.d.luau");
client->definitionsFiles.emplace("@roblox1", "./tests/testdata/extra_definitions_relying_on_mutations.d.luau");

workspace.setupWithConfiguration(defaultTestClientConfiguration());

Expand All @@ -42,4 +42,32 @@ TEST_CASE("handles_definitions_files_relying_on_mutations")
REQUIRE(result.errors.empty());
}

TEST_CASE("package_name_is_recorded_onto_the_loaded_types")
{
auto client = std::make_shared<Client>(Client{});
auto workspace = WorkspaceFolder(client, "$TEST_WORKSPACE", Uri(), std::nullopt);

client->definitionsFiles.emplace("@example", "./tests/testdata/standard_definitions.d.luau");

workspace.setupWithConfiguration(defaultTestClientConfiguration());

auto document = newDocument(workspace, "foo.luau", R"(
local x: Instance
)");

auto result = workspace.frontend.check("foo.luau");
auto module = workspace.frontend.moduleResolver.getModule("foo.luau");
REQUIRE(module);

auto binding = module->getModuleScope()->linearSearchForBinding("x");
REQUIRE(binding);

auto ty = Luau::follow(binding->typeId);
CHECK_EQ(ty->documentationSymbol, "@example/globaltype/Instance");

auto ctv = Luau::get<Luau::ClassType>(ty);
REQUIRE(ctv);
CHECK_EQ(ctv->definitionModuleName, "@example");
}

TEST_SUITE_END();
Loading
Loading