Treat C# `using` as a control statement

[jbcoe]

Summary

Unless SpaceBeforeParensOptions is set to SBPO_Never, a space will be put between using and ( in C# code.

[MyDeveloperDay]

LGTM

[krasimir]

clang/unittests/Format/FormatTestCSharp.cpp

261

Please also add a test for SBPO_NonEmptyParentheses.

263

Can this appear at the top-level? If not, just keep the other test case to avoid confusion.

[jbcoe]

Handle using case where SpaceBeforeParensOptions is set to SBPO_NonEmptyParentheses

[jbcoe]

clang/unittests/Format/FormatTestCSharp.cpp

263

I'm following the example set by by test cases above. I can remove (all) if you prefer and agree it does not test anything new.

[krasimir]

clang/unittests/Format/FormatTestCSharp.cpp

263

I didn't notice the old example. Yes, please remove all of them.

[jbcoe]

Removed test snippets that did not exercise new code.

[MyDeveloperDay]

In D72144#1834265, @jbcoe wrote:

Removed test snippets that did not exercise new code.

Actually i would prefer we didn't remove those tests, because we added them because previously someone called out that it had that effect when at the top level

because clang-format can work on a snippet of code it may be that you don't see the outer scope, I see those tests doing no harm and they assert the behaviour that was previously seen to be different for both in scope and toplevel

[jbcoe]

Extended tests and added some comments explaining why seeming duplicated tests are useful.

[krasimir]

I looked up the C# reference and all the examples are top-level, so having top-level test cases here would be useful, as people would expect copy-pasting an example snippet to just work.
Sorry about asking you to go back-and-forth.

[krasimir]

I'll submit this for you.

clang/lib/Format/TokenAnnotator.cpp

if (Left.is(tok::numeric_constant) && Right.is(tok::percent))
       return Right.WhitespaceRange.getEnd() != Right.WhitespaceRange.getBegin();
   } 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);
+        return Style.SpaceBeforeParens == FormatStyle::SBPO_ControlStatements ||
+               spaceRequiredBeforeParens(Right);
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
     if (Left.is(TT_JsFatArrow))
       return true;
     // for await ( ...
     if (Right.is(tok::l_paren) && Left.is(Keywords.kw_await) && Left.Previous &&
         Left.Previous->is(tok::kw_for))
       return true;
     if (Left.is(Keywords.kw_async) && Right.is(tok::l_paren) &&

clang/unittests/Format/FormatTestCSharp.cpp

verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
                "}");
 }
 
 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"
+               "  using () {}\n"
                "}",
                Style);
 
+  // Ensure clang-format affects top-level snippets correctly.
   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"
+               "  using() {}\n"
                "}",
                Style);
 
-  verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
+  // Ensure clang-format affects top-level snippets correctly.
+  verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
+               Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  verifyFormat("public void foo() {\n"
+               "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
+               "  using () {}\n"
+               "}",
+               Style);
+
+  // Ensure clang-format affects top-level snippets correctly.
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filenameB)) {}",
+               Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+  verifyFormat("public void foo() {\n"
+               "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
+               "  using() {}\n"
+               "}",
+               Style);
+
+  // Ensure clang-format affects top-level snippets correctly.
+  verifyFormat("using (StreamWriter sw = new StreamWriter (filenameB)) {}",
                Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaa "
                "aaaaaaaaaaaaaaa long region");
 }