[clang-format] Handle quotes and escaped braces in C# interpolated strings

[jbcoe]

Summary

This addresses issues raised in https://bugs.llvm.org/show_bug.cgi?id=44454.

There are outstanding issues with multi-line verbatim strings in C# that will be addressed in a follow-up PR.

[MyDeveloperDay]

This looks good, I had lots of problems when I first started the C# support around this which the initial C# support tried to address by adding support in the lexer, but we pulled back from that implementation. So its no surprise to me we are hitting a few errors in this area, one issue I've had problems with is a string ending with \ e.g.

@"ABC\" + x + "ABC"

this can confuse the tokenizer into thinking this is one string 'ABC" + x + '

But I think adding more support here to handle these cases is good. I'd like to see more test examples if possible

[MyDeveloperDay]

Just for reference, take a look at D58404: [clang-format] Add basic support for formatting C# files especially the historical diff

https://reviews.llvm.org/D58404?vs=on&id=188239&whitespace=ignore-most#toc

in the end, we didn't land that Lex changes but incase we feel it needs to come back to provide full support for C# strings it might help a little

[krasimir]

clang/lib/Format/FormatTokenLexer.cpp

190

nit: start with an upper case: // Interpolated ...

229

Curious why do we need this check?

235

(Not needed to be addressed in this patch):
What if there was a newline between CSharpInterpolatedString and NextToken, like in:

string s = $"hello {@"la
     la" + name} world!";

I'm not sure if this is valid C#, and I'm not sure if verbatim C# strings with newlines are formatted well currently, however worth trying several such examples and seeing how they get laid out. The ColumnWidth computation might be incorrect and you might need to set FormatToken::IsMultiline.

clang/unittests/Format/FormatTestCSharp.cpp

424

Please also add a test where }} is skipped.

[jbcoe]

Address review comments.

[jbcoe]

clang/lib/Format/FormatTokenLexer.cpp

229

Check for empty TokenText removed.

[jbcoe]

clang/lib/Format/FormatTokenLexer.cpp

235

Newlines in verbatim strings still cause problems. I'll look into fixing this in a follow-up PR.

clang/lib/Format/FormatTokenLexer.h

public:
   const AdditionalKeywords &getKeywords() { return Keywords; }
 
 private:
   void tryMergePreviousTokens();
 
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
   bool tryMergeJSPrivateIdentifier();
-  bool tryMergeCSharpVerbatimStringLiteral();
+  bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
   bool tryTransformCSharpForEach();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 
   // Returns \c true if \p Tok can only be followed by an operand in JavaScript.

clang/lib/Format/FormatTokenLexer.cpp

void FormatTokenLexer::tryMergePreviousTokens() {
   if (tryMergeConflictMarkers())
     return;
   if (tryMergeLessLess())
     return;
 
   if (Style.isCSharp()) {
     if (tryMergeCSharpKeywordVariables())
       return;
-    if (tryMergeCSharpVerbatimStringLiteral())
+    if (tryMergeCSharpStringLiteral())
       return;
     if (tryMergeCSharpDoubleQuestion())
       return;
     if (tryMergeCSharpNullConditionals())
       return;
     if (tryTransformCSharpForEach())
       return;
     static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
bool FormatTokenLexer::tryMergeJSPrivateIdentifier() {
   Hash->Type = TT_JsPrivateIdentifier;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
 // Search for verbatim or interpolated string literals @"ABC" or
 // $"aaaaa{abc}aaaaa" i and mark the token as TT_CSharpStringLiteral, and to
 // prevent splitting of @, $ and ".
-bool FormatTokenLexer::tryMergeCSharpVerbatimStringLiteral() {
+bool FormatTokenLexer::tryMergeCSharpStringLiteral() {
   if (Tokens.size() < 2)
     return false;
 
-  auto &String = *(Tokens.end() - 1);
-  if (!String->is(tok::string_literal))
-    return false;
+  auto &CSharpStringLiteral = *(Tokens.end() - 2);
+
+  // interpolated strings could contain { } with " characters inside.
+  // $"{x ?? "null"}"
+  // should not be split into $"{x ?? ", null, "}" but should treated as a
+  // single string-literal.
+  //
+  // We opt not to try and format expressions inside {} within a C#
+  // interpolated string. Formatting expressions within an interpolated string
+  // would require similar work as that done for JavaScript template strings
+  // in `handleTemplateStrings()`.
+  auto &CSharpInterpolatedString = *(Tokens.end() - 2);
+  if (CSharpInterpolatedString->Type == TT_CSharpStringLiteral &&
+      (CSharpInterpolatedString->TokenText.startswith(R"($")") ||
+       CSharpInterpolatedString->TokenText.startswith(R"($@")"))) {
+    int UnmatchedOpeningBraceCount = 0;
+
+    auto TokenTextSize = CSharpInterpolatedString->TokenText.size();
+    for (size_t Index = 0; Index < TokenTextSize; ++Index) {
+      char C = CSharpInterpolatedString->TokenText[Index];
+      if (C == '{') {
+        // "{{"  inside an interpolated string is an escaped '{' so skip it.
+        if (Index + 1 < TokenTextSize &&
+            CSharpInterpolatedString->TokenText[Index + 1] == '{') {
+          ++Index;
+          continue;
+        }
+        ++UnmatchedOpeningBraceCount;
+      } else if (C == '}') {
+        // "}}"  inside an interpolated string is an escaped '}' so skip it.
+        if (Index + 1 < TokenTextSize &&
+            CSharpInterpolatedString->TokenText[Index + 1] == '}') {
+          ++Index;
+          continue;
+        }
+        --UnmatchedOpeningBraceCount;
+      }
+    }
+
+    if (UnmatchedOpeningBraceCount > 0) {
+      auto &NextToken = *(Tokens.end() - 1);
+      if (NextToken->TokenText.empty())
+        return false;
+      CSharpInterpolatedString->TokenText =
+          StringRef(CSharpInterpolatedString->TokenText.begin(),
+                    NextToken->TokenText.end() -
+                        CSharpInterpolatedString->TokenText.begin());
+      CSharpInterpolatedString->ColumnWidth += NextToken->ColumnWidth;
+      Tokens.erase(Tokens.end() - 1);
+      return true;
+    }
+  }
 
   // verbatim strings could contain "" which C# sees as an escaped ".
   // @"""Hello""" will have been tokenized as @"" "Hello" "" and needs
   // merging into a single string literal.
-  auto &CSharpStringLiteral = *(Tokens.end() - 2);
+  auto &String = *(Tokens.end() - 1);
+  if (!String->is(tok::string_literal))
+    return false;
+
   if (CSharpStringLiteral->Type == TT_CSharpStringLiteral &&
       (CSharpStringLiteral->TokenText.startswith(R"(@")") ||
        CSharpStringLiteral->TokenText.startswith(R"($@")"))) {
     CSharpStringLiteral->TokenText = StringRef(
         CSharpStringLiteral->TokenText.begin(),
         String->TokenText.end() - CSharpStringLiteral->TokenText.begin());
     CSharpStringLiteral->ColumnWidth += String->ColumnWidth;
     Tokens.erase(Tokens.end() - 1);

clang/unittests/Format/FormatTestCSharp.cpp

 TEST_F(FormatTestCSharp, CSharpEscapedQuotesInVerbatimStrings) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
   verifyFormat(R"(string str = @"""")", Style);
   verifyFormat(R"(string str = @"""Hello world""")", Style);
   verifyFormat(R"(string str = $@"""Hello {friend}""")", Style);
 }
 
+TEST_F(FormatTestCSharp, CSharpQuotesInInterpolatedStrings) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(string str1 = $"{null ?? "null"}";)", Style);
+  verifyFormat(R"(string str2 = $"{{{braceCount}";)", Style);
+}
+
 } // namespace format
 } // end namespace clang