[clang-format] Improved parser for C# properties

[jbcoe]

Summary

Added some examples of properties from Microsoft documentation as test cases.

https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties

Configuration support will be added in a follow up patch to address whether automatic properties are formatted as

Type MyType { get; set }

or

Type MyType 
{ get; set }

[MyDeveloperDay]

Can you please clang-format the patch

[jbcoe]

Format patch.

[MyDeveloperDay]

I think if this code is only used by C# which the first if suggests, we should be OK, just tidy up a bit with the old code and I think this is fine.

Thank you, so great to have other C#'ers doing this ;-) "Clang format all the things..."

clang/lib/Format/UnwrappedLineParser.cpp

1516

the style in LLVM is to not have braces on single line if statements. (not my style but we need to keep to it)

1565

you need to remove old code

[krasimir]

clang/lib/Format/UnwrappedLineParser.cpp

1511

Add a comment illustrating is what is a trivial property accessor.

[jbcoe]

Added missing comments.

Complied with LLVM style.

[krasimir]

Thank you!

clang/lib/Format/UnwrappedLineParser.cpp

 //===--- UnwrappedLineParser.cpp - Format C++ code ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file contains the implementation of the UnwrappedLineParser,
 /// which turns a stream of tokens into UnwrappedLines.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "UnwrappedLineParser.h"
+#include "FormatToken.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <algorithm>
 
 #define DEBUG_TYPE "format-parser"
 

 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);`
-  //  ^^^^^^^^^^^^^
+  // See if we are inside a property accessor.
   //
   // 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();
 
+  // A trivial property accessor is of the form:
+  // { [ACCESS_SPECIFIER] [get]; [ACCESS_SPECIFIER] [set] }
+  // Track these as they do not require line breaks to be introduced.
   bool HasGetOrSet = false;
+  bool IsTrivialPropertyAccessor = true;
   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 (Tok->isNot(tok::r_brace))
+      IsTrivialPropertyAccessor = false;
   }
 
   if (!HasGetOrSet) {
     Tokens->setPosition(StoredPosition);
     return false;
   }
 
+  // Try to parse the property accessor:
+  // https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties
   Tokens->setPosition(StoredPosition);
-  while (FormatTok->isNot(tok::r_brace)) {
+  nextToken();
+  do {
+    switch (FormatTok->Tok.getKind()) {
+    case tok::r_brace:
+      nextToken();
+      if (FormatTok->is(tok::equal)) {
+        while (!eof() && FormatTok->isNot(tok::semi))
+          nextToken();
         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) {
+      addUnwrappedLine();
+      return true;
+    case tok::l_brace:
+      ++Line->Level;
+      parseBlock(/*MustBeDeclaration=*/true);
+      addUnwrappedLine();
+      --Line->Level;
+      break;
+    case tok::equal:
+      if (FormatTok->is(TT_JsFatArrow)) {
+        ++Line->Level;
         do {
           nextToken();
-      if (FormatTok->is(tok::semi))
+        } while (!eof() && FormatTok->isNot(tok::semi));
+        nextToken();
+        addUnwrappedLine();
+        --Line->Level;
         break;
-    } while (!eof());
       }
-
-  // Add an unwrapped line for the whole property accessor.
       nextToken();
+      break;
+    default:
+      if (FormatTok->isOneOf(Keywords.kw_get, Keywords.kw_set) &&
+          !IsTrivialPropertyAccessor) {
+        // Non-trivial get/set needs to be on its own line.
         addUnwrappedLine();
+      }
+      nextToken();
+    }
+  } while (!eof());
+
+  // Unreachable for well-formed code (paired '{' and '}').
   return true;
 }
 
 bool UnwrappedLineParser::tryToParseLambda() {
   if (!Style.isCpp()) {
     nextToken();
     return false;
   }

clang/unittests/Format/FormatTestCSharp.cpp

TEST_F(FormatTestCSharp, CSharpPropertyAccessors) {
 
   // Do not wrap expression body definitions.
   verifyFormat(R"(//
 public string Name {
   get => _name;
   set => _name = value;
 })",
                Style);
+
+  // Examples taken from
+  // https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties
+  verifyFormat(R"(
+// Expression body definitions
+public class SaleItem {
+  public decimal Price {
+    get => _cost;
+    set => _cost = value;
+  }
+})",
+               Style);
+
+  verifyFormat(R"(
+// Properties with backing fields
+class TimePeriod {
+  public double Hours {
+    get { return _seconds / 3600; }
+    set {
+      if (value < 0 || value > 24)
+        throw new ArgumentOutOfRangeException(
+            $"{nameof(value)} must be between 0 and 24.");
+      _seconds = value * 3600;
+    }
+  }
+})",
+               Style);
+
+  verifyFormat(R"(
+// Auto-implemented properties
+public class SaleItem {
+  public decimal Price { get; set; }
+})",
+               Style);
+
+  // Add column limit to wrap long lines.
+  Style.ColumnLimit = 100;
+
+  // Examples with assignment to default value.
+  verifyFormat(R"(
+// Long assignment to default value
+class MyClass {
+  public override VeryLongNamedTypeIndeed VeryLongNamedValue { get; set } =
+      VeryLongNamedTypeIndeed.Create(DefaultFirstArgument, DefaultSecondArgument,
+                                     DefaultThirdArgument);
+})",
+               Style);
+
+  verifyFormat(R"(
+// Long assignment to default value with expression body
+class MyClass {
+  public override VeryLongNamedTypeIndeed VeryLongNamedValue {
+    get => veryLongNamedField;
+    set => veryLongNamedField = value;
+  } = VeryLongNamedTypeIndeed.Create(DefaultFirstArgument, DefaultSecondArgument,
+                                     DefaultThirdArgument);
+})",
+               Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpSpaces) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
   Style.SpaceBeforeSquareBrackets = false;
   Style.SpacesInSquareBrackets = false;
   Style.SpaceBeforeCpp11BracedList = true;
   Style.Cpp11BracedListStyle = false;