From 4c056583548ab59e2ad24deb2ca30790a290a8e3 Mon Sep 17 00:00:00 2001 From: Paul Hoad Date: Fri, 4 Oct 2019 08:10:22 +0000 Subject: [PATCH] [clang-format] [PR43338] C# clang format has space issues betweern C# only keywords Summary: When formatting C# there can be issues with a lack of spaces between `using (` , `foreach (` and generic types The C# code ``` public class Foo { Dictionary foo; } ``` will be formatted as ``` public class Foo { Dictionaryfoo; ^^^^^ missing a space } ``` This revision also reverts some of {D66662} in order to make this cleaner and resolve an issues seen by @owenpan that the formatting didn't add a space when not in a code block This also transforms C# foreach commands to be seen as tok::kw_for commands (to ensure foreach gets the same Brace Wrapping behavior as for without littering the code with `if(Style.isCSharp())` Reviewers: owenpan, klimek, russellmcc, mitchell-stellar Reviewed By: mitchell-stellar Subscribers: cfe-commits Tags: #clang, #clang-format Differential Revision: https://reviews.llvm.org/D67660 llvm-svn: 373709 --- clang/lib/Format/FormatTokenLexer.cpp | 17 +++++++++ clang/lib/Format/FormatTokenLexer.h | 1 + clang/lib/Format/TokenAnnotator.cpp | 14 ++++--- clang/unittests/Format/FormatTestCSharp.cpp | 57 ++++++++++++++++++++++++++++- 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/FormatTokenLexer.cpp b/clang/lib/Format/FormatTokenLexer.cpp index e59a059..5d8a775 100644 --- a/clang/lib/Format/FormatTokenLexer.cpp +++ b/clang/lib/Format/FormatTokenLexer.cpp @@ -80,6 +80,8 @@ void FormatTokenLexer::tryMergePreviousTokens() { return; if (tryMergeCSharpNullConditionals()) return; + if (tryTransformCSharpForEach()) + return; static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater}; if (tryMergeTokens(JSRightArrow, TT_JsFatArrow)) return; @@ -254,6 +256,21 @@ bool FormatTokenLexer::tryMergeCSharpNullConditionals() { return true; } +// In C# transform identifier foreach into kw_foreach +bool FormatTokenLexer::tryTransformCSharpForEach() { + if (Tokens.size() < 1) + return false; + auto &Identifier = *(Tokens.end() - 1); + if (!Identifier->is(tok::identifier)) + return false; + if (Identifier->TokenText != "foreach") + return false; + + Identifier->Type = TT_ForEachMacro; + Identifier->Tok.setKind(tok::kw_for); + return true; +} + bool FormatTokenLexer::tryMergeLessLess() { // Merge X,less,less,Y into X,lessless,Y unless X or Y is less. if (Tokens.size() < 3) diff --git a/clang/lib/Format/FormatTokenLexer.h b/clang/lib/Format/FormatTokenLexer.h index 1e096fc..611211b 100644 --- a/clang/lib/Format/FormatTokenLexer.h +++ b/clang/lib/Format/FormatTokenLexer.h @@ -53,6 +53,7 @@ private: bool tryMergeCSharpKeywordVariables(); bool tryMergeCSharpNullConditionals(); bool tryMergeCSharpDoubleQuestion(); + bool tryTransformCSharpForEach(); bool tryMergeTokens(ArrayRef Kinds, TokenType NewType); diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 319c66c..e6dfffd 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -2640,10 +2640,6 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, return Style.Language == FormatStyle::LK_JavaScript || !Left.TokenText.endswith("=*/"); if (Right.is(tok::l_paren)) { - // using (FileStream fs... - if (Style.isCSharp() && Left.is(tok::kw_using) && - Style.SpaceBeforeParens != FormatStyle::SBPO_Never) - return true; if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) || (Left.is(tok::r_square) && Left.is(TT_AttributeSquare))) return true; @@ -2742,7 +2738,15 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line, // and "%d %d" if (Left.is(tok::numeric_constant) && Right.is(tok::percent)) return Right.WhitespaceRange.getEnd() != Right.WhitespaceRange.getBegin(); - } else if (Style.Language == FormatStyle::LK_JavaScript || Style.isCSharp()) { + } else if (Style.isCSharp()) { + // space between type and variable e.g. Dictionary foo; + if (Left.is(TT_TemplateCloser) && Right.is(TT_StartOfName)) + return true; + // space between keywords and paren e.g. "using (" + if (Right.is(tok::l_paren)) + if (Left.is(tok::kw_using)) + return spaceRequiredBeforeParens(Left); + } else if (Style.Language == FormatStyle::LK_JavaScript) { if (Left.is(TT_JsFatArrow)) return true; // for await ( ... diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp index 031de14..a2f3810 100644 --- a/clang/unittests/Format/FormatTestCSharp.cpp +++ b/clang/unittests/Format/FormatTestCSharp.cpp @@ -147,15 +147,27 @@ TEST_F(FormatTestCSharp, CSharpFatArrows) { } TEST_F(FormatTestCSharp, CSharpNullConditional) { + FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp); + Style.SpaceBeforeParens = FormatStyle::SBPO_Always; + verifyFormat( "public Person(string firstName, string lastName, int? age=null)"); - verifyFormat("switch(args?.Length)"); + verifyFormat("foo () {\n" + " switch (args?.Length) {}\n" + "}", + Style); + + verifyFormat("switch (args?.Length) {}", Style); verifyFormat("public static void Main(string[] args)\n" "{\n" " string dirPath = args?[0];\n" "}"); + + Style.SpaceBeforeParens = FormatStyle::SBPO_Never; + + verifyFormat("switch(args?.Length) {}", Style); } TEST_F(FormatTestCSharp, Attributes) { @@ -221,16 +233,22 @@ TEST_F(FormatTestCSharp, Attributes) { TEST_F(FormatTestCSharp, CSharpUsing) { FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp); Style.SpaceBeforeParens = FormatStyle::SBPO_Always; - verifyFormat("public void foo() {\n" + verifyFormat("public void foo () {\n" " using (StreamWriter sw = new StreamWriter (filenameA)) {}\n" "}", Style); + verifyFormat("using (StreamWriter sw = new StreamWriter (filenameB)) {}", + Style); + Style.SpaceBeforeParens = FormatStyle::SBPO_Never; verifyFormat("public void foo() {\n" " using(StreamWriter sw = new StreamWriter(filenameB)) {}\n" "}", Style); + + verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}", + Style); } TEST_F(FormatTestCSharp, CSharpRegions) { @@ -300,5 +318,40 @@ TEST_F(FormatTestCSharp, AttributesIndentation) { Style); } +TEST_F(FormatTestCSharp, CSharpSpaceBefore) { + FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp); + Style.SpaceBeforeParens = FormatStyle::SBPO_Always; + + verifyFormat("List list;", Style); + verifyFormat("Dictionary dict;", Style); + + verifyFormat("for (int i = 0; i < size (); i++) {\n" + "}", + Style); + verifyFormat("foreach (var x in y) {\n" + "}", + Style); + verifyFormat("switch (x) {}", Style); + verifyFormat("do {\n" + "} while (x);", + Style); + + Style.SpaceBeforeParens = FormatStyle::SBPO_Never; + + verifyFormat("List list;", Style); + verifyFormat("Dictionary dict;", Style); + + verifyFormat("for(int i = 0; i < size(); i++) {\n" + "}", + Style); + verifyFormat("foreach(var x in y) {\n" + "}", + Style); + verifyFormat("switch(x) {}", Style); + verifyFormat("do {\n" + "} while(x);", + Style); +} + } // namespace format } // end namespace clang -- 2.7.4