clang-format: [JS] detect C++ keywords.

[mprobst]

Summary

C++ defines a number of keywords that are regular identifiers in
JavaScript, e.g. concept:

const concept = 1; // legit JS

This change expands the existing IsJavaScriptIdentifier(Tok) function
to return false for C++ keywords that aren't keywords in JS.

[jfb]

Some of these are technically "future reserved keywords": https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Keywords

Wouldn't it be better to list all of JS's keywords at this point?

[krasimir]

In D77311#1957367, @jfb wrote:

Some of these are technically "future reserved keywords": https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Keywords

Wouldn't it be better to list all of JS's keywords at this point?

+1 to consider listing all of JS's keywords instead of listing all C++ keywords that are not JS keywords. With the current approach, any time a new C++ keyword token gets introduced, the switch in IsJavaScriptIdentifier would have to be updated.

But turning the implementation into something that lists JS keywords seems a bit tricky. Just a suggestion: maybe we can use the trick to #include TokenKinds.def used in TokenKinds.h:

bool IsJavaScriptIdentifier(const FormatToken &Tok) const {
switch (Tok.Tok.getKind()) {
  // list all JS keywords defined as KEYWORD in TokenKinds.def
  case tok::kw_break:
  case tok::kw_case:
  ... 
  case tok::kw_return:
  return false;

  // All of the remaining C keywords are JS identifiers.
  #define KEYWORD(X,Y) case tok::kw_ ## X:
  #include "clang/Basic/TokenKinds.def"
  // #undef KEYWORD is not needed -- it's #undef-ed at the end of TokenKinds.def
  return true;

  // handle identifiers and everything else as in the patch.
  case tok::identifier:
  default: return false;
}

[mprobst]

• - improve handling of keywors - rather than blacklisting all C++

[mprobst]

Thanks for the feedback. Indeed, whitelisting the JS keywords is better, but we still needed a way to blacklist all C++ keywords then - Krasimir's suggestion with the #include did the trick, thanks.

[mprobst]

• fix formatting

[MyDeveloperDay]

I think this LGTM and brings more clarity as its much easier to look for things in the positive than the negative

[krasimir]

Thank you!

[krasimir]

clang/lib/Format/FormatToken.h

913

nit: may be worth adding a comment linking to:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Keywords

[mprobst]

clang/lib/Format/FormatToken.h

913

Done, though I used the TS list of keywords (as it's a superset of the JS keywords).

clang/lib/Format/FormatToken.h

 //===--- FormatToken.h - Format C++ code ------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
struct AdditionalKeywords {
   IdentifierInfo *kw_unsafe;
   IdentifierInfo *kw_ushort;
   IdentifierInfo *kw_when;
   IdentifierInfo *kw_where;
 
   /// Returns \c true if \p Tok is a true JavaScript identifier, returns
   /// \c false if it is a keyword or a pseudo keyword.
   bool IsJavaScriptIdentifier(const FormatToken &Tok) const {
-    return Tok.is(tok::identifier) &&
-           JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) ==
-               JsExtraKeywords.end();
+    switch (Tok.Tok.getKind()) {
+    case tok::kw_break:
+    case tok::kw_case:
+    case tok::kw_catch:
+    case tok::kw_class:
+    case tok::kw_continue:
+    case tok::kw_const:
+    case tok::kw_default:
+    case tok::kw_delete:
+    case tok::kw_do:
+    case tok::kw_else:
+    case tok::kw_enum:
+    case tok::kw_export:
+    case tok::kw_false:
+    case tok::kw_for:
+    case tok::kw_if:
+    case tok::kw_import:
+    case tok::kw_module:
+    case tok::kw_new:
+    case tok::kw_private:
+    case tok::kw_protected:
+    case tok::kw_public:
+    case tok::kw_return:
+    case tok::kw_static:
+    case tok::kw_switch:
+    case tok::kw_this:
+    case tok::kw_throw:
+    case tok::kw_true:
+    case tok::kw_try:
+    case tok::kw_typeof:
+    case tok::kw_void:
+    case tok::kw_while:
+      // These are JS keywords that are lexed by LLVM/clang as keywords.
+      return false;
+    case tok::identifier:
+      // For identifiers, make sure they are true identifiers, excluding the
+      // JavaScript pseudo-keywords (not lexed by LLVM/clang as keywords).
+      return JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) ==
+    default:
+      // Other keywords are handled in the switch below, to avoid problems due
+      // to duplicate case labels when using the #include trick.
+      break;
+    }
+
+    switch (Tok.Tok.getKind()) {
+      // Handle C++ keywords not included above: these are all JS identifiers.
+#define KEYWORD(X, Y) case tok::kw_##X:
+#include "clang/Basic/TokenKinds.def"
+      // #undef KEYWORD is not needed -- it's #undef-ed at the end of
+      // TokenKinds.def
+      return true;
+    default:
+      // All other tokens (punctuation etc) are not JS identifiers.
+      return false;
+    }
   }
 
   /// Returns \c true if \p Tok is a C# keyword, returns
   /// \c false if it is a anything else.
   bool isCSharpKeyword(const FormatToken &Tok) const {
     switch (Tok.Tok.getKind()) {
     case tok::kw_bool:
     case tok::kw_break:

clang/lib/Format/TokenAnnotator.cpp

 //===--- TokenAnnotator.cpp - Format C++ code -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
if (Style.isCSharp() && CurrentToken->is(tok::question)) {
         Current.Type = TT_CSharpNullConditionalLSquare;
         return;
       }
     }
 
     if (Style.Language == FormatStyle::LK_JavaScript) {
       if (Current.is(tok::exclaim)) {
         if (Current.Previous &&
-            (Current.Previous->isOneOf(tok::identifier, tok::kw_namespace,
-                                       tok::r_paren, tok::r_square,
-                                       tok::r_brace) ||
+            (Keywords.IsJavaScriptIdentifier(*Current.Previous) ||
+             Current.Previous->isOneOf(tok::kw_namespace, tok::r_paren,
+                                       tok::r_square, tok::r_brace) ||
              Current.Previous->Tok.isLiteral())) {
           Current.Type = TT_JsNonNullAssertion;
           return;
         }
         if (Current.Next &&
             Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
           Current.Type = TT_JsNonNullAssertion;
           return;
if (Left.is(Keywords.kw_async) && Right.is(tok::l_paren) &&
       // as opposed to calling a function called async: `x = async();`
       if (Next && Next->is(TT_JsFatArrow))
         return true;
     }
     if ((Left.is(TT_TemplateString) && Left.TokenText.endswith("${")) ||
         (Right.is(TT_TemplateString) && Right.TokenText.startswith("}")))
       return false;
     // In tagged template literals ("html`bar baz`"), there is no space between
-    // the tag identifier and the template string. getIdentifierInfo makes sure
-    // that the identifier is not a pseudo keyword like `yield`, either.
-    if (Left.is(tok::identifier) && Keywords.IsJavaScriptIdentifier(Left) &&
-        Right.is(TT_TemplateString))
+    // the tag identifier and the template string.
+    if (Keywords.IsJavaScriptIdentifier(Left) && Right.is(TT_TemplateString))
       return false;
     if (Right.is(tok::star) &&
         Left.isOneOf(Keywords.kw_function, Keywords.kw_yield))
       return false;
     if (Right.isOneOf(tok::l_brace, tok::l_square) &&
         Left.isOneOf(Keywords.kw_function, Keywords.kw_yield,
                      Keywords.kw_extends, Keywords.kw_implements))
       return true;

clang/unittests/Format/FormatTestJS.cpp

 //===- unittest/Format/FormatTestJS.cpp - Formatting unit tests for JS ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
TEST_F(FormatTestJS, ReservedWordsParenthesized) {
   verifyFormat("throw (x + y);\n"
                "await (await x).y;\n"
                "typeof (x) === 'string';\n"
                "void (0);\n"
                "delete (x.y);\n"
                "return (x);\n");
 }
 
-TEST_F(FormatTestJS, CppKeywords) {
-  // Make sure we don't mess stuff up because of C++ keywords.
-  verifyFormat("return operator && (aa);");
-  // .. or QT ones.
-  verifyFormat("slots: Slot[];");
-}
-
 TEST_F(FormatTestJS, ES6DestructuringAssignment) {
   verifyFormat("var [a, b, c] = [1, 2, 3];");
   verifyFormat("const [a, b, c] = [1, 2, 3];");
   verifyFormat("let [a, b, c] = [1, 2, 3];");
   verifyFormat("var {a, b} = {a: 1, b: 2};");
   verifyFormat("let {a, b} = {a: 1, b: 2};");
 }
 
verifyFormat(
       "            .foo()!\n"
       "            .foo()!\n"
       "            .foo()!;\n",
       getGoogleJSStyleWithColumns(20));
   verifyFormat("let x = namespace!;\n");
   verifyFormat("return !!x;\n");
 }
 
+TEST_F(FormatTestJS, CppKeywords) {
+  // Make sure we don't mess stuff up because of C++ keywords.
+  verifyFormat("return operator && (aa);");
+  // .. or QT ones.
+  verifyFormat("const slots: Slot[];");
+  // use the "!" assertion operator to validate that clang-format understands
+  // these C++ keywords aren't keywords in JS/TS.
+  verifyFormat("auto!;");
+  verifyFormat("char!;");
+  verifyFormat("concept!;");
+  verifyFormat("double!;");
+  verifyFormat("extern!;");
+  verifyFormat("float!;");
+  verifyFormat("inline!;");
+  verifyFormat("int!;");
+  verifyFormat("long!;");
+  verifyFormat("register!;");
+  verifyFormat("restrict!;");
+  verifyFormat("sizeof!;");
+  verifyFormat("struct!;");
+  verifyFormat("typedef!;");
+  verifyFormat("union!;");
+  verifyFormat("unsigned!;");
+  verifyFormat("volatile!;");
+  verifyFormat("_Alignas!;");
+  verifyFormat("_Alignof!;");
+  verifyFormat("_Atomic!;");
+  verifyFormat("_Bool!;");
+  verifyFormat("_Complex!;");
+  verifyFormat("_Generic!;");
+  verifyFormat("_Imaginary!;");
+  verifyFormat("_Noreturn!;");
+  verifyFormat("_Static_assert!;");
+  verifyFormat("_Thread_local!;");
+  verifyFormat("__func__!;");
+  verifyFormat("__objc_yes!;");
+  verifyFormat("__objc_no!;");
+  verifyFormat("asm!;");
+  verifyFormat("bool!;");
+  verifyFormat("const_cast!;");
+  verifyFormat("dynamic_cast!;");
+  verifyFormat("explicit!;");
+  verifyFormat("friend!;");
+  verifyFormat("mutable!;");
+  verifyFormat("operator!;");
+  verifyFormat("reinterpret_cast!;");
+  verifyFormat("static_cast!;");
+  verifyFormat("template!;");
+  verifyFormat("typename!;");
+  verifyFormat("typeid!;");
+  verifyFormat("using!;");
+  verifyFormat("virtual!;");
+  verifyFormat("wchar_t!;");
+}
+
 TEST_F(FormatTestJS, NullPropagatingOperator) {
   verifyFormat("let x = foo?.bar?.baz();\n");
   verifyFormat("let x = foo?.(foo);\n");
   verifyFormat("let x = foo?.['arr'];\n");
 }
 
 TEST_F(FormatTestJS, NullishCoalescingOperator) {
   verifyFormat("const val = something ?? 'some other default';\n");