[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

llvm-svn: 373707
This commit is contained in:
Paul Hoad
2019-10-04 07:56:49 +00:00
parent 03b216d854
commit a2f963bb61
5 changed files with 189 additions and 69 deletions

View File

@@ -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();

View File

@@ -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;

View File

@@ -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;
}

View File

@@ -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;

View File

@@ -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