From: Paul Hoad Date: Fri, 4 Oct 2019 07:56:49 +0000 (+0000) Subject: [clang-format] [PR43333] Fix C# breaking before function name when using Attributes X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0884008c1c877510735e7441f2a08d8c9c52cae0;p=clang [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 d774c04b6d..7e71b7e8b1 100644 --- 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 T foo() { - /// } - /// template T foo(int aaaaaaaaaaaaaaaaaaaaa, - /// int bbbbbbbbbbbbbbbbbbbbb) { - /// } - /// \endcode - BTDS_No, - /// Force break after template declaration only when the following - /// declaration spans multiple lines. - /// \code - /// template T foo() { - /// } - /// template - /// T foo(int aaaaaaaaaaaaaaaaaaaaa, - /// int bbbbbbbbbbbbbbbbbbbbb) { - /// } - /// \endcode - BTDS_MultiLine, - /// Always break after template declaration. - /// \code - /// template - /// T foo() { - /// } - /// template - /// T foo(int aaaaaaaaaaaaaaaaaaaaa, - /// int bbbbbbbbbbbbbbbbbbbbb) { - /// } - /// \endcode - BTDS_Yes + /// Do not force break before declaration. + /// ``PenaltyBreakTemplateDeclaration`` is taken into account. + /// \code + /// template T foo() { + /// } + /// template T foo(int aaaaaaaaaaaaaaaaaaaaa, + /// int bbbbbbbbbbbbbbbbbbbbb) { + /// } + /// \endcode + BTDS_No, + /// Force break after template declaration only when the following + /// declaration spans multiple lines. + /// \code + /// template T foo() { + /// } + /// template + /// T foo(int aaaaaaaaaaaaaaaaaaaaa, + /// int bbbbbbbbbbbbbbbbbbbbb) { + /// } + /// \endcode + BTDS_MultiLine, + /// Always break after template declaration. + /// \code + /// template + /// T foo() { + /// } + /// template + /// 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 52afb6744a..2ff6e5ec23 100644 --- 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 5b267784ff..cd44c0be85 100644 --- 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 91ac9822c8..319c66c1ea 100644 --- 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 7fda9566dc..031de14338 100644 --- 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