From 12dac2f1f4f48aa587f4162a0a7ec34f4a37ee87 Mon Sep 17 00:00:00 2001 From: Jack <85714123+jackdotink@users.noreply.github.com> Date: Tue, 25 Mar 2025 18:18:22 -0500 Subject: [PATCH] fix parsing string union indexers (#1750) Today this code results in a syntax error: `type foo = { ["bar" | "baz"]: number }`. This is odd and I believe it is a bug. I have fixed this so that it is now parsed as an indexer field with a union type. This change should not affect the way any code is parsed today, and allow types in indexers to begin with string literals. --------- Co-authored-by: ariel --- Ast/include/Luau/Parser.h | 4 +- Ast/src/Parser.cpp | 528 ++++++++++++++++++++++++++------------ tests/Parser.test.cpp | 6 + 3 files changed, 379 insertions(+), 159 deletions(-) diff --git a/Ast/include/Luau/Parser.h b/Ast/include/Luau/Parser.h index 22137832..c82ad5b9 100644 --- a/Ast/include/Luau/Parser.h +++ b/Ast/include/Luau/Parser.h @@ -228,9 +228,9 @@ private: Position colonPosition; }; - TableIndexerResult parseTableIndexer(AstTableAccess access, std::optional accessLocation); + TableIndexerResult parseTableIndexer(AstTableAccess access, std::optional accessLocation, Lexeme begin); // Remove with FFlagLuauStoreCSTData - AstTableIndexer* parseTableIndexer_DEPRECATED(AstTableAccess access, std::optional accessLocation); + AstTableIndexer* parseTableIndexer_DEPRECATED(AstTableAccess access, std::optional accessLocation, Lexeme begin); AstTypeOrPack parseFunctionType(bool allowPack, const AstArray& attributes); AstType* parseFunctionTypeTail( diff --git a/Ast/src/Parser.cpp b/Ast/src/Parser.cpp index 474efd6a..a3608577 100644 --- a/Ast/src/Parser.cpp +++ b/Ast/src/Parser.cpp @@ -28,6 +28,7 @@ LUAU_FASTFLAGVARIABLE(LuauAstTypeGroup3) LUAU_FASTFLAGVARIABLE(ParserNoErrorLimit) LUAU_FASTFLAGVARIABLE(LuauFixDoBlockEndLocation) LUAU_FASTFLAGVARIABLE(LuauParseOptionalAsNode) +LUAU_FASTFLAGVARIABLE(LuauParseStringIndexer) namespace Luau { @@ -1260,75 +1261,158 @@ AstStat* Parser::parseDeclaration(const Location& start, const AstArray> chars = parseCharArray(); - - const Location nameEnd = lexer.previousLocation(); - - expectMatchAndConsume(']', begin); - expectAndConsume(':', "property type annotation"); - AstType* type = parseType(); - - // since AstName contains a char*, it can't contain null - bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); - - if (chars && !containsNull) + // There are two possibilities: Either it's a property or a function. + if (lexer.current().type == Lexeme::ReservedFunction) { - props.push_back(AstDeclaredClassProp{ - AstName(chars->data), Location(nameBegin, nameEnd), type, false, Location(begin.location, lexer.previousLocation()) - }); + props.push_back(parseDeclaredClassMethod()); + } + else if (lexer.current().type == '[') + { + const Lexeme begin = lexer.current(); + nextLexeme(); // [ + + if ((lexer.current().type == Lexeme::RawString || lexer.current().type == Lexeme::QuotedString) && lexer.lookahead().type == ']') + { + const Location nameBegin = lexer.current().location; + std::optional> chars = parseCharArray(); + + const Location nameEnd = lexer.previousLocation(); + + expectMatchAndConsume(']', begin); + expectAndConsume(':', "property type annotation"); + AstType* type = parseType(); + + // since AstName contains a char*, it can't contain null + bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); + + if (chars && !containsNull) + { + props.push_back(AstDeclaredClassProp{ + AstName(chars->data), Location(nameBegin, nameEnd), type, false, Location(begin.location, lexer.previousLocation()) + }); + } + else + { + report(begin.location, "String literal contains malformed escape sequence or \\0"); + } + } + else + { + if (indexer) + { + // maybe we don't need to parse the entire badIndexer... + // however, we either have { or [ to lint, not the entire table type or the bad indexer. + AstTableIndexer* badIndexer; + if (FFlag::LuauStoreCSTData) + badIndexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt, begin).node; + else + badIndexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt, begin); + + // we lose all additional indexer expressions from the AST after error recovery here + report(badIndexer->location, "Cannot have more than one class indexer"); + } + else + { + if (FFlag::LuauStoreCSTData) + indexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt, begin).node; + else + indexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt, begin); + } + } } else { - report(begin.location, "String literal contains malformed escape sequence or \\0"); - } - } - else if (lexer.current().type == '[') - { - if (indexer) - { - // maybe we don't need to parse the entire badIndexer... - // however, we either have { or [ to lint, not the entire table type or the bad indexer. - AstTableIndexer* badIndexer; - if (FFlag::LuauStoreCSTData) - badIndexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt).node; - else - badIndexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt); + Location propStart = lexer.current().location; + std::optional propName = parseNameOpt("property name"); - // we lose all additional indexer expressions from the AST after error recovery here - report(badIndexer->location, "Cannot have more than one class indexer"); - } - else - { - if (FFlag::LuauStoreCSTData) - indexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt).node; - else - indexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt); + if (!propName) + break; + + expectAndConsume(':', "property type annotation"); + AstType* propType = parseType(); + props.push_back( + AstDeclaredClassProp{propName->name, propName->location, propType, false, Location(propStart, lexer.previousLocation())} + ); } } else { - Location propStart = lexer.current().location; - std::optional propName = parseNameOpt("property name"); + // There are two possibilities: Either it's a property or a function. + if (lexer.current().type == Lexeme::ReservedFunction) + { + props.push_back(parseDeclaredClassMethod()); + } + else if (lexer.current().type == '[' && (lexer.lookahead().type == Lexeme::RawString || lexer.lookahead().type == Lexeme::QuotedString)) + { + const Lexeme begin = lexer.current(); + nextLexeme(); // [ - if (!propName) - break; + const Location nameBegin = lexer.current().location; + std::optional> chars = parseCharArray(); - expectAndConsume(':', "property type annotation"); - AstType* propType = parseType(); - props.push_back( - AstDeclaredClassProp{propName->name, propName->location, propType, false, Location(propStart, lexer.previousLocation())} - ); + const Location nameEnd = lexer.previousLocation(); + + expectMatchAndConsume(']', begin); + expectAndConsume(':', "property type annotation"); + AstType* type = parseType(); + + // since AstName contains a char*, it can't contain null + bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); + + if (chars && !containsNull) + { + props.push_back(AstDeclaredClassProp{ + AstName(chars->data), Location(nameBegin, nameEnd), type, false, Location(begin.location, lexer.previousLocation()) + }); + } + else + { + report(begin.location, "String literal contains malformed escape sequence or \\0"); + } + } + else if (lexer.current().type == '[') + { + if (indexer) + { + // maybe we don't need to parse the entire badIndexer... + // however, we either have { or [ to lint, not the entire table type or the bad indexer. + AstTableIndexer* badIndexer; + if (FFlag::LuauStoreCSTData) + // the last param in the parseTableIndexer is ignored since FFlagLuauParseStringIndexer is false here + badIndexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt, lexer.current()).node; + else + // the last param in the parseTableIndexer is ignored since FFlagLuauParseStringIndexer is false here + badIndexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt, lexer.current()); + + // we lose all additional indexer expressions from the AST after error recovery here + report(badIndexer->location, "Cannot have more than one class indexer"); + } + else + { + if (FFlag::LuauStoreCSTData) + // the last param in the parseTableIndexer is ignored since FFlagLuauParseStringIndexer is false here + indexer = parseTableIndexer(AstTableAccess::ReadWrite, std::nullopt, lexer.current()).node; + else + // the last param in the parseTableIndexer is ignored since FFlagLuauParseStringIndexer is false here + indexer = parseTableIndexer_DEPRECATED(AstTableAccess::ReadWrite, std::nullopt, lexer.current()); + } + } + else + { + Location propStart = lexer.current().location; + std::optional propName = parseNameOpt("property name"); + + if (!propName) + break; + + expectAndConsume(':', "property type annotation"); + AstType* propType = parseType(); + props.push_back( + AstDeclaredClassProp{propName->name, propName->location, propType, false, Location(propStart, lexer.previousLocation())} + ); + } } } @@ -1861,10 +1945,13 @@ std::pair Parser::extractString } // TableIndexer ::= `[' Type `]' `:' Type -Parser::TableIndexerResult Parser::parseTableIndexer(AstTableAccess access, std::optional accessLocation) +Parser::TableIndexerResult Parser::parseTableIndexer(AstTableAccess access, std::optional accessLocation, Lexeme begin) { - const Lexeme begin = lexer.current(); - nextLexeme(); // [ + if (!FFlag::LuauParseStringIndexer) + { + begin = lexer.current(); + nextLexeme(); // [ + } AstType* index = parseType(); @@ -1885,10 +1972,13 @@ Parser::TableIndexerResult Parser::parseTableIndexer(AstTableAccess access, std: } // Remove with FFlagLuauStoreCSTData -AstTableIndexer* Parser::parseTableIndexer_DEPRECATED(AstTableAccess access, std::optional accessLocation) +AstTableIndexer* Parser::parseTableIndexer_DEPRECATED(AstTableAccess access, std::optional accessLocation, Lexeme begin) { - const Lexeme begin = lexer.current(); - nextLexeme(); // [ + if (!FFlag::LuauParseStringIndexer) + { + begin = lexer.current(); + nextLexeme(); // [ + } AstType* index = parseType(); @@ -1941,116 +2031,240 @@ AstType* Parser::parseTableType(bool inDeclarationContext) } } - if (lexer.current().type == '[' && (lexer.lookahead().type == Lexeme::RawString || lexer.lookahead().type == Lexeme::QuotedString)) + if (FFlag::LuauParseStringIndexer) { - const Lexeme begin = lexer.current(); - nextLexeme(); // [ - - CstExprConstantString::QuoteStyle style; - unsigned int blockDepth = 0; - if (FFlag::LuauStoreCSTData && options.storeCstData) - std::tie(style, blockDepth) = extractStringDetails(); - - AstArray sourceString; - std::optional> chars = parseCharArray(options.storeCstData ? &sourceString : nullptr); - - Position indexerClosePosition = lexer.current().location.begin; - expectMatchAndConsume(']', begin); - Position colonPosition = lexer.current().location.begin; - expectAndConsume(':', "table field"); - - AstType* type = parseType(); - - // since AstName contains a char*, it can't contain null - bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); - - if (chars && !containsNull) + if (lexer.current().type == '[') { - props.push_back(AstTableProp{AstName(chars->data), begin.location, type, access, accessLocation}); + const Lexeme begin = lexer.current(); + nextLexeme(); // [ + + if ((lexer.current().type == Lexeme::RawString || lexer.current().type == Lexeme::QuotedString) && lexer.lookahead().type == ']') + { + CstExprConstantString::QuoteStyle style; + unsigned int blockDepth = 0; + if (FFlag::LuauStoreCSTData && options.storeCstData) + std::tie(style, blockDepth) = extractStringDetails(); + + AstArray sourceString; + std::optional> chars = parseCharArray(options.storeCstData ? &sourceString : nullptr); + + Position indexerClosePosition = lexer.current().location.begin; + expectMatchAndConsume(']', begin); + Position colonPosition = lexer.current().location.begin; + expectAndConsume(':', "table field"); + + AstType* type = parseType(); + + // since AstName contains a char*, it can't contain null + bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); + + if (chars && !containsNull) + { + props.push_back(AstTableProp{AstName(chars->data), begin.location, type, access, accessLocation}); + if (FFlag::LuauStoreCSTData && options.storeCstData) + cstItems.push_back(CstTypeTable::Item{ + CstTypeTable::Item::Kind::StringProperty, + begin.location.begin, + indexerClosePosition, + colonPosition, + tableSeparator(), + lexer.current().location.begin, + allocator.alloc(sourceString, style, blockDepth) + }); + } + else + report(begin.location, "String literal contains malformed escape sequence or \\0"); + } + else + { + if (indexer) + { + // maybe we don't need to parse the entire badIndexer... + // however, we either have { or [ to lint, not the entire table type or the bad indexer. + AstTableIndexer* badIndexer; + if (FFlag::LuauStoreCSTData) + badIndexer = parseTableIndexer(access, accessLocation, begin).node; + else + badIndexer = parseTableIndexer_DEPRECATED(access, accessLocation, begin); + + // we lose all additional indexer expressions from the AST after error recovery here + report(badIndexer->location, "Cannot have more than one table indexer"); + } + else + { + if (FFlag::LuauStoreCSTData) + { + auto tableIndexerResult = parseTableIndexer(access, accessLocation, begin); + indexer = tableIndexerResult.node; + if (options.storeCstData) + cstItems.push_back(CstTypeTable::Item{ + CstTypeTable::Item::Kind::Indexer, + tableIndexerResult.indexerOpenPosition, + tableIndexerResult.indexerClosePosition, + tableIndexerResult.colonPosition, + tableSeparator(), + lexer.current().location.begin, + }); + } + else + { + indexer = parseTableIndexer_DEPRECATED(access, accessLocation, begin); + } + } + } + } + else if (props.empty() && !indexer && !(lexer.current().type == Lexeme::Name && lexer.lookahead().type == ':')) + { + AstType* type = parseType(); + + // array-like table type: {T} desugars into {[number]: T} + isArray = true; + AstType* index = allocator.alloc(type->location, std::nullopt, nameNumber, std::nullopt, type->location); + indexer = allocator.alloc(AstTableIndexer{index, type, type->location, access, accessLocation}); + + break; + } + else + { + std::optional name = parseNameOpt("table field"); + + if (!name) + break; + + Position colonPosition = lexer.current().location.begin; + expectAndConsume(':', "table field"); + + AstType* type = parseType(inDeclarationContext); + + props.push_back(AstTableProp{name->name, name->location, type, access, accessLocation}); if (FFlag::LuauStoreCSTData && options.storeCstData) cstItems.push_back(CstTypeTable::Item{ - CstTypeTable::Item::Kind::StringProperty, - begin.location.begin, - indexerClosePosition, + CstTypeTable::Item::Kind::Property, + Position{0, 0}, + Position{0, 0}, colonPosition, tableSeparator(), - lexer.current().location.begin, - allocator.alloc(sourceString, style, blockDepth) + lexer.current().location.begin }); } - else - report(begin.location, "String literal contains malformed escape sequence or \\0"); - } - else if (lexer.current().type == '[') - { - if (indexer) - { - // maybe we don't need to parse the entire badIndexer... - // however, we either have { or [ to lint, not the entire table type or the bad indexer. - AstTableIndexer* badIndexer; - if (FFlag::LuauStoreCSTData) - badIndexer = parseTableIndexer(access, accessLocation).node; - else - badIndexer = parseTableIndexer_DEPRECATED(access, accessLocation); - - // we lose all additional indexer expressions from the AST after error recovery here - report(badIndexer->location, "Cannot have more than one table indexer"); - } - else - { - if (FFlag::LuauStoreCSTData) - { - auto tableIndexerResult = parseTableIndexer(access, accessLocation); - indexer = tableIndexerResult.node; - if (options.storeCstData) - cstItems.push_back(CstTypeTable::Item{ - CstTypeTable::Item::Kind::Indexer, - tableIndexerResult.indexerOpenPosition, - tableIndexerResult.indexerClosePosition, - tableIndexerResult.colonPosition, - tableSeparator(), - lexer.current().location.begin, - }); - } - else - { - indexer = parseTableIndexer_DEPRECATED(access, accessLocation); - } - } - } - else if (props.empty() && !indexer && !(lexer.current().type == Lexeme::Name && lexer.lookahead().type == ':')) - { - AstType* type = parseType(); - - // array-like table type: {T} desugars into {[number]: T} - isArray = true; - AstType* index = allocator.alloc(type->location, std::nullopt, nameNumber, std::nullopt, type->location); - indexer = allocator.alloc(AstTableIndexer{index, type, type->location, access, accessLocation}); - - break; } else { - std::optional name = parseNameOpt("table field"); + if (lexer.current().type == '[' && (lexer.lookahead().type == Lexeme::RawString || lexer.lookahead().type == Lexeme::QuotedString)) + { + const Lexeme begin = lexer.current(); + nextLexeme(); // [ + + CstExprConstantString::QuoteStyle style; + unsigned int blockDepth = 0; + if (FFlag::LuauStoreCSTData && options.storeCstData) + std::tie(style, blockDepth) = extractStringDetails(); + + AstArray sourceString; + std::optional> chars = parseCharArray(options.storeCstData ? &sourceString : nullptr); + + Position indexerClosePosition = lexer.current().location.begin; + expectMatchAndConsume(']', begin); + Position colonPosition = lexer.current().location.begin; + expectAndConsume(':', "table field"); + + AstType* type = parseType(); + + // since AstName contains a char*, it can't contain null + bool containsNull = chars && (memchr(chars->data, 0, chars->size) != nullptr); + + if (chars && !containsNull) + { + props.push_back(AstTableProp{AstName(chars->data), begin.location, type, access, accessLocation}); + if (FFlag::LuauStoreCSTData && options.storeCstData) + cstItems.push_back(CstTypeTable::Item{ + CstTypeTable::Item::Kind::StringProperty, + begin.location.begin, + indexerClosePosition, + colonPosition, + tableSeparator(), + lexer.current().location.begin, + allocator.alloc(sourceString, style, blockDepth) + }); + } + else + report(begin.location, "String literal contains malformed escape sequence or \\0"); + } + else if (lexer.current().type == '[') + { + if (indexer) + { + // maybe we don't need to parse the entire badIndexer... + // however, we either have { or [ to lint, not the entire table type or the bad indexer. + AstTableIndexer* badIndexer; + if (FFlag::LuauStoreCSTData) + // the last param in the parseTableIndexer is ignored + badIndexer = parseTableIndexer(access, accessLocation, lexer.current()).node; + else + // the last param in the parseTableIndexer is ignored + badIndexer = parseTableIndexer_DEPRECATED(access, accessLocation, lexer.current()); + + // we lose all additional indexer expressions from the AST after error recovery here + report(badIndexer->location, "Cannot have more than one table indexer"); + } + else + { + if (FFlag::LuauStoreCSTData) + { + // the last param in the parseTableIndexer is ignored + auto tableIndexerResult = parseTableIndexer(access, accessLocation, lexer.current()); + indexer = tableIndexerResult.node; + if (options.storeCstData) + cstItems.push_back(CstTypeTable::Item{ + CstTypeTable::Item::Kind::Indexer, + tableIndexerResult.indexerOpenPosition, + tableIndexerResult.indexerClosePosition, + tableIndexerResult.colonPosition, + tableSeparator(), + lexer.current().location.begin, + }); + } + else + { + // the last param in the parseTableIndexer is ignored + indexer = parseTableIndexer_DEPRECATED(access, accessLocation, lexer.current()); + } + } + } + else if (props.empty() && !indexer && !(lexer.current().type == Lexeme::Name && lexer.lookahead().type == ':')) + { + AstType* type = parseType(); + + // array-like table type: {T} desugars into {[number]: T} + isArray = true; + AstType* index = allocator.alloc(type->location, std::nullopt, nameNumber, std::nullopt, type->location); + indexer = allocator.alloc(AstTableIndexer{index, type, type->location, access, accessLocation}); - if (!name) break; + } + else + { + std::optional name = parseNameOpt("table field"); - Position colonPosition = lexer.current().location.begin; - expectAndConsume(':', "table field"); + if (!name) + break; - AstType* type = parseType(inDeclarationContext); + Position colonPosition = lexer.current().location.begin; + expectAndConsume(':', "table field"); - props.push_back(AstTableProp{name->name, name->location, type, access, accessLocation}); - if (FFlag::LuauStoreCSTData && options.storeCstData) - cstItems.push_back(CstTypeTable::Item{ - CstTypeTable::Item::Kind::Property, - Position{0, 0}, - Position{0, 0}, - colonPosition, - tableSeparator(), - lexer.current().location.begin - }); + AstType* type = parseType(inDeclarationContext); + + props.push_back(AstTableProp{name->name, name->location, type, access, accessLocation}); + if (FFlag::LuauStoreCSTData && options.storeCstData) + cstItems.push_back(CstTypeTable::Item{ + CstTypeTable::Item::Kind::Property, + Position{0, 0}, + Position{0, 0}, + colonPosition, + tableSeparator(), + lexer.current().location.begin + }); + } } if (lexer.current().type == ',' || lexer.current().type == ';') diff --git a/tests/Parser.test.cpp b/tests/Parser.test.cpp index 2f4e7be2..cb5ff2d6 100644 --- a/tests/Parser.test.cpp +++ b/tests/Parser.test.cpp @@ -24,6 +24,7 @@ LUAU_FASTFLAG(LuauPreserveUnionIntersectionNodeForLeadingTokenSingleType) LUAU_FASTFLAG(LuauAstTypeGroup3) LUAU_FASTFLAG(LuauFixDoBlockEndLocation) LUAU_FASTFLAG(LuauParseOptionalAsNode) +LUAU_FASTFLAG(LuauParseStringIndexer) namespace { @@ -3938,5 +3939,10 @@ TEST_CASE_FIXTURE(Fixture, "parsing_type_suffix_for_return_type_with_variadic") CHECK(result.errors.size() == 0); } +TEST_CASE_FIXTURE(Fixture, "parsing_string_union_indexers") +{ + ScopedFastFlag _{FFlag::LuauParseStringIndexer, true}; + parse(R"(type foo = { ["bar" | "baz"]: number })"); +} TEST_SUITE_END();