Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] js handle anonymous classes #106242

Merged
merged 5 commits into from
Aug 28, 2024
Merged

[clang-format] js handle anonymous classes #106242

merged 5 commits into from
Aug 28, 2024

Conversation

krasimirgg
Copy link
Contributor

Addresses a regression in JavaScript when formatting anonymous classes.

Addresses a regression in JavaScript when formatting anonymous classes.
@krasimirgg krasimirgg requested a review from kadircet August 27, 2024 16:12
@krasimirgg krasimirgg marked this pull request as ready for review August 27, 2024 16:13
@llvmbot
Copy link
Member

llvmbot commented Aug 27, 2024

@llvm/pr-subscribers-clang-format

Author: Krasimir Georgiev (krasimirgg)

Changes

Addresses a regression in JavaScript when formatting anonymous classes.


Full diff: https://github.com/llvm/llvm-project/pull/106242.diff

2 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+5-1)
  • (modified) clang/unittests/Format/FormatTestJS.cpp (+9)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 5f1a88d4bcd729..7591eaeae461a7 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3992,6 +3992,9 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   auto IsNonMacroIdentifier = [](const FormatToken *Tok) {
     return Tok->is(tok::identifier) && Tok->TokenText != Tok->TokenText.upper();
   };
+  // JavaScript/TypeScript supports anonymous classes like:
+  // a = class extends foo { }
+  bool JSPastExtendsOrImplements = false;
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
   // An [[attribute]] can be before the identifier.
@@ -4002,6 +4005,7 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
           FormatTok->isOneOf(tok::period, tok::comma))) {
     if (Style.isJavaScript() &&
         FormatTok->isOneOf(Keywords.kw_extends, Keywords.kw_implements)) {
+      JSPastExtendsOrImplements = true;
       // JavaScript/TypeScript supports inline object types in
       // extends/implements positions:
       //     class Foo implements {bar: number} { }
@@ -4027,7 +4031,7 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
     case tok::coloncolon:
       break;
     default:
-      if (!ClassName && Previous->is(tok::identifier) &&
+      if (!JSPastExtendsOrImplements && !ClassName && Previous->is(tok::identifier) &&
           Previous->isNot(TT_AttributeMacro)) {
         ClassName = Previous;
       }
diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp
index b910ce620de7a9..734c1590c41ddb 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -579,6 +579,15 @@ TEST_F(FormatTestJS, GoogScopes) {
                "});");
 }
 
+TEST_F(FormatTestJS, GoogAnonymousClass) {
+  verifyFormat("a = class extends goog.structs.a {\n"
+               "  a() {\n"
+               "    return 0;\n"
+               "  }\n"
+               "};");
+}
+
+
 TEST_F(FormatTestJS, IIFEs) {
   // Internal calling parens; no semi.
   verifyFormat("(function() {\n"

Copy link

github-actions bot commented Aug 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@owenca owenca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG except some nits.

clang/unittests/Format/TokenAnnotatorTest.cpp Outdated Show resolved Hide resolved
@krasimirgg krasimirgg merged commit 77d63cf into llvm:main Aug 28, 2024
8 checks passed
@kadircet kadircet added this to the LLVM 19.X Release milestone Aug 28, 2024
@kadircet
Copy link
Member

i think we should als oget this cherry-picked into the release branch, wdyt @owenca ?

/cherry-pick 77d63cf

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 28, 2024
Addresses a regression in JavaScript when formatting anonymous classes.

---------

Co-authored-by: Owen Pan <[email protected]>
(cherry picked from commit 77d63cf)
@llvmbot
Copy link
Member

llvmbot commented Aug 28, 2024

/pull-request #106390

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Sep 1, 2024
Addresses a regression in JavaScript when formatting anonymous classes.

---------

Co-authored-by: Owen Pan <[email protected]>
(cherry picked from commit 77d63cf)
kadircet added a commit to kadircet/llvm-project that referenced this pull request Sep 13, 2024
This extends the fix in llvm#106242
for other derived class types.
kadircet added a commit to kadircet/llvm-project that referenced this pull request Sep 16, 2024
This extends the fix in llvm#106242
for other derived class types.
kadircet added a commit that referenced this pull request Sep 16, 2024
…st (#108524)

This extends the fix in #106242
for other derived class types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants