Fix string interpolation autocomplete and location (#748)

String interpolation autocomplete was not working and was just using default autocomplete. This fixes it so that inside a string it is treated as normal, and inside an expression it suggests expression level autocomplete. Also fixes the range, which was previously just the first lexeme.
luau-lang · Nov 16, 2022 · aa7c645 · aa7c645
1 parent 816e41a
commit aa7c645
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 17 deletions.
diff --git a/Analysis/src/Autocomplete.cpp b/Analysis/src/Autocomplete.cpp
@@ -1219,6 +1219,31 @@ static std::optional<const ClassTypeVar*> getMethodContainingClass(const ModuleP
     return std::nullopt;
 }
 
+static bool stringPartOfInterpString(const AstNode* node, Position position)
+{
+    const AstExprInterpString* interpString = node->as<AstExprInterpString>();
+    if (!interpString)
+    {
+        return false;
+    }
+
+    for (const AstExpr* expression : interpString->expressions)
+    {
+        if (expression->location.containsClosed(position))
+        {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+static bool isSimpleInterpolatedString(const AstNode* node)
+{
+    const AstExprInterpString* interpString = node->as<AstExprInterpString>();
+    return interpString != nullptr && interpString->expressions.size == 0;
+}
+
 static std::optional<AutocompleteEntryMap> autocompleteStringParams(const SourceModule& sourceModule, const ModulePtr& module,
     const std::vector<AstNode*>& nodes, Position position, StringCompletionCallback callback)
 {
@@ -1227,7 +1252,7 @@ static std::optional<AutocompleteEntryMap> autocompleteStringParams(const Source
         return std::nullopt;
     }
 
-    if (!nodes.back()->is<AstExprConstantString>() && !nodes.back()->is<AstExprError>())
+    if (!nodes.back()->is<AstExprConstantString>() && !isSimpleInterpolatedString(nodes.back()) && !nodes.back()->is<AstExprError>())
     {
         return std::nullopt;
     }
@@ -1432,7 +1457,7 @@ static AutocompleteResult autocomplete(const SourceModule& sourceModule, const M
         return autocompleteExpression(sourceModule, *module, singletonTypes, &typeArena, ancestry, position);
     else if (AstStatRepeat* statRepeat = extractStat<AstStatRepeat>(ancestry); statRepeat)
         return {autocompleteStatement(sourceModule, *module, ancestry, position), ancestry, AutocompleteContext::Statement};
-    else if (AstExprTable* exprTable = parent->as<AstExprTable>(); exprTable && (node->is<AstExprGlobal>() || node->is<AstExprConstantString>()))
+    else if (AstExprTable* exprTable = parent->as<AstExprTable>(); exprTable && (node->is<AstExprGlobal>() || node->is<AstExprConstantString>() || node->is<AstExprInterpString>()))
     {
         for (const auto& [kind, key, value] : exprTable->items)
         {
@@ -1471,7 +1496,7 @@ static AutocompleteResult autocomplete(const SourceModule& sourceModule, const M
     {
         return {*ret, ancestry, AutocompleteContext::String};
     }
-    else if (node->is<AstExprConstantString>())
+    else if (node->is<AstExprConstantString>() || isSimpleInterpolatedString(node))
     {
         AutocompleteEntryMap result;
 
@@ -1497,6 +1522,13 @@ static AutocompleteResult autocomplete(const SourceModule& sourceModule, const M
 
         return {result, ancestry, AutocompleteContext::String};
     }
+    else if (stringPartOfInterpString(node, position))
+    {
+        // We're not a simple interpolated string, we're something like `a{"b"}@1`, and we
+        // can't know what to format to
+        AutocompleteEntryMap map;
+        return {map, ancestry, AutocompleteContext::String};
+    }
 
     if (node->is<AstExprConstantNumber>())
         return {};

diff --git a/Ast/src/Parser.cpp b/Ast/src/Parser.cpp
@@ -2661,23 +2661,24 @@ AstExpr* Parser::parseInterpString()
     TempVector<AstExpr*> expressions(scratchExpr);
 
     Location startLocation = lexer.current().location;
+    Location endLocation;
 
     do
     {
         Lexeme currentLexeme = lexer.current();
         LUAU_ASSERT(currentLexeme.type == Lexeme::InterpStringBegin || currentLexeme.type == Lexeme::InterpStringMid ||
                     currentLexeme.type == Lexeme::InterpStringEnd || currentLexeme.type == Lexeme::InterpStringSimple);
 
-        Location location = currentLexeme.location;
+        endLocation = currentLexeme.location;
 
-        Location startOfBrace = Location(location.end, 1);
+        Location startOfBrace = Location(endLocation.end, 1);
 
         scratchData.assign(currentLexeme.data, currentLexeme.length);
 
         if (!Lexer::fixupQuotedString(scratchData))
         {
             nextLexeme();
-            return reportExprError(startLocation, {}, "Interpolated string literal contains malformed escape sequence");
+            return reportExprError(Location{startLocation, endLocation}, {}, "Interpolated string literal contains malformed escape sequence");
         }
 
         AstArray<char> chars = copy(scratchData);
@@ -2688,15 +2689,36 @@ AstExpr* Parser::parseInterpString()
 
         if (currentLexeme.type == Lexeme::InterpStringEnd || currentLexeme.type == Lexeme::InterpStringSimple)
         {
-            AstArray<AstArray<char>> stringsArray = copy(strings);
-            AstArray<AstExpr*> expressionsArray = copy(expressions);
-
-            return allocator.alloc<AstExprInterpString>(startLocation, stringsArray, expressionsArray);
+            break;
         }
 
-        AstExpr* expression = parseExpr();
+        bool errorWhileChecking = false;
+
+        switch (lexer.current().type)
+        {
+        case Lexeme::InterpStringMid:
+        case Lexeme::InterpStringEnd:
+        {
+            errorWhileChecking = true;
+            nextLexeme();
+            expressions.push_back(reportExprError(endLocation, {}, "Malformed interpolated string, expected expression inside '{}'"));
+            break;
+        }
+        case Lexeme::BrokenString:
+        {
+            errorWhileChecking = true;
+            nextLexeme();
+            expressions.push_back(reportExprError(endLocation, {}, "Malformed interpolated string, did you forget to add a '`'?"));
+            break;
+        }
+        default:
+            expressions.push_back(parseExpr());
+        }
 
-        expressions.push_back(expression);
+        if (errorWhileChecking)
+        {
+            break;
+        }
 
         switch (lexer.current().type)
         {
@@ -2706,14 +2728,18 @@ AstExpr* Parser::parseInterpString()
             break;
         case Lexeme::BrokenInterpDoubleBrace:
             nextLexeme();
-            return reportExprError(location, {}, ERROR_INVALID_INTERP_DOUBLE_BRACE);
+            return reportExprError(endLocation, {}, ERROR_INVALID_INTERP_DOUBLE_BRACE);
         case Lexeme::BrokenString:
             nextLexeme();
-            return reportExprError(location, {}, "Malformed interpolated string, did you forget to add a '}'?");
+            return reportExprError(endLocation, {}, "Malformed interpolated string, did you forget to add a '}'?");
         default:
-            return reportExprError(location, {}, "Malformed interpolated string, got %s", lexer.current().toString().c_str());
+            return reportExprError(endLocation, {}, "Malformed interpolated string, got %s", lexer.current().toString().c_str());
         }
     } while (true);
+
+    AstArray<AstArray<char>> stringsArray = copy(strings);
+    AstArray<AstExpr*> expressionsArray = copy(expressions);
+    return allocator.alloc<AstExprInterpString>(Location{startLocation, endLocation}, stringsArray, expressionsArray);
 }
 
 AstExpr* Parser::parseNumber()

diff --git a/tests/AstJsonEncoder.test.cpp b/tests/AstJsonEncoder.test.cpp
@@ -183,7 +183,7 @@ TEST_CASE_FIXTURE(JsonEncoderFixture, "encode_AstExprInterpString")
     AstStat* statement = expectParseStatement("local a = `var = {x}`");
 
     std::string_view expected =
-        R"({"type":"AstStatLocal","location":"0,0 - 0,18","vars":[{"luauType":null,"name":"a","type":"AstLocal","location":"0,6 - 0,7"}],"values":[{"type":"AstExprInterpString","location":"0,10 - 0,18","strings":["var = ",""],"expressions":[{"type":"AstExprGlobal","location":"0,18 - 0,19","global":"x"}]}]})";
+        R"({"type":"AstStatLocal","location":"0,0 - 0,21","vars":[{"luauType":null,"name":"a","type":"AstLocal","location":"0,6 - 0,7"}],"values":[{"type":"AstExprInterpString","location":"0,10 - 0,21","strings":["var = ",""],"expressions":[{"type":"AstExprGlobal","location":"0,18 - 0,19","global":"x"}]}]})";
 
     CHECK(toJson(statement) == expected);
 }

diff --git a/tests/Autocomplete.test.cpp b/tests/Autocomplete.test.cpp
@@ -7,6 +7,7 @@
 #include "Luau/StringUtils.h"
 
 #include "Fixture.h"
+#include "ScopedFlags.h"
 
 #include "doctest.h"
 
@@ -2708,13 +2709,77 @@ a = if temp then even else abc@3
     CHECK(ac.entryMap.count("abcdef"));
 }
 
-TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string")
+TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string_constant")
 {
+    ScopedFastFlag sff{"LuauInterpolatedStringBaseSupport", true};
+
+    check(R"(f(`@1`))");
+    auto ac = autocomplete('1');
+    CHECK(ac.entryMap.empty());
+    CHECK_EQ(ac.context, AutocompleteContext::String);
+
+    check(R"(f(`@1 {"a"}`))");
+    ac = autocomplete('1');
+    CHECK(ac.entryMap.empty());
+    CHECK_EQ(ac.context, AutocompleteContext::String);
+
+    check(R"(f(`{"a"} @1`))");
+    ac = autocomplete('1');
+    CHECK(ac.entryMap.empty());
+    CHECK_EQ(ac.context, AutocompleteContext::String);
+
+    check(R"(f(`{"a"} @1 {"b"}`))");
+    ac = autocomplete('1');
+    CHECK(ac.entryMap.empty());
+    CHECK_EQ(ac.context, AutocompleteContext::String);
+}
+
+TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string_expression")
+{
+    ScopedFastFlag sff{"LuauInterpolatedStringBaseSupport", true};
+
     check(R"(f(`expression = {@1}`))");
+    auto ac = autocomplete('1');
+    CHECK(ac.entryMap.count("table"));
+    CHECK_EQ(ac.context, AutocompleteContext::Expression);
+}
+
+TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string_expression_with_comments")
+{
+    ScopedFastFlag sff{"LuauInterpolatedStringBaseSupport", true};
+
+    check(R"(f(`expression = {--[[ bla bla bla ]]@1`))");
 
     auto ac = autocomplete('1');
     CHECK(ac.entryMap.count("table"));
     CHECK_EQ(ac.context, AutocompleteContext::Expression);
+
+    check(R"(f(`expression = {@1 --[[ bla bla bla ]]`))");
+    ac = autocomplete('1');
+    CHECK(!ac.entryMap.empty());
+    CHECK(ac.entryMap.count("table"));
+    CHECK_EQ(ac.context, AutocompleteContext::Expression);
+}
+
+TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string_as_singleton")
+{
+    ScopedFastFlag sff{"LuauInterpolatedStringBaseSupport", true};
+
+    check(R"(
+        --!strict
+        local function f(a: "cat" | "dog") end
+
+        f(`@1`)
+        f(`uhhh{'try'}@2`)
+    )");
+
+    auto ac = autocomplete('1');
+    CHECK(ac.entryMap.count("cat"));
+    CHECK_EQ(ac.context, AutocompleteContext::String);
+
+    ac = autocomplete('2');
+    CHECK(ac.entryMap.empty());
+    CHECK_EQ(ac.context, AutocompleteContext::String);
 }
 
 TEST_CASE_FIXTURE(ACFixture, "autocomplete_explicit_type_pack")