clang-format: [JS] support optional methods.

[mprobst]

Summary

Optional methods use ? tokens like this:

interface X { y?(): z; }

It seems easiest to detect and disambiguate these
from ternary expressions by checking if the code
is in a declaration context. Turns out that that
didn't quite work properly for interfaces in Java
and JS, and for JS file root contexts.

[mprobst]

lib/Format/TokenAnnotator.cpp

1870

Note that this change is unrelated, I just stumbled across this code and found it hard to read with all those boolean clauses. Might be useful to go over this and add comments explaining the individual parts at some point.

[mprobst]

For some reason with these changes interfaces are formatted onto one line:

interface X { y: string; }

Instead of

interface X {
  y: string;
}

No idea why, I'll investigate a bit.

[mprobst]

Turns out that wasn't a regression, just a missing feature. See D9494.

[djasper]

Submitted as r236488.

lib/Format/TokenAnnotator.cpp

1870

I have merged the first two, but after starting to write comments, I realized that I would just duplicate what's inside the "if (..)". I think this is pretty readable as is.

lib/Format/TokenAnnotator.cpp

case tok::minus:
       if (!Tok->Previous && Line.MustBeDeclaration)
         Tok->Type = TT_ObjCMethodSpecifier;
       break;
     case tok::colon:
       if (!Tok->Previous)
         return false;
       // Colons from ?: are handled in parseConditional().
       if (Style.Language == FormatStyle::LK_JavaScript) {
-        if (Contexts.back().ColonIsForRangeExpr ||
-            (Contexts.size() == 1 &&
+        if (Contexts.back().ColonIsForRangeExpr || // colon in for loop
+            (Contexts.size() == 1 &&               // switch/case labels
              !Line.First->isOneOf(tok::kw_enum, tok::kw_case)) ||
-            Contexts.back().ContextKind == tok::l_paren ||
-            Contexts.back().ContextKind == tok::l_square) {
+            Contexts.back().ContextKind == tok::l_paren ||  // function params
+            Contexts.back().ContextKind == tok::l_square || // array type
+            Line.MustBeDeclaration) { // method/property declaration
           Tok->Type = TT_JsTypeColon;
           break;
         }
       }
       if (Contexts.back().ColonIsDictLiteral) {
         Tok->Type = TT_DictLiteral;
       } else if (Contexts.back().ColonIsObjCMethodExpr ||
                  Line.First->is(TT_ObjCMethodSpecifier)) {
case tok::question:
           Tok->Next->isOneOf(tok::colon, tok::semi, tok::r_paren,
                              tok::r_brace)) {
         // Question marks before semicolons, colons, commas, etc. indicate
         // optional types (fields, parameters), e.g.
         // `function(x?: string, y?) {...}` or `class X {y?;}`
         Tok->Type = TT_JsTypeOptionalQuestion;
         break;
       }
+      // Declarations cannot be conditional expressions, this can only be part
+      // of a type declaration.
+      if (Line.MustBeDeclaration &&
+          Style.Language == FormatStyle::LK_JavaScript)
+        break;
       parseConditional();
       break;
     case tok::kw_template:
       parseTemplateDeclaration();
       break;
     case tok::comma:
       if (Contexts.back().InCtorInitializer)
         Tok->Type = TT_CtorInitializerComma;
if (Current.is(Keywords.kw_instanceof)) {
       Current.Type = determinePlusMinusCaretUsage(Current);
       if (Current.is(TT_UnaryOperator) && Current.is(tok::caret))
         Contexts.back().CaretFound = true;
     } else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
       Current.Type = determineIncrementUsage(Current);
     } else if (Current.isOneOf(tok::exclaim, tok::tilde)) {
       Current.Type = TT_UnaryOperator;
     } else if (Current.is(tok::question)) {
+      if (Style.Language == FormatStyle::LK_JavaScript &&
+          Line.MustBeDeclaration) {
+        // In JavaScript, `interface X { foo?(): bar; }` is an optional method
+        // on the interface, not a ternary expression.
+        Current.Type = TT_JsTypeOptionalQuestion;
+      } else {
         Current.Type = TT_ConditionalExpr;
+      }
     } else if (Current.isBinaryOperator() &&
                (!Current.Previous || Current.Previous->isNot(tok::l_square))) {
       Current.Type = TT_BinaryOperator;
     } else if (Current.is(tok::comment)) {
       Current.Type =
           Current.TokenText.startswith("/*") ? TT_BlockComment : TT_LineComment;
     } else if (Current.is(tok::r_paren)) {
       if (rParenEndsCast(Current))
bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
   if (Right.is(tok::comma))
     return false;
   if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
     return true;
   if (Left.is(tok::kw_operator))
     return Right.is(tok::coloncolon);
   if (Right.is(TT_OverloadedOperatorLParen))
     return false;
-  if (Right.is(tok::colon))
-    return !Line.First->isOneOf(tok::kw_case, tok::kw_default) &&
-           Right.getNextNonComment() && Right.isNot(TT_ObjCMethodExpr) &&
-           !Left.is(tok::question) &&
-           !(Right.is(TT_InlineASMColon) && Left.is(tok::coloncolon)) &&
-           (Right.isNot(TT_DictLiteral) || Style.SpacesInContainerLiterals);
+  if (Right.is(tok::colon)) {
+    if (Line.First->isOneOf(tok::kw_case, tok::kw_default))
+      return false;
+    if (!Right.getNextNonComment())
+      return false;
+    if (Right.is(TT_ObjCMethodExpr))
+      return false;
+    if (Left.is(tok::question))
+      return false;
+    if (Right.is(TT_InlineASMColon) && Left.is(tok::coloncolon))
+      return false;
+    if (Right.is(TT_DictLiteral))
+      return Style.SpacesInContainerLiterals;
+    return true;
+  }
   if (Left.is(TT_UnaryOperator))
     return Right.is(TT_BinaryOperator);
   if (Left.is(TT_CastRParen))
     return Style.SpaceAfterCStyleCast || Right.is(TT_BinaryOperator);
   if (Left.is(tok::greater) && Right.is(tok::greater)) {
     return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
            (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
   }

lib/Format/UnwrappedLineParser.cpp

if (!PPLevelBranchIndex.empty()) {
       assert(PPLevelBranchIndex.back() <= PPLevelBranchCount.back());
     }
   } while (!PPLevelBranchIndex.empty());
 
   return StructuralError;
 }
 
 void UnwrappedLineParser::parseFile() {
-  ScopedDeclarationState DeclarationState(
-      *Line, DeclarationScopeStack,
-      /*MustBeDeclaration=*/!Line->InPPDirective);
+  // The top-level context in a file always has declarations, except for pre-
+  // processor directives and JavaScript files.
+  bool MustBeDeclaration =
+      !(Line->InPPDirective || Style.Language == FormatStyle::LK_JavaScript);
+  ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
+                                          MustBeDeclaration);
   parseLevel(/*HasOpeningBrace=*/false);
   // Make sure to format the remaining tokens.
   flushComments(true);
   addUnwrappedLine();
 }
 
 void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
   bool SwitchLabelEncountered = false;
case tok::identifier: {
       StringRef Text = FormatTok->TokenText;
       // Parse function literal unless 'function' is the first token in a line
       // in which case this should be treated as a free-standing function.
       if (Style.Language == FormatStyle::LK_JavaScript && Text == "function" &&
           Line->Tokens.size() > 0) {
         tryToParseJSFunction();
         break;
       }
+      if ((Style.Language == FormatStyle::LK_JavaScript ||
+           Style.Language == FormatStyle::LK_Java) &&
+          FormatTok->is(Keywords.kw_interface)) {
+        parseRecord();
+        break;
+      }
+
       nextToken();
       if (Line->Tokens.size() == 1 &&
           // JS doesn't have macros, and within classes colons indicate fields,
           // not labels.
           Style.Language != FormatStyle::LK_JavaScript) {
         if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
           parseLabel();
           return;
if (ShouldBreakBeforeBrace(Style, InitialToken))
       addUnwrappedLine();
 
     parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
                /*MunchSemi=*/false);
   }
   // We fall through to parsing a structural element afterwards, so
   // class A {} n, m;
   // will end up in one unwrapped line.
-  // This does not apply for Java.
+  // This does not apply for Java and JavaScript.
   if (Style.Language == FormatStyle::LK_Java ||
       Style.Language == FormatStyle::LK_JavaScript)
     addUnwrappedLine();
 }
 
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do

unittests/Format/FormatTestJS.cpp

 }
 
 TEST_F(FormatTestJS, OptionalTypes) {
   verifyFormat("function x(a?: b, c?, d?) {\n}");
   verifyFormat("class X {\n"
                "  y?: z;\n"
                "  z?;\n"
                "}");
+  verifyFormat("interface X { y?(): z; }");
+  verifyFormat("x ? 1 : 2;");
 }
 
 TEST_F(FormatTestJS, IndexSignature) {
   verifyFormat("var x: {[k: string]: v};");
 }
 
 } // end namespace tooling
 } // end namespace clang