[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0734fb21ed5e: clang-format: [JS] Handle more keyword-named 
methods. (authored by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D72827?vs=238442&id=238713#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72827

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


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -358,6 +358,22 @@
   "x();\n"
   "  }\n"
   "}\n");
+  verifyFormat("class KeywordNamedMethods {\n"
+   "  do() {\n"
+   "  }\n"
+   "  for() {\n"
+   "  }\n"
+   "  while() {\n"
+   "  }\n"
+   "  if() {\n"
+   "  }\n"
+   "  else() {\n"
+   "  }\n"
+   "  try() {\n"
+   "  }\n"
+   "  catch() {\n"
+   "  }\n"
+   "}\n");
 }
 
 TEST_F(FormatTestJS, ReservedWordsParenthesized) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1011,13 +1011,22 @@
   parseAccessSpecifier();
 return;
   case tok::kw_if:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseIfThenElse();
 return;
   case tok::kw_for:
   case tok::kw_while:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseForOrWhileLoop();
 return;
   case tok::kw_do:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseDoWhile();
 return;
   case tok::kw_switch:
@@ -1045,6 +1054,9 @@
 return;
   case tok::kw_try:
   case tok::kw___try:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseTryCatch();
 return;
   case tok::kw_extern:
@@ -1290,6 +1302,12 @@
   // element continues.
   break;
 case tok::kw_try:
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+  Line->MustBeDeclaration) {
+// field/method declaration.
+nextToken();
+break;
+  }
   // We arrive here when parsing function-try blocks.
   if (Style.BraceWrapping.AfterFunction)
 addUnwrappedLine();


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -358,6 +358,22 @@
   "x();\n"
   "  }\n"
   "}\n");
+  verifyFormat("class KeywordNamedMethods {\n"
+   "  do() {\n"
+   "  }\n"
+   "  for() {\n"
+   "  }\n"
+   "  while() {\n"
+   "  }\n"
+   "  if() {\n"
+   "  }\n"
+   "  else() {\n"
+   "  }\n"
+   "  try() {\n"
+   "  }\n"
+   "  catch() {\n"
+   "  }\n"
+   "}\n");
 }
 
 TEST_F(FormatTestJS, ReservedWordsParenthesized) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1011,13 +1011,22 @@
   parseAccessSpecifier();
 return;
   case tok::kw_if:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseIfThenElse();
 return;
   case tok::kw_for:
   case tok::kw_while:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseForOrWhileLoop();
 return;
   case tok::kw_do:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseDoWhile();
 return;
   case tok::kw_switch:
@@ -1045,6 +1054,9 @@
 return;
   case tok::kw_try:
   case tok::kw___try:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseTryCatch();
 return;
   case tok::kw_extern:
@@ -1290,6 +1302,12 @@
   // element continues.
   break;
 case tok::kw_try:
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+  Line->MustBeDeclaration) {
+// 

[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[Martin Probst via Phabricator via cfe-commits]

mprobst added a comment.

I've added tests and a fix for try/catch and if/else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72827



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

[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[Martin Probst via Phabricator via cfe-commits]

mprobst added a comment.

In D72827#1823895 , @krasimir wrote:

> How about `if` and `try`? Is there a list somewhere for all such valid 
> identifiers?


This is a bit annoying - generally speaking, we should parse all keywords in 
declaration locations as simple names. However `UnwrappedLineParser` isn't 
really structured to support that, nor is it precise enough to reliably detect 
these situations :-/ So I think the best we can do is incrementally detect 
these issues and fix as they come up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72827



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

[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM thanks for the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72827



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

[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[Krasimir Georgiev via Phabricator via cfe-commits]

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

How about `if` and `try`? Is there a list somewhere for all such valid 
identifiers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72827



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

[PATCH] D72827: clang-format: [JS] Handle keyword-named methods.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: krasimir.
Herald added a project: clang.

Including `do`, `for`, and `while`, in addition to the previously
handled fields. The unit test explicitly uses methods, but this code
path handles both fields and methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72827

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


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -343,6 +343,14 @@
"  x: 'x'\n"
"};",
"const Axis = {for: 'for', x:   'x'};");
+  verifyFormat("class KeywordNamedMethods {\n"
+   "  do() {\n"
+   "  }\n"
+   "  for() {\n"
+   "  }\n"
+   "  while() {\n"
+   "  }\n"
+   "}\n");
 }
 
 TEST_F(FormatTestJS, ReservedWordsMethods) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1015,9 +1015,15 @@
 return;
   case tok::kw_for:
   case tok::kw_while:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseForOrWhileLoop();
 return;
   case tok::kw_do:
+if (Style.Language == FormatStyle::LK_JavaScript && 
Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseDoWhile();
 return;
   case tok::kw_switch:


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -343,6 +343,14 @@
"  x: 'x'\n"
"};",
"const Axis = {for: 'for', x:   'x'};");
+  verifyFormat("class KeywordNamedMethods {\n"
+   "  do() {\n"
+   "  }\n"
+   "  for() {\n"
+   "  }\n"
+   "  while() {\n"
+   "  }\n"
+   "}\n");
 }
 
 TEST_F(FormatTestJS, ReservedWordsMethods) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1015,9 +1015,15 @@
 return;
   case tok::kw_for:
   case tok::kw_while:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseForOrWhileLoop();
 return;
   case tok::kw_do:
+if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
+  // field/method declaration.
+  break;
 parseDoWhile();
 return;
   case tok::kw_switch:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits