[clang-format] Add more support for C# 8 nullables

[exv]

Summary

This adds support for the null-coalescing assignment and null-forgiving
operators.

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-coalescing-operator

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving

[MyDeveloperDay]

Thank you for the patch

clang/lib/Format/TokenAnnotator.cpp

3197

should this honour SpaceBeforeAssignmentOperators ?

[exv]

Remove special whitespace handling for ??=

clang/lib/Format/TokenAnnotator.cpp

3197

Good point. Looking more closely, since I have called setKind(tok::equal) on the operator, it seems that the rest of clang-format reliably understands it as an assignment operator and formats it correctly if I don't intervene. I have updated the test case to test this behavior.

[exv]

Fix incorrect arc usage

[exv]

Fixing arc, hopefully?

[exv]

Hey all, I'm really sorry for the noise, I screwed up my arc command to revise the submission.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTestCSharp.cpp

365

Why do you escape ?? I've never seen that.

[exv]

clang/unittests/Format/FormatTestCSharp.cpp

365

Apparently, ??= is a trigraph for "#" in C, so it must be escaped:

https://en.wikipedia.org/wiki/Digraphs_and_trigraphs#C

To be honest, I'd never heard of this bizarre language "feature" until I wrote this patch.

[curdeius]

Looks very good already. Just some questions and test requests.

Requesting changes so it shows up nicely in the revision queue.

clang/lib/Format/TokenAnnotator.cpp

1593

Does adding false and true here may negatively affect JS in any way? (Just thinking out loud)

3195

Should it be renamed to include C#?

clang/unittests/Format/FormatTestCSharp.cpp

365

Not only in C but in C++ too. Fortunately removed in C++17.
Nobody misses trigraphs...

380

Please test also true! or !true!.

381

Could you test that ! is correctly kept together with the corresponding identifier around the line breaks, please?
You might adjust ColumnLimit for that.

[exv]

• [clang-format] Add more support for C# 8 nullables
• Fix null-coalescing assignment whitespace behavior
• Incorporate feedback from curdeius
• Combine JS and CS types/parsing

clang/lib/Format/TokenAnnotator.cpp

1593

Nothing I can see: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#non-null-assertion-operator

3195

Now that you've mentioned it, I do see that JS also defines an almost duplicate set of null-related operators, including null-propagating, null-coalescing and null-coalescing assignment operators, and clang-format handles them with a different code path than C# 😅. They do have slightly different names, but the way they should be parsed and interpreted is the same, and I think it makes sense to use the same logic and token definitions for both of these languages. This resulted in a lot of special-handling for C# tokens that wasn't necessary being removed too.

However, renaming them would introduce a lot of noise, so I think I would prefer to do that in a separate commit.

[curdeius]

LGTM. That's great. Thanks for working on this and for the cleanup.

clang/lib/Format/FormatTokenLexer.cpp

102

Could you explain in what case the operator precedence may be wrong here?

clang/lib/Format/TokenAnnotator.cpp

3195

Fair enough.

[exv]

• Remove obsolete comment

[exv]

According to the wiki, it seems like someone who has commit access must now submit this change.

clang/lib/Format/FormatTokenLexer.cpp

102

It's unlikely this comment is still relevant for this section. I will delete it.

[curdeius]

In D101702#2736886, @exv wrote:

According to the wiki, it seems like someone who has commit access must now submit this change.

Yes, I'll land it. Please provide "Name <mail@address>" for commit attribution if it differs from the info in your Phabricator account.

[exv]

Eliza Velasquez <[e-mail]>

[exv]

I have completed two other formatting fixes related to C# 8 nullables since my last commit. Should I create a new review request for those or update this one?

[curdeius]

Preferably a new one unless it's something really minor.

[curdeius]

Could you please rebase to re-run the CI (that failed for sum unrelated reason)?

[krasimir]

clang/lib/Format/TokenAnnotator.cpp

1585

nit: s/isIdentifier/IsIdentifier/ (LLVM style uses CamelCase for local variables)

clang/unittests/Format/FormatTestCSharp.cpp

365

nit: you could use raw string literals for the ??= test cases (e.g., R"(test ??= ABC;)").

[krasimir]

clang/lib/Format/FormatTokenLexer.cpp

111

nit: use spaces for indentation, no tabs (Phabricator makes it look this line starts with a tab character)

[jbcoe]

Thanks for this patch!

[HazardyKnusperkeks]

clang/lib/Format/FormatTokenLexer.cpp

111

That's not a tab, I fell for that one too once. It just visualizes that there was only whitespace added.

[exv]

• Incorporate krasimir's feedback

[exv]

Rebased and comments addressed!

[exv]

Rebase again in an attempt to fix CI

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(TypeDeclarationParen)                                                   \
   TYPE(TypenameMacro)                                                          \
   TYPE(UnaryOperator)                                                          \
   TYPE(UntouchableMacroFunc)                                                   \
   TYPE(CSharpStringLiteral)                                                    \
   TYPE(CSharpNamedArgumentColon)                                               \
   TYPE(CSharpNullable)                                                         \
   TYPE(CSharpNullCoalescing)                                                   \
+  TYPE(CSharpNullCoalescingAssignment)                                         \
   TYPE(CSharpNullConditional)                                                  \
   TYPE(CSharpNullConditionalLSquare)                                           \
   TYPE(CSharpGenericTypeConstraint)                                            \
   TYPE(CSharpGenericTypeConstraintColon)                                       \
   TYPE(CSharpGenericTypeConstraintComma)                                       \
   TYPE(Unknown)
 
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a

clang/lib/Format/FormatTokenLexer.h

 private:
   void tryMergePreviousTokens();
 
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
   bool tryMergeJSPrivateIdentifier();
   bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
-  bool tryMergeCSharpDoubleQuestion();
+  bool tryMergeCSharpNullCoalescing();
+  bool tryMergeCSharpNullCoalescingAssignment();
   bool tryMergeCSharpNullConditional();
   bool tryTransformCSharpForEach();
   bool tryMergeForEach();
   bool tryTransformTryUsageForC();
 
   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 (Style.isCpp() && tryTransformTryUsageForC())
     return;
 
   if (Style.isCSharp()) {
     if (tryMergeCSharpKeywordVariables())
       return;
     if (tryMergeCSharpStringLiteral())
       return;
-    if (tryMergeCSharpDoubleQuestion())
+    if (tryMergeCSharpNullCoalescing())
+      return;
+    if (tryMergeCSharpNullCoalescingAssignment())
       return;
     if (tryMergeCSharpNullConditional())
       return;
     if (tryTransformCSharpForEach())
       return;
     static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
     if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
       return;
   }
 
   if (tryMergeNSStringLiteral())
     return;
 
   if (Style.Language == FormatStyle::LK_JavaScript) {
     static const tok::TokenKind JSIdentity[] = {tok::equalequal, tok::equal};
     static const tok::TokenKind JSNotIdentity[] = {tok::exclaimequal,
                                                    tok::equal};

 
 // Valid C# attribute targets:
 // https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets
 const llvm::StringSet<> FormatTokenLexer::CSharpAttributeTargets = {
     "assembly", "module",   "field",  "event", "method",
     "param",    "property", "return", "type",
 };
 
-bool FormatTokenLexer::tryMergeCSharpDoubleQuestion() {
+bool FormatTokenLexer::tryMergeCSharpNullCoalescing() {
   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); // no '??' in clang tokens.
   FirstQuestion->TokenText = StringRef(FirstQuestion->TokenText.begin(),
                                        SecondQuestion->TokenText.end() -
                                            FirstQuestion->TokenText.begin());
   FirstQuestion->ColumnWidth += SecondQuestion->ColumnWidth;
   FirstQuestion->setType(TT_CSharpNullCoalescing);
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
+bool FormatTokenLexer::tryMergeCSharpNullCoalescingAssignment() {
+  if (Tokens.size() < 2)
+    return false;
+  auto &NullCoalescing = *(Tokens.end() - 2);
+  auto &Equal = *(Tokens.end() - 1);
+  if (NullCoalescing->getType() != TT_CSharpNullCoalescing ||
+      !Equal->is(tok::equal))
+    return false;
+  NullCoalescing->Tok.setKind(tok::equal); // no '??=' in clang tokens.
+  NullCoalescing->TokenText =
+      StringRef(NullCoalescing->TokenText.begin(),
+                Equal->TokenText.end() - NullCoalescing->TokenText.begin());
+  NullCoalescing->ColumnWidth += Equal->ColumnWidth;
+  NullCoalescing->setType(TT_CSharpNullCoalescingAssignment);
+  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))

clang/lib/Format/TokenAnnotator.cpp

if (Style.isCSharp() && CurrentToken->is(tok::question)) {
         return;
       }
       if (CurrentToken->TokenText == "?[") {
         Current.setType(TT_CSharpNullConditionalLSquare);
         return;
       }
     }
 
-    if (Style.Language == FormatStyle::LK_JavaScript) {
-      if (Current.is(tok::exclaim)) {
-        if (Current.Previous &&
-            (Keywords.IsJavaScriptIdentifier(
-                 *Current.Previous, /* AcceptIdentifierName= */ true) ||
-             Current.Previous->isOneOf(
-                 tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
-                 Keywords.kw_type, Keywords.kw_get, Keywords.kw_set) ||
-             Current.Previous->Tok.isLiteral())) {
+    if ((Style.Language == FormatStyle::LK_JavaScript || Style.isCSharp()) &&
+        Current.is(tok::exclaim)) {
+      if (Current.Previous) {
+        bool isIdentifier =
+            Style.Language == FormatStyle::LK_JavaScript
+                ? Keywords.IsJavaScriptIdentifier(
+                      *Current.Previous, /* AcceptIdentifierName= */ true)
+                : Current.Previous->is(tok::identifier);
+        if (isIdentifier ||
+                tok::kw_false, tok::kw_true, Keywords.kw_type, Keywords.kw_get,
+                Keywords.kw_set) ||
+            Current.Previous->Tok.isLiteral()) {
           Current.setType(TT_JsNonNullAssertion);
           return;
         }
-        if (Current.Next &&
-            Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
-          Current.setType(TT_JsNonNullAssertion);
-          return;
-        }
-      }
-    }
+      }
 
     // Line.MightBeFunctionDecl can only be true after the parentheses of a
     // function declaration have been found. In this case, 'Current' is a
     // trailing token of this declaration and thus cannot be a name.
     if (Current.is(Keywords.kw_instanceof)) {
       Current.setType(TT_BinaryOperator);
     } else if (isStartOfName(Current) &&
                (!Line.MightBeFunctionDecl || Current.NestingLevel != 0)) {
if (Style.isCpp()) {
     // 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 null forgiving '!'.
+    if (Right.is(TT_JsNonNullAssertion))
+      return false;
+
     // No space before '?['.
     if (Right.is(TT_CSharpNullConditionalLSquare))
       return false;
 
     // No space between consecutive commas '[,,]'.
     if (Left.is(tok::comma) && Right.is(tok::comma))
       return false;
 

clang/unittests/Format/FormatTestCSharp.cpp

 }
 
 TEST_F(FormatTestCSharp, CSharpNullCoalescing) {
   verifyFormat("var test = ABC ?? DEF");
   verifyFormat("string myname = name ?? \"ABC\";");
   verifyFormat("return _name ?? \"DEF\";");
 }
 
+TEST_F(FormatTestCSharp, CSharpNullCoalescingAssignment) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeAssignmentOperators = true;
+
+  verifyFormat("test \?\?= ABC;", Style);
+  verifyFormat("test \?\?= true;", Style);
+
+  Style.SpaceBeforeAssignmentOperators = false;
+
+  verifyFormat("test\?\?= ABC;", Style);
+  verifyFormat("test\?\?= true;", Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpNullForgiving) {
+  verifyFormat("var test = null!;");
+  verifyFormat("string test = someFunctionCall()! + \"ABC\"!");
+  verifyFormat("int test = (1! + 2 + bar! + foo())!");
+  verifyFormat("test \?\?= !foo!;");
+  verifyFormat("test = !bar! \?\? !foo!;");
+  verifyFormat("bool test = !(!true || !null! || !false && !false! && !bar()! "
+               "+ (!foo()))!");
+}
+
 TEST_F(FormatTestCSharp, AttributesIndentation) {
   FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
 
   verifyFormat("[STAThread]\n"
                "static void Main(string[] args)\n"
                "{\n"
                "}",