AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

[MyDeveloperDay]

Summary

https://github.com/llvm/llvm-project/issues/49846

Fixes #49846

AlignConsecutiveDeclarations is not working for "let" and "const" in JavaScript

let letVariable = 5;
const constVariable = 10;

[MyDeveloperDay]

Double check the interaction with the AlignConsecutiveAssignments

[MyDeveloperDay]

clang-format the file

[curdeius]

LGTM! Optional nit comment.

clang/lib/Format/TokenAnnotator.cpp

1874

Nit.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

1839

Remove braces.

1874

+1

[MyDeveloperDay]

clang/lib/Format/TokenAnnotator.cpp

1874

I WILL make the change, but just my 2c worth, when a complex expression is on the last line (as was before), it's a bu***er to debug! I personally use this style so I can just drop a breakpoint on the return and I know its being hit for that case, in clang-format where these functions are called over and over again that's super important... (that's my defend your honour!)

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

1874

Alright! I withdraw my +1 then. :)

[curdeius]

clang/lib/Format/TokenAnnotator.cpp

1874

@MyDeveloperDay, it's up to you whether to change it or not.

clang/include/clang/Format/Format.h

enum LanguageKind : unsigned char {
     LK_TableGen,
     /// Should be used for Protocol Buffer messages in text format
     /// (https://developers.google.com/protocol-buffers/).
     LK_TextProto
   };
   bool isCpp() const { return Language == LK_Cpp || Language == LK_ObjC; }
   bool isCSharp() const { return Language == LK_CSharp; }
   bool isJson() const { return Language == LK_Json; }
+  bool isJavaScript() const { return Language == LK_JavaScript; }
 
   /// Language, this format style is targeted at.
   /// \version 3.5
   LanguageKind Language;
 
   /// Indentation logic for lambda bodies.
   enum LambdaBodyIndentationKind : unsigned char {
     /// Align lambda body relative to the lambda signature. This is the default.

clang/lib/Format/TokenAnnotator.cpp

private:
   /// something that is likely a type.
   bool isStartOfName(const FormatToken &Tok) {
     if (Tok.isNot(tok::identifier) || !Tok.Previous)
       return false;
 
     if (Tok.Previous->isOneOf(TT_LeadingJavaAnnotation, Keywords.kw_instanceof,
                               Keywords.kw_as))
       return false;
-    if (Style.Language == FormatStyle::LK_JavaScript &&
-        Tok.Previous->is(Keywords.kw_in))
+    if (Style.isJavaScript() && Tok.Previous->is(Keywords.kw_in))
       return false;
 
     // Skip "const" as it does not have an influence on whether this is a name.
     FormatToken *PreviousNotConst = Tok.getPreviousNonComment();
+
+    // For javascript const can be like "let" or "var"
+    if (!Style.isJavaScript())
       while (PreviousNotConst && PreviousNotConst->is(tok::kw_const))
         PreviousNotConst = PreviousNotConst->getPreviousNonComment();
 
     if (!PreviousNotConst)
       return false;
 
     bool IsPPKeyword = PreviousNotConst->is(tok::identifier) &&
                        PreviousNotConst->Previous &&
                        PreviousNotConst->Previous->is(tok::hash);
 
     if (PreviousNotConst->is(TT_TemplateCloser))
       return PreviousNotConst && PreviousNotConst->MatchingParen &&
              PreviousNotConst->MatchingParen->Previous &&
              PreviousNotConst->MatchingParen->Previous->isNot(tok::period) &&
              PreviousNotConst->MatchingParen->Previous->isNot(tok::kw_template);
 
     if (PreviousNotConst->is(tok::r_paren) &&
         PreviousNotConst->is(TT_TypeDeclarationParen))
       return true;
 
-    return (!IsPPKeyword &&
-            PreviousNotConst->isOneOf(tok::identifier, tok::kw_auto)) ||
-           PreviousNotConst->is(TT_PointerOrReference) ||
-           PreviousNotConst->isSimpleTypeSpecifier();
+    // If is a preprocess keyword like #define.
+    if (IsPPKeyword)
+      return false;
+
+    // int a or auto a.
+    if (PreviousNotConst->isOneOf(tok::identifier, tok::kw_auto))
+      return true;
+
+    // *a or &a or &&a.
+    if (PreviousNotConst->is(TT_PointerOrReference))
+      return true;
+
+    // MyClass a;
+    if (PreviousNotConst->isSimpleTypeSpecifier())
+      return true;
+
+    // const a = in JavaScript.
+    return (Style.isJavaScript() && PreviousNotConst->is(tok::kw_const));
   }
 
   /// Determine whether ')' is ending a cast.
   bool rParenEndsCast(const FormatToken &Tok) {
     // C-style casts are only used in C++, C# and Java.
     if (!Style.isCSharp() && !Style.isCpp() &&
         Style.Language != FormatStyle::LK_Java)
       return false;

clang/unittests/Format/FormatTestJS.cpp

verifyFormat(
       "this is ExtendedState<DependencyState&ExportedState>;\n"
       "}\n");
 }
 
 TEST_F(FormatTestJS, NumericSeparators) {
   verifyFormat("x = 1_000_000 + 12;", "x = 1_000_000   + 12;");
 }
 
+TEST_F(FormatTestJS, AlignConsecutiveDeclarations) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("let    letVariable = 5;\n"
+               "double constVariable = 10;",
+               Style);
+
+  verifyFormat("let   letVariable = 5;\n"
+               "const constVariable = 10;",
+               Style);
+
+  verifyFormat("let          letVariable = 5;\n"
+               "static const constVariable = 10;",
+               Style);
+
+  verifyFormat("let        letVariable = 5;\n"
+               "static var constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable = 5;\n"
+               "var constVariable = 10;",
+               Style);
+
+  verifyFormat("double letVariable = 5;\n"
+               "var    constVariable = 10;",
+               Style);
+
+  verifyFormat("const letVariable = 5;\n"
+               "var   constVariable = 10;",
+               Style);
+
+  verifyFormat("int letVariable = 5;\n"
+               "int constVariable = 10;",
+               Style);
+}
+
+TEST_F(FormatTestJS, AlignConsecutiveAssignments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  verifyFormat("let letVariable      = 5;\n"
+               "double constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable     = 5;\n"
+               "const constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable            = 5;\n"
+               "static const constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable          = 5;\n"
+               "static var constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable   = 5;\n"
+               "var constVariable = 10;",
+               Style);
+
+  verifyFormat("double letVariable = 5;\n"
+               "var constVariable  = 10;",
+               Style);
+
+  verifyFormat("const letVariable = 5;\n"
+               "var constVariable = 10;",
+               Style);
+
+  verifyFormat("int letVariable   = 5;\n"
+               "int constVariable = 10;",
+               Style);
+}
+
+TEST_F(FormatTestJS, AlignConsecutiveAssignmentsAndDeclarations) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  verifyFormat("let    letVariable   = 5;\n"
+               "double constVariable = 10;",
+               Style);
+
+  verifyFormat("let   letVariable   = 5;\n"
+               "const constVariable = 10;",
+               Style);
+
+  verifyFormat("let          letVariable   = 5;\n"
+               "static const constVariable = 10;",
+               Style);
+
+  verifyFormat("let        letVariable   = 5;\n"
+               "static var constVariable = 10;",
+               Style);
+
+  verifyFormat("let letVariable   = 5;\n"
+               "var constVariable = 10;",
+               Style);
+
+  verifyFormat("double letVariable   = 5;\n"
+               "var    constVariable = 10;",
+               Style);
+
+  verifyFormat("const letVariable   = 5;\n"
+               "var   constVariable = 10;",
+               Style);
+
+  verifyFormat("int letVariable   = 5;\n"
+               "int constVariable = 10;",
+               Style);
+}
+
 } // namespace format
 } // end namespace clang