[clang] 44ad58b - [clang-format] Improved parser for C# properties

[Jonathan Coe via cfe-commits]

Author: Jonathan Coe
Date: 2020-04-28T14:11:09+01:00
New Revision: 44ad58b9915d29eb48be4f89368be6d30d174825

URL: https://github.com/llvm/llvm-project/commit/44ad58b9915d29eb48be4f89368be6d30d174825
DIFF: https://github.com/llvm/llvm-project/commit/44ad58b9915d29eb48be4f89368be6d30d174825.diff

LOG: [clang-format] Improved parser for C# properties

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 }
```

Reviewers: krasimir, MyDeveloperDay

Reviewed By: krasimir

Subscribers: cfe-commits

Tags: #clang-format, #clang

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

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/unittests/Format/FormatTestCSharp.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index fdae725d625b..4734ff16921b 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnwrappedLineParser.h"
+#include "FormatToken.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -1495,9 +1496,7 @@ bool UnwrappedLineParser::tryToParsePropertyAccessor() {
   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
@@ -1505,7 +1504,11 @@ bool UnwrappedLineParser::tryToParsePropertyAccessor() {
   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,
@@ -1515,10 +1518,9 @@ bool UnwrappedLineParser::tryToParsePropertyAccessor() {
       Tok = Tokens->getNextToken();
       continue;
     }
-    if (Tok->is(tok::r_brace))
-      break;
-    Tokens->setPosition(StoredPosition);
-    return false;
+    if (Tok->isNot(tok::r_brace))
+      IsTrivialPropertyAccessor = false;
+    break;
   }
 
   if (!HasGetOrSet) {
@@ -1526,33 +1528,51 @@ bool UnwrappedLineParser::tryToParsePropertyAccessor() {
     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();
-  }
-
-  // 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();
+  do {
+    switch (FormatTok->Tok.getKind()) {
+    case tok::r_brace:
       nextToken();
-      if (FormatTok->is(tok::semi))
+      if (FormatTok->is(tok::equal)) {
+        while (!eof() && FormatTok->isNot(tok::semi))
+          nextToken();
+        nextToken();
+      }
+      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();
+        } while (!eof() && FormatTok->isNot(tok::semi));
+        nextToken();
+        addUnwrappedLine();
+        --Line->Level;
         break;
-    } while (!eof());
-  }
+      }
+      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());
 
-  // Add an unwrapped line for the whole property accessor.
-  nextToken();
-  addUnwrappedLine();
+  // Unreachable for well-formed code (paired '{' and '}').
   return true;
 }
 

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp
index d37a533c3c5d..75112a786867 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -611,6 +611,64 @@ TEST_F(FormatTestCSharp, CSharpPropertyAccessors) {
 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);
 }