clang-format: [JS] do not insert whitespace in call positions.

[mprobst]

Summary

In JavaScript, may keywords can be used in method names and thus call sites:

foo.delete();
foo.instanceof();

clang-format would previously insert whitespace after the instanceof. This
change generically skips inserting whitespace between a keyword and a
parenthesis if preceded by a dot, i.e. in a callsite.

[mprobst]

• support switch, case, delete.

[djasper]

lib/Format/TokenAnnotator.cpp

2355 ↗

(On Diff #109089)

Why is instanceof not required in this list?

unittests/Format/FormatTestJS.cpp

237 ↗

(On Diff #109089)

no test for delete?

[mprobst]

• clean up implementation, just use getIdentifierInfo
• clean up tests, remove illegal syntax, add more tests

[mprobst]

lib/Format/TokenAnnotator.cpp

2355 ↗

(On Diff #109089)

instanceof is not a keyword in the C++ sense, so it is covered by the getIdentifierInfo part. But this was actually a misunderstanding - getIdentifierInfo actually covers all keyword-ish tokens, so we don't need the list of tokens here at all.

[djasper]

Looks good.

cfe/trunk/lib/Format/TokenAnnotator.cpp

if (Left.is(tok::identifier) && Keywords.IsJavaScriptIdentifier(Left) &&
       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;
+    if (Right.is(tok::l_paren)) {
       // JS methods can use some keywords as names (e.g. `delete()`).
-    if (Right.is(tok::l_paren) && Line.MustBeDeclaration &&
+      if (Line.MustBeDeclaration && Left.Tok.getIdentifierInfo())
+        return false;
+      // Valid JS method names can include keywords, e.g. `foo.delete()` or
+      // `bar.instanceof()`. Recognize call positions by preceding period.
+      if (Left.Previous && Left.Previous->is(tok::period) &&
           Left.Tok.getIdentifierInfo())
         return false;
-    if (Right.is(tok::l_paren) &&
-        Left.isOneOf(tok::kw_throw, Keywords.kw_await, Keywords.kw_typeof, tok::kw_void))
+      // Additional unary JavaScript operators that need a space after.
+      if (Left.isOneOf(tok::kw_throw, Keywords.kw_await, Keywords.kw_typeof,
+                       tok::kw_void))
         return true;
+    }
     if ((Left.isOneOf(Keywords.kw_let, Keywords.kw_var, Keywords.kw_in,
                       tok::kw_const) ||
          // "of" is only a keyword if it appears after another identifier
          // (e.g. as "const x of y" in a for loop).
          (Left.is(Keywords.kw_of) && Left.Previous &&
           Left.Previous->Tok.getIdentifierInfo())) &&
         (!Left.Previous || !Left.Previous->is(tok::period)))
       return true;

cfe/trunk/unittests/Format/FormatTestJS.cpp

 
 TEST_F(FormatTestJS, ReservedWords) {
   // JavaScript reserved words (aka keywords) are only illegal when used as
   // Identifiers, but are legal as IdentifierNames.
   verifyFormat("x.class.struct = 1;");
   verifyFormat("x.case = 1;");
   verifyFormat("x.interface = 1;");
   verifyFormat("x.for = 1;");
-  verifyFormat("x.of() = 1;");
+  verifyFormat("x.of();");
   verifyFormat("of(null);");
   verifyFormat("import {of} from 'x';");
-  verifyFormat("x.in() = 1;");
-  verifyFormat("x.let() = 1;");
-  verifyFormat("x.var() = 1;");
-  verifyFormat("x.for() = 1;");
-  verifyFormat("x.as() = 1;");
+  verifyFormat("x.in();");
+  verifyFormat("x.let();");
+  verifyFormat("x.var();");
+  verifyFormat("x.for();");
+  verifyFormat("x.as();");
+  verifyFormat("x.instanceof();");
+  verifyFormat("x.switch();");
+  verifyFormat("x.case();");
+  verifyFormat("x.delete();");
+  verifyFormat("x.throw();");
+  verifyFormat("x.throws();");
+  verifyFormat("x.if();");
   verifyFormat("x = {\n"
                "  a: 12,\n"
                "  interface: 1,\n"
                "  switch: 1,\n"
                "};");
   verifyFormat("var struct = 2;");
   verifyFormat("var union = 2;");
   verifyFormat("var interface = 2;");
verifyFormat("try {\n"
                "  g();\n"
                "} finally {\n"
                "  h();\n"
                "}");
 
   // But, of course, "catch" is a perfectly fine function name in JavaScript.
   verifyFormat("someObject.catch();");
   verifyFormat("someObject.new();");
-  verifyFormat("someObject.delete();");
 }
 
 TEST_F(FormatTestJS, StringLiteralConcatenation) {
   verifyFormat("var literal = 'hello ' +\n"
                "    'world';");
 }
 
 TEST_F(FormatTestJS, RegexLiteralClassification) {