[clang] ffb48d4 - [clang-format] successive C# attributes cause line breaking issues

[via cfe-commits]

Author: mydeveloperday
Date: 2021-05-29T16:43:55+01:00
New Revision: ffb48d48e45c72ed81dda4983ccb06e800cdbbd0

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

LOG: [clang-format] successive C# attributes cause line breaking issues

{D74265} reduced the aggressiveness of line breaking following C# attributes, however this change removed any support for attributes on properties, causing significant ugliness to be introduced.

This revision goes some way to addressing that by re-introducing the more aggressive check to `mustBreakBefore()`, but constraining it to the most common cases where we use properties which should not impact the "caller info attributes"  or the "[In , Out]" decorations that are normally put on pinvoke

It does not address my additional concerns of the original change regarding multiple C# attributes, as these are somewhat incorrectly handled by virtue of the fact its not recognising the second attribute as an attribute at all. But instead thinking its an array.

The purpose of this revision is to get back to where we were for the most common of cases as a stepping stone to resolving this. However {D74265} has broken a lot of C# code and this revision will go someway alone to addressing the majority.

Reviewed By: jbcoe, HazardyKnusperkeks, curdeius

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index daa624000ff6d..4bd9311ebadd4 100755
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3520,6 +3520,17 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
       return false;
     if (Right.is(TT_CSharpGenericTypeConstraint))
       return true;
+
+    // Break after C# [...] and before public/protected/private/internal.
+    if (Left.is(TT_AttributeSquare) && Left.is(tok::r_square) &&
+        (Right.isAccessSpecifier(/*ColonRequired=*/false) ||
+         Right.is(Keywords.kw_internal)))
+      return true;
+    // Break between ] and [ but only when there are really 2 attributes.
+    if (Left.is(TT_AttributeSquare) && Right.is(TT_AttributeSquare) &&
+        Left.is(tok::r_square) && Right.is(tok::l_square))
+      return true;
+
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
     // FIXME: This might apply to other languages and token kinds.
     if (Right.is(tok::string_literal) && Left.is(tok::plus) && Left.Previous &&

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp
index 427f7257eeb9a..651b54cd342a7 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -247,6 +247,37 @@ TEST_F(FormatTestCSharp, Attributes) {
   verifyFormat("[TestMethod]\n"
                "public string Host { set; get; }");
 
+  // Adjacent properties should not cause line wrapping issues
+  verifyFormat("[JsonProperty(\"foo\")]\n"
+               "public string Foo { set; get; }\n"
+               "[JsonProperty(\"bar\")]\n"
+               "public string Bar { set; get; }\n"
+               "[JsonProperty(\"bar\")]\n"
+               "protected string Bar { set; get; }\n"
+               "[JsonProperty(\"bar\")]\n"
+               "internal string Bar { set; get; }");
+
+  // Multiple attributes should always be split (not just the first ones)
+  verifyFormat("[XmlIgnore]\n"
+               "[JsonProperty(\"foo\")]\n"
+               "public string Foo { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+               "[JsonProperty(\"foo\")]\n"
+               "public string Foo { set; get; }\n"
+               "[XmlIgnore]\n"
+               "[JsonProperty(\"bar\")]\n"
+               "public string Bar { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+               "[ScriptIgnore]\n"
+               "[JsonProperty(\"foo\")]\n"
+               "public string Foo { set; get; }\n"
+               "[XmlIgnore]\n"
+               "[ScriptIgnore]\n"
+               "[JsonProperty(\"bar\")]\n"
+               "public string Bar { set; get; }");
+
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "listening on provided host\")]\n"
                "public string Host { set; get; }");
@@ -271,6 +302,34 @@ TEST_F(FormatTestCSharp, Attributes) {
                "{\n"
                "}");
 
+  verifyFormat("void MethodA([In, Out] ref double x)\n"
+               "{\n"
+               "}");
+
+  verifyFormat("void MethodA([In, Out] double[] x)\n"
+               "{\n"
+               "}");
+
+  verifyFormat("void MethodA([In] double[] x)\n"
+               "{\n"
+               "}");
+
+  verifyFormat("void MethodA(int[] x)\n"
+               "{\n"
+               "}");
+  verifyFormat("void MethodA(int[][] x)\n"
+               "{\n"
+               "}");
+  verifyFormat("void MethodA([] x)\n"
+               "{\n"
+               "}");
+
+  verifyFormat("public void Log([CallerLineNumber] int line = -1, "
+               "[CallerFilePath] string path = null,\n"
+               "                [CallerMemberName] string name = null)\n"
+               "{\n"
+               "}");
+
   // [] in an attribute do not cause premature line wrapping or indenting.
   verifyFormat(R"(//
 public class A