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

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 28 03:34:34 PDT 2021


MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: jbcoe, krasimir, HazardyKnusperkeks, curdeius.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

D74265: [clang-format] Improve handling of C# attributes <https://reviews.llvm.org/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: [clang-format] Improve handling of C# attributes <https://reviews.llvm.org/D74265> has broken a lot of C# code and this revision will go someway alone to addressing the majority.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103307

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


Index: clang/unittests/Format/FormatTestCSharp.cpp
===================================================================
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -247,6 +247,16 @@
   verifyFormat("[TestMethod]\n"
                "public string Host { set; get; }");
 
+  // Adjacent attributes caused 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; }");
+
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "listening on provided host\")]\n"
                "public string Host { set; get; }");
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3520,6 +3520,13 @@
       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;
+
   } 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 &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103307.348485.patch
Type: text/x-patch
Size: 1733 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210528/d2fe9be2/attachment-0001.bin>


More information about the cfe-commits mailing list