Use overloaded documentation symbol for class methods/table props (#724)

Right now `getDocumentationSymbol` on an overloaded class method or
table prop just gives the prop name, not prepended with `/overload/ +
toString(overload)`.

This causes the incorrect symbol to be used for `table.insert`,
`BrickColor.new`, `Color3.new` etc. (present in
https://raw.githubusercontent.com/MaximumADHD/Roblox-Client-Tracker/roblox/api-docs/en-us.json)

Co-authored-by: vegorov-rbx <75688451+vegorov-rbx@users.noreply.github.com>
This commit is contained in:
JohnnyMorganz 2022-10-26 00:22:19 +01:00 committed by GitHub
parent e64d489e57
commit ad7d006fb2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 100 additions and 27 deletions

View File

@ -427,6 +427,36 @@ ExprOrLocal findExprOrLocalAtPosition(const SourceModule& source, Position pos)
return findVisitor.result; return findVisitor.result;
} }
static std::optional<DocumentationSymbol> checkOverloadedDocumentationSymbol(
const Module& module, const TypeId ty, const AstExpr* parentExpr, const std::optional<DocumentationSymbol> documentationSymbol)
{
if (!documentationSymbol)
return std::nullopt;
// This might be an overloaded function.
if (get<IntersectionTypeVar>(follow(ty)))
{
TypeId matchingOverload = nullptr;
if (parentExpr && parentExpr->is<AstExprCall>())
{
if (auto it = module.astOverloadResolvedTypes.find(parentExpr))
{
matchingOverload = *it;
}
}
if (matchingOverload)
{
std::string overloadSymbol = *documentationSymbol + "/overload/";
// Default toString options are fine for this purpose.
overloadSymbol += toString(matchingOverload);
return overloadSymbol;
}
}
return documentationSymbol;
}
std::optional<DocumentationSymbol> getDocumentationSymbolAtPosition(const SourceModule& source, const Module& module, Position position) std::optional<DocumentationSymbol> getDocumentationSymbolAtPosition(const SourceModule& source, const Module& module, Position position)
{ {
std::vector<AstNode*> ancestry = findAstAncestryOfPosition(source, position); std::vector<AstNode*> ancestry = findAstAncestryOfPosition(source, position);
@ -436,31 +466,7 @@ std::optional<DocumentationSymbol> getDocumentationSymbolAtPosition(const Source
if (std::optional<Binding> binding = findBindingAtPosition(module, source, position)) if (std::optional<Binding> binding = findBindingAtPosition(module, source, position))
{ {
if (binding->documentationSymbol) return checkOverloadedDocumentationSymbol(module, binding->typeId, parentExpr, binding->documentationSymbol);
{
// This might be an overloaded function binding.
if (get<IntersectionTypeVar>(follow(binding->typeId)))
{
TypeId matchingOverload = nullptr;
if (parentExpr && parentExpr->is<AstExprCall>())
{
if (auto it = module.astOverloadResolvedTypes.find(parentExpr))
{
matchingOverload = *it;
}
}
if (matchingOverload)
{
std::string overloadSymbol = *binding->documentationSymbol + "/overload/";
// Default toString options are fine for this purpose.
overloadSymbol += toString(matchingOverload);
return overloadSymbol;
}
}
}
return binding->documentationSymbol;
} }
if (targetExpr) if (targetExpr)
@ -474,14 +480,14 @@ std::optional<DocumentationSymbol> getDocumentationSymbolAtPosition(const Source
{ {
if (auto propIt = ttv->props.find(indexName->index.value); propIt != ttv->props.end()) if (auto propIt = ttv->props.find(indexName->index.value); propIt != ttv->props.end())
{ {
return propIt->second.documentationSymbol; return checkOverloadedDocumentationSymbol(module, propIt->second.type, parentExpr, propIt->second.documentationSymbol);
} }
} }
else if (const ClassTypeVar* ctv = get<ClassTypeVar>(parentTy)) else if (const ClassTypeVar* ctv = get<ClassTypeVar>(parentTy))
{ {
if (auto propIt = ctv->props.find(indexName->index.value); propIt != ctv->props.end()) if (auto propIt = ctv->props.find(indexName->index.value); propIt != ctv->props.end())
{ {
return propIt->second.documentationSymbol; return checkOverloadedDocumentationSymbol(module, propIt->second.type, parentExpr, propIt->second.documentationSymbol);
} }
} }
} }

View File

@ -72,6 +72,73 @@ TEST_CASE_FIXTURE(DocumentationSymbolFixture, "overloaded_fn")
CHECK_EQ(symbol, "@test/global/foo/overload/(string) -> number"); CHECK_EQ(symbol, "@test/global/foo/overload/(string) -> number");
} }
TEST_CASE_FIXTURE(DocumentationSymbolFixture, "class_method")
{
loadDefinition(R"(
declare class Foo
function bar(self, x: string): number
end
)");
std::optional<DocumentationSymbol> symbol = getDocSymbol(R"(
local x: Foo
x:bar("asdf")
)",
Position(2, 11));
CHECK_EQ(symbol, "@test/globaltype/Foo.bar");
}
TEST_CASE_FIXTURE(DocumentationSymbolFixture, "overloaded_class_method")
{
loadDefinition(R"(
declare class Foo
function bar(self, x: string): number
function bar(self, x: number): string
end
)");
std::optional<DocumentationSymbol> symbol = getDocSymbol(R"(
local x: Foo
x:bar("asdf")
)",
Position(2, 11));
CHECK_EQ(symbol, "@test/globaltype/Foo.bar/overload/(Foo, string) -> number");
}
TEST_CASE_FIXTURE(DocumentationSymbolFixture, "table_function_prop")
{
loadDefinition(R"(
declare Foo: {
new: (number) -> string
}
)");
std::optional<DocumentationSymbol> symbol = getDocSymbol(R"(
Foo.new("asdf")
)",
Position(1, 13));
CHECK_EQ(symbol, "@test/global/Foo.new");
}
TEST_CASE_FIXTURE(DocumentationSymbolFixture, "table_overloaded_function_prop")
{
loadDefinition(R"(
declare Foo: {
new: ((number) -> string) & ((string) -> number)
}
)");
std::optional<DocumentationSymbol> symbol = getDocSymbol(R"(
Foo.new("asdf")
)",
Position(1, 13));
CHECK_EQ(symbol, "@test/global/Foo.new/overload/(string) -> number");
}
TEST_SUITE_END(); TEST_SUITE_END();
TEST_SUITE_BEGIN("AstQuery"); TEST_SUITE_BEGIN("AstQuery");