[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<string,string> foo;
}

```
will be formatted as

```
public class Foo
{
    Dictionary<string, string>foo;
                           ^^^^^   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

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

diff --git a/lib/Format/FormatTokenLexer.cpp b/lib/Format/FormatTokenLexer.cpp
index e59a059fd6b23cff59fefc8005c9d3ff1ccf0f16..5d8a77577c0beeeee2459b676b86f2b883e6b7be 100644 (file)
--- a/lib/Format/FormatTokenLexer.cpp
+++ b/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/lib/Format/FormatTokenLexer.h b/lib/Format/FormatTokenLexer.h
index 1e096fc5020566864d052ef7d44d356fd45f7189..611211be055a2284d3a47ab76837803c8aaf15fb 100644 (file)
--- a/lib/Format/FormatTokenLexer.h
+++ b/lib/Format/FormatTokenLexer.h
@@ -53,6 +53,7 @@ private:
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
+  bool tryTransformCSharpForEach();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 319c66c1ead2dbd0d901de1be5192fbc999f6989..e6dfffd9733449476519f82387482f3ba23110a4 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/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<string,string> 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/unittests/Format/FormatTestCSharp.cpp b/unittests/Format/FormatTestCSharp.cpp
index 031de143389f869f3c4af9105b6230533089104c..a2f381078d42df4dbc47f0d47e32c0caa4272a59 100644 (file)
--- a/unittests/Format/FormatTestCSharp.cpp
+++ b/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<string> list;", Style);
+  verifyFormat("Dictionary<string, string> 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<string> list;", Style);
+  verifyFormat("Dictionary<string, string> 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