This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by MyDeveloperDay on Aug 23 2019, 8:57 AM.

Details

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.

Diff Detail

Repository
rC Clang

Event Timeline

MyDeveloperDay created this revision.Aug 23 2019, 8:57 AM
owenpan added inline comments.Aug 23 2019, 3:44 PM
clang/lib/Format/TokenAnnotator.cpp
2618

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 added inline comments.Aug 23 2019, 8:28 PM
clang/unittests/Format/FormatTestCSharp.cpp
169

I meant setting SpaceBeforeParens to Always Never.

owenpan added inline comments.Aug 24 2019, 12:32 AM
clang/unittests/Format/FormatTestCSharp.cpp
169

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

owenpan added a comment.EditedAug 24 2019, 10:33 PM

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

MyDeveloperDay marked 4 inline comments as done.

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"
             "}");
owenpan accepted this revision.Aug 26 2019, 11:55 PM
This revision is now accepted and ready to land.Aug 26 2019, 11:55 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 12 2019, 3:18 AM