clang-format: [JS] handle conditionals in fields, default params.

[mprobst]

Summary

This causes a regression for top level ternary expressions, but that's deemed
acceptable, as those really should be very rare.

[djasper]

lib/Format/TokenAnnotator.cpp

1001 ↗

(On Diff #61716)

Well, if you keep the Line.MustBeDeclaration here, then the test case does not regress. I believe that the test in itself probably doesn't make much sense, but at least then we aren't regressing it. At any rate, it seems bad to keep the Line.MustBeDeclaration above and remove it here.

[mprobst]

ok

[djasper]

Looks good except that the patch description can now be updated.

cfe/trunk/lib/Format/TokenAnnotator.cpp

case tok::question:
         // types (fields, parameters), e.g.
         //   function(x?: string, y?) {...}
         //   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 &&
+      if (Line.MustBeDeclaration && !Contexts.back().IsExpression &&
           Style.Language == FormatStyle::LK_JavaScript)
         break;
       parseConditional();
       break;
     case tok::kw_template:
       parseTemplateDeclaration();
       break;
     case tok::comma:
if (Current.is(Keywords.kw_instanceof)) {
       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) {
+          Line.MustBeDeclaration && !Contexts.back().IsExpression) {
         // 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))) {

cfe/trunk/unittests/Format/FormatTestJS.cpp

TEST_F(FormatTestJS, OptionalTypes) {
   verifyFormat("function x(a?: b, c?, d?) {}");
   verifyFormat("class X {\n"
                "  y?: z;\n"
                "  z?;\n"
                "}");
   verifyFormat("interface X {\n"
                "  y?(): z;\n"
                "}");
-  verifyFormat("x ? 1 : 2;");
   verifyFormat("constructor({aa}: {\n"
                "  aa?: string,\n"
                "  aaaaaaaa?: string,\n"
                "  aaaaaaaaaaaaaaa?: boolean,\n"
                "  aaaaaa?: List<string>\n"
                "}) {}");
 }
 
TEST_F(FormatTestJS, NonNullAssertionOperator) {
   verifyFormat("let x = foo!.bar();\n");
   verifyFormat("let x = foo ? bar! : baz;\n");
   verifyFormat("let x = !foo;\n");
   verifyFormat("let x = foo[0]!;\n");
   verifyFormat("let x = (foo)!;\n");
   verifyFormat("let x = {foo: 1}!;\n");
 }
 
+TEST_F(FormatTestJS, Conditional) {
+  verifyFormat("y = x ? 1 : 2;");
+  verifyFormat("x ? 1 : 2;");
+  verifyFormat("class Foo {\n"
+               "  field = true ? 1 : 2;\n"
+               "  method(a = true ? 1 : 2) {}\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang