From d9536cecd8cbaae5f6f76af79209418f0a78efaf Mon Sep 17 00:00:00 2001 From: Andy Friesen Date: Fri, 6 Sep 2024 13:34:50 -0700 Subject: [PATCH] Sync to upstream/release/642 (#1385) ## New Solver * The type functions `keyof` and `index` now also walk the inheritance chain when they are used on class types like Roblox instances. --------- Co-authored-by: Aaron Weiss --- Analysis/src/Module.cpp | 3 +- Analysis/src/TypeFunction.cpp | 70 +++++++++++++++++++++++-------- tests/TypeFunction.test.cpp | 28 +++++++++++++ tests/TypeInfer.builtins.test.cpp | 19 ++------- 4 files changed, 85 insertions(+), 35 deletions(-) diff --git a/Analysis/src/Module.cpp b/Analysis/src/Module.cpp index dc6c3fc0..3a049216 100644 --- a/Analysis/src/Module.cpp +++ b/Analysis/src/Module.cpp @@ -15,7 +15,6 @@ #include LUAU_FASTFLAG(LuauSolverV2); -LUAU_FASTFLAGVARIABLE(LuauSkipEmptyInstantiations, false); namespace Luau { @@ -122,7 +121,7 @@ struct ClonePublicInterface : Substitution if (FunctionType* ftv = getMutable(result)) { - if (FFlag::LuauSkipEmptyInstantiations && ftv->generics.empty() && ftv->genericPacks.empty()) + if (ftv->generics.empty() && ftv->genericPacks.empty()) { GenericTypeFinder marker; marker.traverse(result); diff --git a/Analysis/src/TypeFunction.cpp b/Analysis/src/TypeFunction.cpp index cd508368..9d4de8bc 100644 --- a/Analysis/src/TypeFunction.cpp +++ b/Analysis/src/TypeFunction.cpp @@ -1897,7 +1897,30 @@ bool computeKeysOf(TypeId ty, Set& result, DenseHashSet& se return res; } - // this should not be reachable since the type should be a valid tables part from normalization. + if (auto classTy = get(ty)) + { + for (auto [key, _] : classTy->props) + result.insert(key); + + bool res = true; + if (classTy->metatable && !isRaw) + { + // findMetatableEntry demands the ability to emit errors, so we must give it + // the necessary state to do that, even if we intend to just eat the errors. + ErrorVec dummy; + + std::optional mmType = findMetatableEntry(ctx->builtins, dummy, ty, "__index", Location{}); + if (mmType) + res = res && computeKeysOf(*mmType, result, seen, isRaw, ctx); + } + + if (classTy->parent) + res = res && computeKeysOf(follow(*classTy->parent), result, seen, isRaw, ctx); + + return res; + } + + // this should not be reachable since the type should be a valid tables or classes part from normalization. LUAU_ASSERT(false); return false; } @@ -1941,34 +1964,32 @@ TypeFunctionReductionResult keyofFunctionImpl( { LUAU_ASSERT(!normTy->hasTables()); + // seen set for key computation for classes + DenseHashSet seen{{}}; + auto classesIter = normTy->classes.ordering.begin(); auto classesIterEnd = normTy->classes.ordering.end(); - LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check + LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check earlier - auto classTy = get(*classesIter); - if (!classTy) - { - LUAU_ASSERT(false); // this should not be possible according to normalization's spec - return {std::nullopt, true, {}, {}}; - } - - for (auto [key, _] : classTy->props) - keys.insert(key); + // collect all the properties from the first class type + if (!computeKeysOf(*classesIter, keys, seen, isRaw, ctx)) + return {ctx->builtins->stringType, false, {}, {}}; // if it failed, we have a top type! // we need to look at each class to remove any keys that are not common amongst them all while (++classesIter != classesIterEnd) { - auto classTy = get(*classesIter); - if (!classTy) - { - LUAU_ASSERT(false); // this should not be possible according to normalization's spec - return {std::nullopt, true, {}, {}}; - } + seen.clear(); // we'll reuse the same seen set + + Set localKeys{{}}; + + // we can skip to the next class if this one is a top type + if (!computeKeysOf(*classesIter, localKeys, seen, isRaw, ctx)) + continue; for (auto key : keys) { // remove any keys that are not present in each class - if (classTy->props.find(key) == classTy->props.end()) + if (!localKeys.contains(key)) keys.erase(key); } } @@ -2222,6 +2243,19 @@ TypeFunctionReductionResult indexFunctionImpl( if (searchPropsAndIndexer(ty, classTy->props, classTy->indexer, properties, ctx)) continue; // Indexer was found in this class, so we can move on to the next + auto parent = classTy->parent; + bool foundInParent = false; + while (parent && !foundInParent) + { + auto parentClass = get(follow(*parent)); + foundInParent = searchPropsAndIndexer(ty, parentClass->props, parentClass->indexer, properties, ctx); + parent = parentClass->parent; + } + + // we move on to the next type if any of the parents we went through had the property. + if (foundInParent) + continue; + // If code reaches here,that means the property not found -> check in the metatable's __index // findMetatableEntry demands the ability to emit errors, so we must give it diff --git a/tests/TypeFunction.test.cpp b/tests/TypeFunction.test.cpp index 9d7aeb6d..4d500d18 100644 --- a/tests/TypeFunction.test.cpp +++ b/tests/TypeFunction.test.cpp @@ -619,6 +619,20 @@ TEST_CASE_FIXTURE(ClassFixture, "keyof_type_function_common_subset_if_union_of_d LUAU_REQUIRE_NO_ERRORS(result); } +TEST_CASE_FIXTURE(ClassFixture, "keyof_type_function_works_with_parent_classes_too") +{ + if (!FFlag::LuauSolverV2) + return; + + CheckResult result = check(R"( + type KeysOfMyObject = keyof + + local function ok(idx: KeysOfMyObject): "BaseField" | "BaseMethod" | "Method" | "Touched" return idx end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_CASE_FIXTURE(ClassFixture, "binary_type_function_works_with_default_argument") { if (!FFlag::LuauSolverV2) @@ -1033,6 +1047,20 @@ TEST_CASE_FIXTURE(ClassFixture, "index_type_function_works_on_classes") LUAU_REQUIRE_NO_ERRORS(result); } +TEST_CASE_FIXTURE(ClassFixture, "index_type_function_works_on_classes_with_parents") +{ + if (!FFlag::LuauSolverV2) + return; + + CheckResult result = check(R"( + type KeysOfMyObject = index + + local function ok(idx: KeysOfMyObject): number return idx end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_CASE_FIXTURE(BuiltinsFixture, "index_type_function_works_w_index_metatables") { if (!FFlag::LuauSolverV2) diff --git a/tests/TypeInfer.builtins.test.cpp b/tests/TypeInfer.builtins.test.cpp index 16813e86..eba8b09c 100644 --- a/tests/TypeInfer.builtins.test.cpp +++ b/tests/TypeInfer.builtins.test.cpp @@ -793,9 +793,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "select_with_variadic_typepack_tail_and_strin TEST_CASE_FIXTURE(Fixture, "string_format_as_method") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check("local _ = ('%s'):format(5)"); @@ -809,6 +807,7 @@ TEST_CASE_FIXTURE(Fixture, "string_format_as_method") TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument") { + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local _ = ("%s"):format("%d", "hello") )"); @@ -820,10 +819,7 @@ TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument") TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument2") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local _ = ("%s %d").format("%d %s", "A type error", 2) )"); @@ -882,10 +878,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "debug_info_is_crazy") TEST_CASE_FIXTURE(BuiltinsFixture, "aliased_string_format") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local fmt = string.format local s = fmt("%d", "oops") @@ -945,10 +938,6 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "select_on_variadic") TEST_CASE_FIXTURE(BuiltinsFixture, "string_format_report_all_type_errors_at_correct_positions") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( ("%s%d%s"):format(1, "hello", true)