[clang-format] C# property formatting can be controlled by config options

[jbcoe]

Summary

Allow brace wrapping in C# property accessors to be controlled by configuration options.

Add new tests and revert old test results for Microsoft style to their previous state (as intended).

FormatStyle.BraceWrapping.AfterFunction = true; will change automatic property formatting from

Type MyType { get; set }

to

Type MyType
{ get; set }

[MyDeveloperDay]

Do you think we need to support

verifyFormat(R"(//
public class SaleItem {
  public decimal Price
  { 
    get; 
    set; 
   }
})"Style);

Is that currently possible, and what if users actually want?

public string Host { set; get; }

Do you think we need an option to support variations?

[krasimir]

I'm curious why we don't go the opposite direction -- why are even those two styles needed separately?

public int Style1 { get; set }
// vs.
public int Style2
{ get; set }

I'm sure there is a good reason; part of this is to make sure we document it so we may consistently do more updates and support more complicated cases in the same spirit.
IMO starting with some reasonable formatting, and later supporting different styles as needed, has been practical to keep clang-format from becoming harder to maintain.

[jbcoe]

I've just merged this in by mistake. Can revert or amend as we like.

[jbcoe]

Reverted this commit. Will pick this up tomorrow. Apologies for the noise!

[jbcoe]

public int Style2
{ get; set }

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

public int Style2 { get; set }

is the style we use in our code (where the formatter will be put to immediate use).

Other options can be added/supported (maybe a CSharpPropertyStyle enum?) but I don't think I'm going to have time to devote to that in the immediate future. Happy to review/advise as needed.

[MyDeveloperDay]

In D79000#2009813, @jbcoe wrote:

public int Style2
{ get; set }

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

public int Style2 { get; set }

is the style we use in our code (where the formatter will be put to immediate use).

Other options can be added/supported (maybe a CSharpPropertyStyle enum?) but I don't think I'm going to have time to devote to that in the immediate future. Happy to review/advise as needed.

Maybe consider adding

AfterProperty to the BraceWrapping style?

[krasimir]

I think this is OK as-is.

@MyDeveloperDay's suggestion of adding AfterProperty to the BraceWrapping style makes sense.
However I think in this case it is natural to use the same style for these properties and for methods (functions) until we have a reason to allow for divergence.
This MS example illustrates how close such "property blocks" match "function body blocks": if you squint, add an () before the property block and sprinkle semicolons inside it, it looks like a method.
The fact that we only need this specifically for C# properties makes it a bit niche.
I'd rather be lazy about BraceWrapping.AfterProperty and add it later, as need arises.

[MyDeveloperDay]

I'm ok with your suggestion if you want to land this.

[jbcoe]

Submitted as 8fa743ab82027da443bac050e86b70bcdb78cbee

clang/lib/Format/UnwrappedLineParser.cpp

bool UnwrappedLineParser::tryToParsePropertyAccessor() {
   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);
+  if (Style.BraceWrapping.AfterFunction == true)
+    addUnwrappedLine();
   nextToken();
   do {
     switch (FormatTok->Tok.getKind()) {
     case tok::r_brace:
       nextToken();
       if (FormatTok->is(tok::equal)) {
         while (!eof() && FormatTok->isNot(tok::semi))
           nextToken();

clang/unittests/Format/FormatTestCSharp.cpp

TEST_F(FormatTestCSharp, Attributes) {
 
   verifyFormat("[Verb(\"start\", HelpText = \"Starts the server listening on "
                "provided port\")]\n"
                "class Test\n"
                "{\n"
                "}");
 
   verifyFormat("[TestMethod]\n"
-               "public string Host { set; get; }");
+               "public string Host\n"
+               "{ set; get; }");
 
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "listening on provided host\")]\n"
-               "public string Host { set; get; }");
+               "public string Host\n"
+               "{ set; get; }");
 
   verifyFormat(
       "[DllImport(\"Hello\", EntryPoint = \"hello_world\")]\n"
       "// The const char* returned by hello_world must not be deleted.\n"
       "private static extern IntPtr HelloFromCpp();)");
 
   // Class attributes go on their own line and do not affect layout of
   // interfaces. Line wrapping decisions previously caused each interface to be

 class MyClass {
   public override VeryLongNamedTypeIndeed VeryLongNamedValue {
     get => veryLongNamedField;
     set => veryLongNamedField = value;
   } = VeryLongNamedTypeIndeed.Create(DefaultFirstArgument, DefaultSecondArgument,
                                      DefaultThirdArgument);
 })",
                Style);
+
+  // Brace wrapping and single-lining of accessor can be controlled by config.
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+
+  verifyFormat(R"(//
+public class SaleItem {
+  public decimal Price
+  { get; set; }
+})",
+               Style);
+
+  verifyFormat(R"(//
+class TimePeriod {
+  public double Hours
+  {
+    get {
+      return _seconds / 3600;
+    }
+    set {
+      _seconds = value * 3600;
+    }
+  }
+})",
+               Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpSpaces) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
   Style.SpaceBeforeSquareBrackets = false;
   Style.SpacesInSquareBrackets = false;
   Style.SpaceBeforeCpp11BracedList = true;
   Style.Cpp11BracedListStyle = false;