[clang-format] Handle C# property accessors when parsing lines

Summary:
Improve C# `{ get; set; } = default;` formatting by handling it in the UnwrappedLineParser rather than trying to merge lines later.

Remove old logic to merge lines.

Update tests as formatting output has changed (as intended).

Reviewers: krasimir, MyDeveloperDay

Reviewed By: krasimir

Subscribers: cfe-commits

Tags: #clang-format, #clang

Differential Revision: https://reviews.llvm.org/D78642

Author: jbcoe

clang/lib/Format/UnwrappedLineFormatter.cpp

@@ -294,13 +294,6 @@ class LineJoiner {
       }
     }
 
-    // Try to merge a CSharp property declaration like `{ get; private set }`.
-    if (Style.isCSharp()) {
-      unsigned CSPA = tryMergeCSharpPropertyAccessor(I, E, Limit);
-      if (CSPA > 0)
-        return CSPA;
-    }
-
     // Try to merge a function block with left brace unwrapped
     if (TheLine->Last->is(TT_FunctionLBrace) &&
         TheLine->First != TheLine->Last) {
@@ -430,64 +423,6 @@ class LineJoiner {
     return 0;
   }
 
-  // true for lines of the form [access-modifier] {get,set} [;]
-  bool isMergeablePropertyAccessor(const AnnotatedLine *Line) {
-    auto *Tok = Line->First;
-    if (!Tok)
-      return false;
-
-    if (Tok->isOneOf(tok::kw_public, tok::kw_protected, tok::kw_private,
-                     Keywords.kw_internal))
-      Tok = Tok->Next;
-
-    if (!Tok || (Tok->TokenText != "get" && Tok->TokenText != "set"))
-      return false;
-
-    if (!Tok->Next || Tok->Next->is(tok::semi))
-      return true;
-
-    return false;
-  }
-
-  unsigned tryMergeCSharpPropertyAccessor(
-      SmallVectorImpl<AnnotatedLine *>::const_iterator I,
-      SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned /*Limit*/) {
-
-    auto CurrentLine = I;
-    // Does line start with `{`
-    if (!(*CurrentLine)->Last || (*CurrentLine)->Last->isNot(TT_FunctionLBrace))
-      return 0;
-    ++CurrentLine;
-
-    unsigned MergedLines = 0;
-    bool HasGetOrSet = false;
-    while (CurrentLine != E) {
-      bool LineIsGetOrSet = isMergeablePropertyAccessor(*CurrentLine);
-      HasGetOrSet = HasGetOrSet || LineIsGetOrSet;
-      if (LineIsGetOrSet) {
-        ++CurrentLine;
-        ++MergedLines;
-        continue;
-      }
-      auto *Tok = (*CurrentLine)->First;
-      if (Tok && Tok->is(tok::r_brace)) {
-        ++CurrentLine;
-        ++MergedLines;
-        // See if the next line is a default value so that we can merge `{ get;
-        // set } = 0`
-        if (CurrentLine != E && (*CurrentLine)->First &&
-            (*CurrentLine)->First->is(tok::equal)) {
-          ++MergedLines;
-        }
-        break;
-      }
-      // Not a '}' or a get/set line so do not merege lines.
-      return 0;
-    }
-
-    return HasGetOrSet ? MergedLines : 0;
-  }
-
   unsigned
   tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
                             SmallVectorImpl<AnnotatedLine *>::const_iterator E,

clang/lib/Format/UnwrappedLineParser.cpp

@@ -1334,7 +1334,7 @@ void UnwrappedLineParser::parseStructuralElement() {
         parseChildBlock();
       break;
     case tok::l_brace:
-      if (!tryToParseBracedList()) {
+      if (!tryToParsePropertyAccessor() && !tryToParseBracedList()) {
         // A block outside of parentheses must be the last part of a
         // structural element.
         // FIXME: Figure out cases where this is not true, and add projections
@@ -1487,6 +1487,75 @@ void UnwrappedLineParser::parseStructuralElement() {
   } while (!eof());
 }
 
+bool UnwrappedLineParser::tryToParsePropertyAccessor() {
+  assert(FormatTok->is(tok::l_brace));
+  if (!Style.isCSharp())
+    return false;
+  // See if it's a property accessor.
+  if (FormatTok->Previous->isNot(tok::identifier))
+    return false;
+
+  // Try to parse the property accessor braces and contents:
+  // `{ get; set; } = new MyType(defaultValue);`
+  //  ^^^^^^^^^^^^^
+  //
+  // Record the current tokenPosition so that we can advance and
+  // reset the current token. `Next` is not set yet so we need
+  // another way to advance along the token stream.
+  unsigned int StoredPosition = Tokens->getPosition();
+  FormatToken *Tok = Tokens->getNextToken();
+
+  bool HasGetOrSet = false;
+  while (!eof()) {
+    if (Tok->isOneOf(tok::semi, tok::kw_public, tok::kw_private,
+                     tok::kw_protected, Keywords.kw_internal, Keywords.kw_get,
+                     Keywords.kw_set)) {
+      if (Tok->isOneOf(Keywords.kw_get, Keywords.kw_set))
+        HasGetOrSet = true;
+      Tok = Tokens->getNextToken();
+      continue;
+    }
+    if (Tok->is(tok::r_brace))
+      break;
+    Tokens->setPosition(StoredPosition);
+    return false;
+  }
+
+  if (!HasGetOrSet) {
+    Tokens->setPosition(StoredPosition);
+    return false;
+  }
+
+  Tokens->setPosition(StoredPosition);
+  while (FormatTok->isNot(tok::r_brace)) {
+    nextToken();
+  }
+
+  // Try to parse (optional) assignment to default value:
+  // `{ get; set; } = new MyType(defaultValue);`
+  //                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  // There may be some very complicated expressions inside default value
+  // assignment, the simple parse block below will not handle them.
+  // The parse block below would need extending to handle opening parens etc.
+  StoredPosition = Tokens->getPosition();
+  Tok = Tokens->getNextToken();
+  bool NextTokenIsEqual = Tok->is(tok::equal);
+  Tokens->setPosition(StoredPosition);
+
+  if (NextTokenIsEqual) {
+    do {
+      nextToken();
+      if (FormatTok->is(tok::semi))
+        break;
+    } while (!eof());
+  }
+
+  // Add an unwrapped line for the whole property accessor.
+  nextToken();
+  addUnwrappedLine();
+  return true;
+}
+
 bool UnwrappedLineParser::tryToParseLambda() {
   if (!Style.isCpp()) {
     nextToken();

clang/lib/Format/UnwrappedLineParser.h

@@ -132,6 +132,7 @@ class UnwrappedLineParser {
   void parseCSharpGenericTypeConstraint();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
+  bool tryToParsePropertyAccessor();
   void tryToParseJSFunction();
   void addUnwrappedLine();
   bool eof() const;

clang/unittests/Format/FormatTestCSharp.cpp

@@ -245,13 +245,11 @@ TEST_F(FormatTestCSharp, Attributes) {
                "}");
 
   verifyFormat("[TestMethod]\n"
-               "public string Host\n"
-               "{ set; get; }");
+               "public string Host { set; get; }");
 
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "listening on provided host\")]\n"
-               "public string Host\n"
-               "{ set; get; }");
+               "public string Host { set; get; }");
 
   verifyFormat(
       "[DllImport(\"Hello\", EntryPoint = \"hello_world\")]\n"