r373707 - [clang-format] [PR43333] Fix C# breaking before function name when using Attributes
Paul Hoad via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 4 00:56:49 PDT 2019
Author: paulhoad
Date: Fri Oct 4 00:56:49 2019
New Revision: 373707
URL: http://llvm.org/viewvc/llvm-project?rev=373707&view=rev
Log:
[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
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestCSharp.cpp
Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=373707&r1=373706&r2=373707&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Fri Oct 4 00:56:49 2019
@@ -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();
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=373707&r1=373706&r2=373707&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Oct 4 00:56:49 2019
@@ -473,7 +473,10 @@ bool ContinuationIndenter::mustBreak(con
}
// 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;
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=373707&r1=373706&r2=373707&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri Oct 4 00:56:49 2019
@@ -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(FormatStyl
Style.AllowShortCaseLabelsOnASingleLine = false;
Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
Style.AllowShortLoopsOnASingleLine = false;
+ Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
return Style;
}
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373707&r1=373706&r2=373707&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct 4 00:56:49 2019
@@ -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;
Modified: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestCSharp.cpp?rev=373707&r1=373706&r2=373707&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp Fri Oct 4 00:56:49 2019
@@ -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 override String toString() {}");
- verifyFormat("private override String toString() {}");
- verifyFormat("protected override String toString() {}");
- verifyFormat("internal override 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("internal static 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()\n"
+ "{\n"
+ "}");
}
TEST_F(FormatTestCSharp, NoStringLiteralBreaks) {
@@ -124,45 +152,70 @@ TEST_F(FormatTestCSharp, CSharpNullCondi
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, CSharpNullCoale
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
More information about the cfe-commits
mailing list