[clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

[MyDeveloperDay]

Summary

Addresses https://bugs.llvm.org/show_bug.cgi?id=43100

Formatting using statement in C# with clang-format removes the space between using and paren even when SpaceBeforeParens is !

using(FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize : 1))

this change simply overcomes this for when using C# settings in the .clang-format file

using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize : 1))

All FormatTests pass..

[==========] 688 tests from 21 test cases ran. (88508 ms total)
[  PASSED  ] 688 tests.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

2622

if (Style.isCSharp() && Left.is(tok::kw_using)) would suffice as Right.is(tok::l_paren) is already checked on Line 2613.

clang/unittests/Format/FormatTestCSharp.cpp

169

Maybe set SpaceBeforeParens to Always first in order to really test the new behavior?

[owenpan]

clang/unittests/Format/FormatTestCSharp.cpp

169

I meant setting SpaceBeforeParens to Always Never.

[owenpan]

clang/unittests/Format/FormatTestCSharp.cpp

169

Also, the right parenthesis for the using statement is missing.

[owenpan]

The test case fails after the missing ) is added, so it seems that the patch has no effect.

[owenpan]

The patch would have no effect if the using statement is not in a block, but that would probably be against C# syntax.

Please see my comments about the test cases. Otherwise, LGTM.

clang/unittests/Format/FormatTestCSharp.cpp

168

I'd add the following as the first test case because it tests the new behavior. The other two test cases only verify that the patch does not cause regressions. (The SBPO_Always case may be redundant if it's already covered in FormatTest.cpp.)

// SpaceBeforeParens: ControlStatements
verifyFormat("public void foo() {\n"
             "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
             "}");

clang/lib/Format/TokenAnnotator.cpp

if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
       (Right.is(tok::r_brace) && Right.MatchingParen &&
        Right.MatchingParen->BlockKind != BK_Block))
     return !Style.Cpp11BracedListStyle;
   if (Left.is(TT_BlockComment))
     // No whitespace in x(/*foo=*/1), except for JavaScript.
     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;
     return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
            (Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
             (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
                           tok::kw_switch, tok::kw_case, TT_ForEachMacro,
                           TT_ObjCForIn) ||
              Left.isIf(Line.Type != LT_PreprocessorDirective) ||
              (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
                            tok::kw_new, tok::kw_delete) &&
               (!Left.Previous || Left.Previous->isNot(tok::period))))) ||
            (spaceRequiredBeforeParens(Right) &&

clang/unittests/Format/FormatTestCSharp.cpp

TEST_F(FormatTestCSharp, Attributes) {
   verifyFormat("[TestMethod]\n"
                "public string Host {\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}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("public void foo() {\n"
+               "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
+               "}",
+               Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("public void foo() {\n"
+               "  using(StreamWriter sw = new StreamWriter(filenameB)) {}\n"
+               "}",
+               Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaa "
                "aaaaaaaaaaaaaaa long region");
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
   verifyFormat("public enum var { none, @string, bool, @enum }");
 }