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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371720: [clang-format] [PR43100] clang-format C#  support 
does not add a space between… (authored by paulhoad, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D2?vs=217167=219873#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestCSharp.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 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;
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"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 aaa a "
"aaa long region");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 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;
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"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 aaa a "
"aaa long region");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

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.




Comment at: clang/unittests/Format/FormatTestCSharp.cpp:168
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);

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"
   "}");
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 217167.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"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 aaa a "
"aaa long region");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 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;


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"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 aaa a "
"aaa long region");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 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;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

owenpan wrote:
> owenpan wrote:
> > Maybe set `SpaceBeforeParens` to `Always` first in order to really test the 
> > new behavior?
> I meant setting `SpaceBeforeParens` to ~~`Always`~~ `Never`. 
Also, the right parenthesis for the `using` statement is missing.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

owenpan wrote:
> Maybe set `SpaceBeforeParens` to `Always` first in order to really test the 
> new behavior?
I meant setting `SpaceBeforeParens` to ~~`Always`~~ `Never`. 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2618
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) && Right.is(tok::l_paren))
+  return true;

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



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D2/new/

https://reviews.llvm.org/D2



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: djasper, klimek, owenpan.
MyDeveloperDay added a project: clang.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D2

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,10 @@
"public string Host {\n  set;\n  get;\n}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaa a "
"aaa long region");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2614,6 +2614,9 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) && Right.is(tok::l_paren))
+  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,


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,10 @@
"public string Host {\n  set;\n  get;\n}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaa a "
"aaa long region");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2614,6 +2614,9 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) && Right.is(tok::l_paren))
+  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,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits