[clang-format] Handle NullCoalescing and NullConditional operators in C#

[jbcoe]

Summary

Disable merging of Type? into a single token.

Merge ?? ?. and ?[ into a single token.

[jbcoe]

There's a weird test failure I'm unable to reproduce outside of the tests with real source code where

public static void Main(string[] args)
{ 
  string dirPath = args?[0];
}

is (mis)-formatted as

public static void Main(string[] args)
{ string dirPath = args?[0];
}

[jbcoe]

Fix failing test by parsing tokens after '?[' in the same way as tokens after '['.

[krasimir]

clang/include/clang/Basic/TokenKinds.def

237 ↗

(On Diff #247399)

I think this is not a good place to add these.
I think these definitions are generally for languages clang itself supports (compiling)
This could have unexpected ripple effects in unrelated parts of the clang.
Could you try to handle these somehow in lib/Format/FormatTokenLexer?
That's where the clang-format has some glue / heuristics for handling other languages syntax.

[jbcoe]

Handle ??, ?. and ?[ in lib/Format/FormatTokenLexer.

[jbcoe]

Do not add new punctuators for ??, ?. and ?[

[krasimir]

clang/lib/Format/FormatToken.h

109

nit: consider renaming to CSharpNullConditionalLSquare for consistency with other similar token types.

clang/lib/Format/TokenAnnotator.cpp

2955

please add a test case about this where Style.SpacesInSquareBrakets == true.

[krasimir]

Sorry, did the last round of comments not make sense?

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(TemplateString)                                                         \
   TYPE(ProtoExtensionLSquare)                                                  \
   TYPE(TrailingAnnotation)                                                     \
   TYPE(TrailingReturnArrow)                                                    \
   TYPE(TrailingUnaryOperator)                                                  \
   TYPE(TypenameMacro)                                                          \
   TYPE(UnaryOperator)                                                          \
   TYPE(CSharpStringLiteral)                                                    \
-  TYPE(CSharpNullCoalescing)                                                   \
   TYPE(CSharpNamedArgumentColon)                                               \
-  TYPE(CSharpNullableTypeQuestionMark)                                         \
+  TYPE(CSharpNullable)                                                         \
+  TYPE(CSharpNullCoalescing)                                                   \
+  TYPE(CSharpNullConditional)                                                  \
+  TYPE(CSharpNullConditionalSq)                                                \
   TYPE(Unknown)
 
 enum TokenType {
 #define TYPE(X) TT_##X,
   LIST_TOKEN_TYPES
 #undef TYPE
       NUM_TOKEN_TYPES
 };

clang/lib/Format/FormatTokenLexer.h

 private:
   void tryMergePreviousTokens();
 
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
   bool tryMergeJSPrivateIdentifier();
   bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
-  bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
+  bool tryMergeCSharpNullConditional();
   bool tryTransformCSharpForEach();
   bool tryMergeCSharpAttributeAndTarget();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 
   // Returns \c true if \p Tok can only be followed by an operand in JavaScript.
   bool precedesOperand(FormatToken *Tok);
 

clang/lib/Format/FormatTokenLexer.cpp

if (Style.isCSharp()) {
     if (tryMergeCSharpAttributeAndTarget())
       return;
     if (tryMergeCSharpKeywordVariables())
       return;
     if (tryMergeCSharpStringLiteral())
       return;
     if (tryMergeCSharpDoubleQuestion())
       return;
-    if (tryMergeCSharpNullConditionals())
+    if (tryMergeCSharpNullConditional())
       return;
     if (tryTransformCSharpForEach())
       return;
     static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
     if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
       return;
   }
 

 
 bool FormatTokenLexer::tryMergeCSharpDoubleQuestion() {
   if (Tokens.size() < 2)
     return false;
   auto &FirstQuestion = *(Tokens.end() - 2);
   auto &SecondQuestion = *(Tokens.end() - 1);
   if (!FirstQuestion->is(tok::question) || !SecondQuestion->is(tok::question))
     return false;
-  FirstQuestion->Tok.setKind(tok::question);
+  FirstQuestion->Tok.setKind(tok::question); // no '??' in clang tokens.
   FirstQuestion->TokenText = StringRef(FirstQuestion->TokenText.begin(),
                                        SecondQuestion->TokenText.end() -
                                            FirstQuestion->TokenText.begin());
   FirstQuestion->ColumnWidth += SecondQuestion->ColumnWidth;
   FirstQuestion->Type = TT_CSharpNullCoalescing;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
+// Merge '?[' and '?.' pairs into single tokens.
+bool FormatTokenLexer::tryMergeCSharpNullConditional() {
+  if (Tokens.size() < 2)
+    return false;
+  auto &Question = *(Tokens.end() - 2);
+  auto &PeriodOrLSquare = *(Tokens.end() - 1);
+  if (!Question->is(tok::question) ||
+      !PeriodOrLSquare->isOneOf(tok::l_square, tok::period))
+    return false;
+  Question->TokenText =
+      StringRef(Question->TokenText.begin(),
+                PeriodOrLSquare->TokenText.end() - Question->TokenText.begin());
+  Question->ColumnWidth += PeriodOrLSquare->ColumnWidth;
+
+  if (PeriodOrLSquare->is(tok::l_square)) {
+    Question->Tok.setKind(tok::question); // no '?[' in clang tokens.
+    Question->Type = TT_CSharpNullConditionalSq;
+  } else {
+    Question->Tok.setKind(tok::question); // no '?.' in clang tokens.
+    Question->Type = TT_CSharpNullConditional;
+  }
+
+  Tokens.erase(Tokens.end() - 1);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeCSharpKeywordVariables() {
   if (Tokens.size() < 2)
     return false;
   auto &At = *(Tokens.end() - 2);
   auto &Keyword = *(Tokens.end() - 1);
   if (!At->is(tok::at))
     return false;
   if (!Keywords.isCSharpKeyword(*Keyword))
     return false;
 
   At->Tok.setKind(tok::identifier);
   At->TokenText = StringRef(At->TokenText.begin(),
                             Keyword->TokenText.end() - At->TokenText.begin());
   At->ColumnWidth += Keyword->ColumnWidth;
   At->Type = Keyword->Type;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
-// In C# merge the Identifier and the ? together e.g. arg?.
-bool FormatTokenLexer::tryMergeCSharpNullConditionals() {
-  if (Tokens.size() < 2)
-    return false;
-  auto &Identifier = *(Tokens.end() - 2);
-  auto &Question = *(Tokens.end() - 1);
-  if (!Identifier->isOneOf(tok::r_square, tok::identifier) ||
-      !Question->is(tok::question))
-    return false;
-  Identifier->TokenText =
-      StringRef(Identifier->TokenText.begin(),
-                Question->TokenText.end() - Identifier->TokenText.begin());
-  Identifier->ColumnWidth += Question->ColumnWidth;
-  Tokens.erase(Tokens.end() - 1);
-  return true;
-}
-
 // In C# transform identifier foreach into kw_foreach
 bool FormatTokenLexer::tryTransformCSharpForEach() {
   if (Tokens.size() < 1)
     return false;
   auto &Identifier = *(Tokens.end() - 1);
   if (!Identifier->is(tok::identifier))
     return false;
   if (Identifier->TokenText != "foreach")

clang/lib/Format/TokenAnnotator.cpp

case tok::kw_operator:
       }
       if (CurrentToken) {
         CurrentToken->Type = TT_OverloadedOperatorLParen;
         if (CurrentToken->Previous->is(TT_BinaryOperator))
           CurrentToken->Previous->Type = TT_OverloadedOperator;
       }
       break;
     case tok::question:
+      if (Tok->is(TT_CSharpNullConditionalSq)) {
+        if (!parseSquare())
+          return false;
+        break;
+      }
+      if (Tok->isOneOf(TT_CSharpNullConditional, TT_CSharpNullCoalescing))
+        break;
       if (Style.Language == FormatStyle::LK_JavaScript && Tok->Next &&
           Tok->Next->isOneOf(tok::semi, tok::comma, tok::colon, tok::r_paren,
                              tok::r_brace)) {
         // Question marks before semicolons, colons, etc. indicate optional
         // types (fields, parameters), e.g.
         //   function(x?: string, y?) {...}
         //   class X { y?; }
         Tok->Type = TT_JsTypeOptionalQuestion;
         break;
       }
       // Declarations cannot be conditional expressions, this can only be part
       // of a type declaration.
       if (Line.MustBeDeclaration && !Contexts.back().IsExpression &&
           Style.Language == FormatStyle::LK_JavaScript)
         break;
-      if (Style.isCSharp() && Line.MustBeDeclaration) {
-        Tok->Type = TT_CSharpNullableTypeQuestionMark;
+      if (Style.isCSharp()) {
+        if (Line.MustBeDeclaration && !Contexts.back().IsExpression) {
+          Tok->Type = TT_CSharpNullable;
           break;
         }
+      }
       parseConditional();
       break;
     case tok::kw_template:
       parseTemplateDeclaration();
       break;
     case tok::comma:
       if (Contexts.back().InCtorInitializer)
         Tok->Type = TT_CtorInitializerComma;
static bool isDeductionGuide(FormatToken &Current) {
     return false;
   }
 
   void determineTokenType(FormatToken &Current) {
     if (!Current.is(TT_Unknown))
       // The token type is already known.
       return;
 
+    if (Style.isCSharp() && CurrentToken->is(tok::question)) {
+      if (CurrentToken->TokenText == "??") {
+        Current.Type = TT_CSharpNullCoalescing;
+        return;
+      }
+      if (CurrentToken->TokenText == "?.") {
+        Current.Type = TT_CSharpNullConditional;
+        return;
+      }
+      if (CurrentToken->TokenText == "?[") {
+        Current.Type = TT_CSharpNullConditionalSq;
+        return;
+      }
+    }
+
     if (Style.Language == FormatStyle::LK_JavaScript) {
       if (Current.is(tok::exclaim)) {
         if (Current.Previous &&
             (Current.Previous->isOneOf(tok::identifier, tok::kw_namespace,
                                        tok::r_paren, tok::r_square,
                                        tok::r_brace) ||
              Current.Previous->Tok.isLiteral())) {
           Current.Type = TT_JsNonNullAssertion;
if (Style.isCpp()) {
     if (Left.is(TT_TemplateCloser) && Right.is(TT_StartOfName))
       return true;
 
     // spaces inside square brackets.
     if (Left.is(tok::l_square) || Right.is(tok::r_square))
       return Style.SpacesInSquareBrackets;
 
     // No space before ? in nullable types.
-    if (Right.is(TT_CSharpNullableTypeQuestionMark))
+    if (Right.is(TT_CSharpNullable))
+      return false;
+
+    // Require space after ? in nullable types.
+    if (Left.is(TT_CSharpNullable))
+      return true;
+
+    // No space before or after '?.'.
+    if (Left.is(TT_CSharpNullConditional) || Right.is(TT_CSharpNullConditional))
       return false;
 
+    // Space before and after '??'.
+    if (Left.is(TT_CSharpNullCoalescing) || Right.is(TT_CSharpNullCoalescing))
+      return true;
+
+    // No space before '?['.
+    if (Right.is(TT_CSharpNullConditionalSq))
+      return false;
+
+    // Possible space inside `?[ 0 ]`.
+    if (Left.is(TT_CSharpNullConditionalSq))
+      return Style.SpacesInSquareBrackets;
+
     // space between keywords and paren e.g. "using ("
     if (Right.is(tok::l_paren))
       if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when))
         return Style.SpaceBeforeParens == FormatStyle::SBPO_ControlStatements ||
                spaceRequiredBeforeParens(Right);
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
     if (Left.is(TT_JsFatArrow))
       return true;

clang/unittests/Format/FormatTestCSharp.cpp

verifyFormat(
       "var verbatimInterpolated = $@\"C:\\Users\\{userName}\\Documents\\\";");
 }
 
 TEST_F(FormatTestCSharp, CSharpFatArrows) {
   verifyFormat("Task serverTask = Task.Run(async() => {");
   verifyFormat("public override string ToString() => \"{Name}\\{Age}\";");
 }
 
+TEST_F(FormatTestCSharp, CSharpConditionalExpressions) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  // conditional expression is not seen as a NullConditional.
+  verifyFormat("var y = A < B ? -1 : 1;", Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpNullConditional) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
 
   verifyFormat(
       "public Person(string firstName, string lastName, int? age = null)");
 
   verifyFormat("foo () {\n"