[PATCH] D141547: Fix format for `case` in .proto files

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaf393da1823: Fix format for `case` in .proto files 
(authored by fowles, committed by krasimir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141547

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestProto.cpp


Index: clang/unittests/Format/FormatTestProto.cpp
===
--- clang/unittests/Format/FormatTestProto.cpp
+++ clang/unittests/Format/FormatTestProto.cpp
@@ -113,6 +113,13 @@
"}");
 }
 
+TEST_F(FormatTestProto, CaseAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required string case = 1;\n"
+   "  repeated int32 fizz = 2;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();


Index: clang/unittests/Format/FormatTestProto.cpp
===
--- clang/unittests/Format/FormatTestProto.cpp
+++ clang/unittests/Format/FormatTestProto.cpp
@@ -113,6 +113,13 @@
"}");
 }
 
+TEST_F(FormatTestProto, CaseAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required string case = 1;\n"
+   "  repeated int32 fizz = 2;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141547: Fix format for `case` in .proto files

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir accepted this revision.
krasimir added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141547

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

[PATCH] D141547: Fix format for `case` in .proto files

[Matt Kulukundis via Phabricator via cfe-commits]

fowles updated this revision to Diff 488979.
fowles added a comment.

Move from lit test to unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141547

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestProto.cpp


Index: clang/unittests/Format/FormatTestProto.cpp
===
--- clang/unittests/Format/FormatTestProto.cpp
+++ clang/unittests/Format/FormatTestProto.cpp
@@ -113,6 +113,13 @@
"}");
 }
 
+TEST_F(FormatTestProto, CaseAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required string case = 1;\n"
+   "  repeated int32 fizz = 2;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();


Index: clang/unittests/Format/FormatTestProto.cpp
===
--- clang/unittests/Format/FormatTestProto.cpp
+++ clang/unittests/Format/FormatTestProto.cpp
@@ -113,6 +113,13 @@
"}");
 }
 
+TEST_F(FormatTestProto, CaseAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required string case = 1;\n"
+   "  repeated int32 fizz = 2;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141547: Fix format for `case` in .proto files

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clang/test/Format/case.proto:15
+repeated int32 fizz = 1;
+}

could you please turn this into an unit test here instead > 
https://github.com/llvm/llvm-project/blob/main/clang/unittests/Format/FormatTestProto.cpp.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141547

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

[PATCH] D141547: Fix format for `case` in .proto files

[Eric Christopher via Phabricator via cfe-commits]

echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141547

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

[PATCH] D141547: Fix format for `case` in .proto files

[Matt Kulukundis via Phabricator via cfe-commits]

fowles created this revision.
fowles added a reviewer: krasimir.
Herald added a project: All.
fowles requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix format for `case` in .proto files


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141547

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/test/Format/case.proto


Index: clang/test/Format/case.proto
===
--- /dev/null
+++ clang/test/Format/case.proto
@@ -0,0 +1,15 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -assume-filename=case.proto -style=Google \
+// RUN:   | FileCheck -strict-whitespace %s
+
+syntax = "proto2";
+
+package foo.bar;
+
+message Baz {
+  optional int64 fred = 1;
+// CHECK: {{^\ {2}required}}
+required string case = 2;
+// CHECK: {{^\ {2}repeated}}
+repeated int32 fizz = 1;
+}
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();


Index: clang/test/Format/case.proto
===
--- /dev/null
+++ clang/test/Format/case.proto
@@ -0,0 +1,15 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -assume-filename=case.proto -style=Google \
+// RUN:   | FileCheck -strict-whitespace %s
+
+syntax = "proto2";
+
+package foo.bar;
+
+message Baz {
+  optional int64 fred = 1;
+// CHECK: {{^\ {2}required}}
+required string case = 2;
+// CHECK: {{^\ {2}repeated}}
+repeated int32 fizz = 1;
+}
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -590,8 +590,9 @@
   [[fallthrough]];
 }
 case tok::kw_case:
-  if (Style.isVerilog() ||
+  if (Style.isProto() || Style.isVerilog() ||
   (Style.isJavaScript() && Line->MustBeDeclaration)) {
+// Proto: there are no switch/case statements
 // Verilog: Case labels don't have this word. We handle case
 // labels including default in TokenAnnotator.
 // JavaScript: A 'case: string' style field declaration.
@@ -1620,7 +1621,11 @@
 // e.g. "default void f() {}" in a Java interface.
 break;
   case tok::kw_case:
-// In Verilog switch is called case.
+// Proto: there are no switch/case statements.
+if (Style.isProto()) {
+  nextToken();
+  return;
+}
 if (Style.isVerilog()) {
   parseBlock();
   addUnwrappedLine();
@@ -2100,6 +2105,11 @@
   parseNew();
   break;
 case tok::kw_case:
+  // Proto: there are no switch/case statements.
+  if (Style.isProto()) {
+nextToken();
+return;
+  }
   // In Verilog switch is called case.
   if (Style.isVerilog()) {
 parseBlock();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits