[clang-format] messes up indentation when using JavaScript private fields and methods

[MyDeveloperDay]

Summary

Addresses PR40999 https://bugs.llvm.org/show_bug.cgi?id=40999

Private fields and methods in javasceipt would get incorrectly indented (it sees them as preprocessor directives and hence left aligns them)

In this revision "#identifier" tokens tok::hash->tok::identifier are merged into a single new token
tok::identifier with the '#' contained inside the TokenText

class Example {
  pub = 1;
#priv = 2;

  static pub2 = "foo";
  static #priv2 = "bar";

  method() { this.#priv = 5; }

  static staticMethod() {
    switch (this.#priv) {
    case '1':
#priv = 3;
      break;
    }
  }

#privateMethod() {
  this.#privateMethod(); // infinite loop
}

static #staticPrivateMethod() {}
}

After this fix the code will be correctly indented

class Example {
  pub = 1;
  #priv = 2;

  static pub2 = "foo";
  static #priv2 = "bar";

  method() { this.#priv = 5; }

  static staticMethod() {
    switch (this.#priv) {
    case '1':
      #priv = 3;
      break;
    }
  }

  #privateMethod() {
    this.#privateMethod(); // infinite loop
  }

  static #staticPrivateMethod() {}
}

NOTE: There might be some Javascript code out there which uses the C processor to preprocess .js files http://www.nongnu.org/espresso/js-cpp.html, Its not clear how this revision or even private fields and methods would interact.

[MyDeveloperDay]

ping @mprobst,@mitchellwills noticed you just reviewed another clang-format-js commit, wondered if you were interested in reviewing this for me?, finding it hard to get any review traction.

[mprobst]

Actually I'll fix the nits.

clang/docs/ReleaseNotes.rst

174

Honestly, I think we don't really do release notes for clang-format. Revert this, or I'll drop it when committing your change.

clang/lib/Format/FormatToken.h

66

nit: please keep alpha sorted, i.e. before Js[T]

[mprobst]

Landed in r356449, thanks!

clang/docs/ReleaseNotes.rst

 ------------
 
 - ...
 
 clang-format
 ------------
 
 
-- ...
+- Add support for correct indenting of private fields and methods in Javascript.
 
 libclang
 --------
 
 - When `CINDEXTEST_INCLUDE_ATTRIBUTED_TYPES` is not provided when making a
   CXType, the equivalent type of the AttributedType is returned instead of the
   modified type if the user does not want attribute sugar. The equivalent type
   represents the minimally-desugared type which the AttributedType is

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(JsComputedPropertyName)                                                 \
   TYPE(JsExponentiation)                                                       \
   TYPE(JsExponentiationEqual)                                                  \
   TYPE(JsFatArrow)                                                             \
   TYPE(JsNonNullAssertion)                                                     \
   TYPE(JsTypeColon)                                                            \
   TYPE(JsTypeOperator)                                                         \
   TYPE(JsTypeOptionalQuestion)                                                 \
+  TYPE(JsPrivateIdentifier)                                                    \
   TYPE(LambdaArrow)                                                            \
   TYPE(LambdaLSquare)                                                          \
   TYPE(LeadingJavaAnnotation)                                                  \
   TYPE(LineComment)                                                            \
   TYPE(MacroBlockBegin)                                                        \
   TYPE(MacroBlockEnd)                                                          \
   TYPE(ObjCBlockLBrace)                                                        \
   TYPE(ObjCBlockLParen)                                                        \

clang/lib/Format/FormatTokenLexer.h

public:
 
   const AdditionalKeywords &getKeywords() { return Keywords; }
 
 private:
   void tryMergePreviousTokens();
 
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
+  bool tryMergeJSPrivateIdentifier();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 
   // Returns \c true if \p Tok can only be followed by an operand in JavaScript.
   bool precedesOperand(FormatToken *Tok);
 
   bool canPrecedeRegexLiteral(FormatToken *Prev);
 

clang/lib/Format/FormatTokenLexer.cpp

if (Style.Language == FormatStyle::LK_JavaScript) {
     if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
       return;
     if (tryMergeTokens(JSExponentiation, TT_JsExponentiation))
       return;
     if (tryMergeTokens(JSExponentiationEqual, TT_JsExponentiationEqual)) {
       Tokens.back()->Tok.setKind(tok::starequal);
       return;
     }
+    if (tryMergeJSPrivateIdentifier())
+      return;
   }
 
   if (Style.Language == FormatStyle::LK_Java) {
     static const tok::TokenKind JavaRightLogicalShiftAssign[] = {
         tok::greater, tok::greater, tok::greaterequal};
     if (tryMergeTokens(JavaRightLogicalShiftAssign, TT_BinaryOperator))
       return;
   }
bool FormatTokenLexer::tryMergeNSStringLiteral() {
   At->TokenText = StringRef(At->TokenText.begin(),
                             String->TokenText.end() - At->TokenText.begin());
   At->ColumnWidth += String->ColumnWidth;
   At->Type = TT_ObjCStringLiteral;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
+bool FormatTokenLexer::tryMergeJSPrivateIdentifier() {
+  // Merges #idenfier into a single identifier with the text #identifier
+  // but the token tok::identifier.
+  if (Tokens.size() < 2)
+    return false;
+  auto &Hash = *(Tokens.end() - 2);
+  auto &Identifier = *(Tokens.end() - 1);
+  if (!Hash->is(tok::hash) || !Identifier->is(tok::identifier))
+    return false;
+  Hash->Tok.setKind(tok::identifier);
+  Hash->TokenText =
+      StringRef(Hash->TokenText.begin(),
+                Identifier->TokenText.end() - Hash->TokenText.begin());
+  Hash->ColumnWidth += Identifier->ColumnWidth;
+  Hash->Type = TT_JsPrivateIdentifier;
+  Tokens.erase(Tokens.end() - 1);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)
     return false;
 
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
     FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);

clang/unittests/Format/FormatTestJS.cpp

TEST_F(FormatTestJS, ConditionalTypes) {
   // Formatting below is not necessarily intentional, this just ensures that
   // clang-format does not break the code.
   verifyFormat( // wrap
       "type UnionToIntersection<U> =\n"
       "    (U extends any ? (k: U) => void :\n"
       "                     never) extends((k: infer I) => void) ? I : never;");
 }
 
-} // end namespace tooling
+TEST_F(FormatTestJS, SupportPrivateFieldsAndMethods) {
+  verifyFormat("class Example {\n"
+               "  pub = 1;\n"
+               "  #priv = 2;\n"
+               "  static pub2 = 'foo';\n"
+               "  static #priv2 = 'bar';\n"
+               "  method() {\n"
+               "    this.#priv = 5;\n"
+               "  }\n"
+               "  static staticMethod() {\n"
+               "    switch (this.#priv) {\n"
+               "      case '1':\n"
+               "        #priv = 3;\n"
+               "        break;\n"
+               "    }\n"
+               "  }\n"
+               "  #privateMethod() {\n"
+               "    this.#privateMethod();  // infinite loop\n"
+               "  }\n"
+               "  static #staticPrivateMethod() {}\n");
+}
+
+} // namespace format
 } // end namespace clang