[clang-format] [PR43333] Fix C# breaking before function name when using Attributes

Summary:
This is  a fix for https://bugs.llvm.org/show_bug.cgi?id=43333

This comes with 3 main parts

  - C# attributes cause function names on a new line even when AlwaysBreakAfterReturnType is set to None
  - Add AlwaysBreakAfterReturnType  to None by default in the Microsoft style,
  - C# unit tests are not using Microsoft style (which we created to define the default C# style to match a vanilla C# project).

Reviewers: owenpan, klimek, russellmcc, mitchell-stellar

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

Tags: #clang-tools-extra, #clang, #clang-format

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@373707 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index d774c04b6de5547313a0c1f2c007db8c65458160..7e71b7e8b167b0b95a643a38039c54ceab3b8da7 100644 (file)
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -489,38 +489,38 @@ struct FormatStyle {
 
   /// Different ways to break after the template declaration.
   enum BreakTemplateDeclarationsStyle {
-      /// Do not force break before declaration.
-      /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
-      /// \code
-      ///    template <typename T> T foo() {
-      ///    }
-      ///    template <typename T> T foo(int aaaaaaaaaaaaaaaaaaaaa,
-      ///                                int bbbbbbbbbbbbbbbbbbbbb) {
-      ///    }
-      /// \endcode
-      BTDS_No,
-      /// Force break after template declaration only when the following
-      /// declaration spans multiple lines.
-      /// \code
-      ///    template <typename T> T foo() {
-      ///    }
-      ///    template <typename T>
-      ///    T foo(int aaaaaaaaaaaaaaaaaaaaa,
-      ///          int bbbbbbbbbbbbbbbbbbbbb) {
-      ///    }
-      /// \endcode
-      BTDS_MultiLine,
-      /// Always break after template declaration.
-      /// \code
-      ///    template <typename T>
-      ///    T foo() {
-      ///    }
-      ///    template <typename T>
-      ///    T foo(int aaaaaaaaaaaaaaaaaaaaa,
-      ///          int bbbbbbbbbbbbbbbbbbbbb) {
-      ///    }
-      /// \endcode
-      BTDS_Yes
+    /// Do not force break before declaration.
+    /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
+    /// \code
+    ///    template <typename T> T foo() {
+    ///    }
+    ///    template <typename T> T foo(int aaaaaaaaaaaaaaaaaaaaa,
+    ///                                int bbbbbbbbbbbbbbbbbbbbb) {
+    ///    }
+    /// \endcode
+    BTDS_No,
+    /// Force break after template declaration only when the following
+    /// declaration spans multiple lines.
+    /// \code
+    ///    template <typename T> T foo() {
+    ///    }
+    ///    template <typename T>
+    ///    T foo(int aaaaaaaaaaaaaaaaaaaaa,
+    ///          int bbbbbbbbbbbbbbbbbbbbb) {
+    ///    }
+    /// \endcode
+    BTDS_MultiLine,
+    /// Always break after template declaration.
+    /// \code
+    ///    template <typename T>
+    ///    T foo() {
+    ///    }
+    ///    template <typename T>
+    ///    T foo(int aaaaaaaaaaaaaaaaaaaaa,
+    ///          int bbbbbbbbbbbbbbbbbbbbb) {
+    ///    }
+    /// \endcode
+    BTDS_Yes
   };
 
   /// The template declaration breaking style to use.
@@ -2186,6 +2186,10 @@ FormatStyle getWebKitStyle();
 /// http://www.gnu.org/prep/standards/standards.html
 FormatStyle getGNUStyle();
 
+/// Returns a format style complying with Microsoft style guide:
+/// https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference?view=vs-2017
+FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language);
+
 /// Returns style indicating formatting should be not applied at all.
 FormatStyle getNoStyle();
 

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 52afb6744ad3b5e215d58d4bcb895cce3f692da2..2ff6e5ec23446f584e43b2bf42c1ec830e1957b5 100644 (file)
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -473,7 +473,10 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
   }
 
   // If the return type spans multiple lines, wrap before the function name.
-  if ((Current.is(TT_FunctionDeclarationName) ||
+  if (((Current.is(TT_FunctionDeclarationName) &&
+        // Don't break before a C# function when no break after return type
+        (!Style.isCSharp() ||
+         Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None)) ||
        (Current.is(tok::kw_operator) && !Previous.is(tok::coloncolon))) &&
       !Previous.is(tok::kw_template) && State.Stack.back().BreakBeforeParameter)
     return true;

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 5b267784ff1ed8e245d280ff1dc124b84581a322..cd44c0be85f01a37b146f25cd9af734c469afd82 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -1084,7 +1084,7 @@ FormatStyle getGNUStyle() {
 }
 
 FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
-  FormatStyle Style = getLLVMStyle();
+  FormatStyle Style = getLLVMStyle(Language);
   Style.ColumnLimit = 120;
   Style.TabWidth = 4;
   Style.IndentWidth = 4;
@@ -1105,6 +1105,8 @@ FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
   Style.AllowShortCaseLabelsOnASingleLine = false;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   Style.AllowShortLoopsOnASingleLine = false;
+  Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   return Style;
 }
 

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 91ac9822c8ace9ea5ef24e6e7ac0e303020879b3..319c66c1ead2dbd0d901de1be5192fbc999f6989 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -395,6 +395,12 @@ private:
                          Keywords.kw_internal)) {
       return true;
     }
+
+    // incase its a [XXX] retval func(....
+    if (AttrTok->Next &&
+        AttrTok->Next->startsSequence(tok::identifier, tok::l_paren))
+      return true;
+
     return false;
   }
 
@@ -489,6 +495,8 @@ private:
       } else if (Style.isCpp() && Contexts.back().ContextKind == tok::l_brace &&
                  Parent && Parent->isOneOf(tok::l_brace, tok::comma)) {
         Left->Type = TT_DesignatedInitializerLSquare;
+      } else if (IsCSharp11AttributeSpecifier) {
+        Left->Type = TT_AttributeSquare;
       } else if (CurrentToken->is(tok::r_square) && Parent &&
                  Parent->is(TT_TemplateCloser)) {
         Left->Type = TT_ArraySubscriptLSquare;
@@ -536,8 +544,6 @@ private:
                                  // Should only be relevant to JavaScript:
                                  tok::kw_default)) {
         Left->Type = TT_ArrayInitializerLSquare;
-      } else if (IsCSharp11AttributeSpecifier) {
-        Left->Type = TT_AttributeSquare;
       } else {
         BindingIncrease = 10;
         Left->Type = TT_ArraySubscriptLSquare;

diff --git a/unittests/Format/FormatTestCSharp.cpp b/unittests/Format/FormatTestCSharp.cpp
index 7fda9566dc8f2f217d743835d95e86c614a284ad..031de143389f869f3c4af9105b6230533089104c 100644 (file)
--- a/unittests/Format/FormatTestCSharp.cpp
+++ b/unittests/Format/FormatTestCSharp.cpp
@@ -32,48 +32,76 @@ protected:
 
   static std::string
   format(llvm::StringRef Code,
-         const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+         const FormatStyle &Style = getMicrosoftStyle(FormatStyle::LK_CSharp)) {
     return format(Code, 0, Code.size(), Style);
   }
 
   static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
-    FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+    FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
     Style.ColumnLimit = ColumnLimit;
     return Style;
   }
 
   static void verifyFormat(
       llvm::StringRef Code,
-      const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+      const FormatStyle &Style = getMicrosoftStyle(FormatStyle::LK_CSharp)) {
     EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
     EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 };
 
 TEST_F(FormatTestCSharp, CSharpClass) {
-  verifyFormat("public class SomeClass {\n"
-               "  void f() {}\n"
-               "  int g() { return 0; }\n"
-               "  void h() {\n"
-               "    while (true) f();\n"
-               "    for (;;) f();\n"
-               "    if (true) f();\n"
-               "  }\n"
+  verifyFormat("public class SomeClass\n"
+               "{\n"
+               "    void f()\n"
+               "    {\n"
+               "    }\n"
+               "    int g()\n"
+               "    {\n"
+               "        return 0;\n"
+               "    }\n"
+               "    void h()\n"
+               "    {\n"
+               "        while (true)\n"
+               "            f();\n"
+               "        for (;;)\n"
+               "            f();\n"
+               "        if (true)\n"
+               "            f();\n"
+               "    }\n"
                "}");
 }
 
 TEST_F(FormatTestCSharp, AccessModifiers) {
-  verifyFormat("public String toString() {}");
-  verifyFormat("private String toString() {}");
-  verifyFormat("protected String toString() {}");
-  verifyFormat("internal String toString() {}");
+  verifyFormat("public String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("private String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("protected String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("internal String toString()\n"
+               "{\n"
+               "}");
 
-  verifyFormat("public override String toString() {}");
-  verifyFormat("private override String toString() {}");
-  verifyFormat("protected override String toString() {}");
-  verifyFormat("internal override String toString() {}");
+  verifyFormat("public override String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("private override String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("protected override String toString()\n"
+               "{\n"
+               "}");
+  verifyFormat("internal override String toString()\n"
+               "{\n"
+               "}");
 
-  verifyFormat("internal static String toString() {}");
+  verifyFormat("internal static String toString()\n"
+               "{\n"
+               "}");
 }
 
 TEST_F(FormatTestCSharp, NoStringLiteralBreaks) {
@@ -124,45 +152,70 @@ TEST_F(FormatTestCSharp, CSharpNullConditional) {
 
   verifyFormat("switch(args?.Length)");
 
-  verifyFormat("public static void Main(string[] args) { string dirPath "
-               "= args?[0]; }");
+  verifyFormat("public static void Main(string[] args)\n"
+               "{\n"
+               "    string dirPath = args?[0];\n"
+               "}");
 }
 
 TEST_F(FormatTestCSharp, Attributes) {
   verifyFormat("[STAThread]\n"
-               "static void\n"
-               "Main(string[] args) {}");
+               "static void Main(string[] args)\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
-               "private class Test {}");
+               "private class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
-               "protected class Test {}");
+               "protected class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
-               "internal class Test {}");
+               "internal class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
-               "class Test {}");
+               "class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
                "[DeploymentItem(\"Test.txt\")]\n"
-               "public class Test {}");
+               "public class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[System.AttributeUsage(System.AttributeTargets.Method)]\n"
                "[System.Runtime.InteropServices.ComVisible(true)]\n"
-               "public sealed class STAThreadAttribute : Attribute {}");
+               "public sealed class STAThreadAttribute : Attribute\n"
+               "{\n"
+               "}");
 
   verifyFormat("[Verb(\"start\", HelpText = \"Starts the server listening on "
                "provided port\")]\n"
-               "class Test {}");
+               "class Test\n"
+               "{\n"
+               "}");
 
   verifyFormat("[TestMethod]\n"
-               "public string Host {\n  set;\n  get;\n}");
+               "public string Host\n"
+               "{\n"
+               "    set;\n"
+               "    get;\n"
+               "}");
 
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "listening on provided host\")]\n"
-               "public string Host {\n  set;\n  get;\n}");
+               "public string Host\n"
+               "{\n"
+               "    set;\n"
+               "    get;\n"
+               "}");
 }
 
 TEST_F(FormatTestCSharp, CSharpUsing) {
@@ -195,5 +248,57 @@ TEST_F(FormatTestCSharp, CSharpNullCoalescing) {
   verifyFormat("return _name ?? \"DEF\";");
 }
 
+TEST_F(FormatTestCSharp, AttributesIndentation) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+  Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
+
+  verifyFormat("[STAThread]\n"
+               "static void Main(string[] args)\n"
+               "{\n"
+               "}",
+               Style);
+
+  verifyFormat("[STAThread]\n"
+               "void "
+               "veryLooooooooooooooongFunctionName(string[] args)\n"
+               "{\n"
+               "}",
+               Style);
+
+  verifyFormat("[STAThread]\n"
+               "veryLoooooooooooooooooooongReturnType "
+               "veryLooooooooooooooongFunctionName(string[] args)\n"
+               "{\n"
+               "}",
+               Style);
+
+  verifyFormat("[SuppressMessage(\"A\", \"B\", Justification = \"C\")]\n"
+               "public override X Y()\n"
+               "{\n"
+               "}\n",
+               Style);
+
+  verifyFormat("[SuppressMessage]\n"
+               "public X Y()\n"
+               "{\n"
+               "}\n",
+               Style);
+
+  verifyFormat("[SuppressMessage]\n"
+               "public override X Y()\n"
+               "{\n"
+               "}\n",
+               Style);
+
+  verifyFormat("public A(B b) : base(b)\n"
+               "{\n"
+               "    [SuppressMessage]\n"
+               "    public override X Y()\n"
+               "    {\n"
+               "    }\n"
+               "}\n",
+               Style);
+}
+
 } // namespace format
 } // end namespace clang