Implement trailing comma for comma-separated lists

Author: mateusrodriguesxyz

include/swift/Basic/Features.def

@@ -401,6 +401,9 @@ EXPERIMENTAL_FEATURE(DebugDescriptionMacro, true)
 
 EXPERIMENTAL_FEATURE(ReinitializeConsumeInMultiBlockDefer, false)
 
+// Enable trailing comma for comma-separated lists.
+EXPERIMENTAL_FEATURE(TrailingComma, true)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

include/swift/Parse/Parser.h

@@ -1966,6 +1966,8 @@ class Parser {
   /// expression can be parsed.
   bool isStartOfStmt(bool preferExpr);
 
+  bool isStartOfConditionalStmtBody();
+
   bool isTerminatorForBraceItemListKind(BraceItemListKind Kind,
                                         ArrayRef<ASTNode> ParsedDecls);
   ParserResult<Stmt> parseStmt(bool fromASTGen = false);

lib/AST/FeatureSet.cpp

@@ -771,6 +771,7 @@ static bool usesFeatureSensitive(Decl *decl) {
 
 UNINTERESTING_FEATURE(DebugDescriptionMacro)
 UNINTERESTING_FEATURE(ReinitializeConsumeInMultiBlockDefer)
+UNINTERESTING_FEATURE(TrailingComma)
 
 // ----------------------------------------------------------------------------
 // MARK: - FeatureSet

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -62,6 +62,7 @@ extension Parser.ExperimentalFeatures {
     mapFeature(.NonescapableTypes, to: .nonescapableTypes)
     mapFeature(.TransferringArgsAndResults, to: .transferringArgsAndResults)
     mapFeature(.SendingArgsAndResults, to: .sendingArgsAndResults)
+    mapFeature(.TrailingComma, to: .trailingComma)
   }
 }
 

lib/Parse/ParseDecl.cpp

@@ -2549,7 +2549,7 @@ Parser::parseMacroRoleAttribute(
   SmallVector<MacroIntroducedDeclName, 2> names;
   SmallVector<TypeExpr *, 2> conformances;
   auto argumentsStatus = parseList(tok::r_paren, lParenLoc, rParenLoc,
-                                   /*AllowSepAfterLast=*/false,
+                                   /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
                                    diag::expected_rparen_expr_list,
                                    [&] {
     ParserStatus status;
@@ -9351,6 +9351,10 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags,
     if (!Tok.is(tok::comma))
       break;
     CommaLoc = consumeToken(tok::comma);
+    if (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+        (Tok.isAtStartOfLine() || Tok.is(tok::r_brace) || Tok.is(tok::semi))) {
+      break;
+    }
   }
 
   if (!(Flags & PD_AllowEnumElement)) {

lib/Parse/ParseExpr.cpp

@@ -1914,6 +1914,10 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
   default:
   UnknownCharacter:
     checkForInputIncomplete();
+    if (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+        Tok.is(tok::eof) && Tok.getText() == ")") {
+      return nullptr;
+    }
     // FIXME: offer a fixit: 'Self' -> 'self'
     diagnose(Tok, ID);
     return nullptr;
@@ -2758,7 +2762,8 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
         VD->setIsSelfParamCapture();
 
       captureList.push_back(CLE);
-    } while (HasNext);
+    } while (HasNext && !(Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+                          Tok.is(tok::r_square)));
 
     // The capture list needs to be closed off with a ']'.
     SourceLoc rBracketLoc = Tok.getLoc();
@@ -3278,7 +3283,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
   StructureMarkerRAII ParsingExprList(*this, Tok);
 
   leftLoc = consumeToken(leftTok);
-  return parseList(rightTok, leftLoc, rightLoc, /*AllowSepAfterLast=*/false,
+  return parseList(rightTok, leftLoc, rightLoc, /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
                    rightTok == tok::r_paren ? diag::expected_rparen_expr_list
                                             : diag::expected_rsquare_expr_list,
                    [&] () -> ParserStatus {

lib/Parse/ParseGeneric.cpp

@@ -47,6 +47,7 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
                         SmallVectorImpl<GenericTypeParamDecl *> &GenericParams) {
   ParserStatus Result;
   bool HasNextParam{};
+  bool IsEndOfList;
   do {
     // Note that we're parsing a declaration.
     StructureMarkerRAII ParsingDecl(*this, Tok.getLoc(),
@@ -132,7 +133,9 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
 
     // Parse the comma, if the list continues.
     HasNextParam = consumeIf(tok::comma);
-  } while (HasNextParam);
+    IsEndOfList = (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+                   startsWithGreater(Tok));
+  } while (HasNextParam && !IsEndOfList);
 
   return Result;
 }
@@ -276,6 +279,7 @@ ParserStatus Parser::parseGenericWhereClause(
   // Parse the 'where'.
   WhereLoc = consumeToken(tok::kw_where);
   bool HasNextReq;
+  bool IsEndOfList;
   do {
     if (Tok.is(tok::code_complete)) {
       if (CodeCompletionCallbacks) {
@@ -401,6 +405,8 @@ ParserStatus Parser::parseGenericWhereClause(
       break;
     }
     HasNextReq = consumeIf(tok::comma);
+    IsEndOfList = (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+                   Tok.is(tok::l_brace));
     // If there's a comma, keep parsing the list.
     // If there's a "&&", diagnose replace with a comma and keep parsing
     if (Tok.isBinaryOperator() && Tok.getText() == "&&" && !HasNextReq) {
@@ -409,7 +415,7 @@ ParserStatus Parser::parseGenericWhereClause(
       consumeToken();
       HasNextReq = true;
     }
-  } while (HasNextReq);
+  } while (HasNextReq && !IsEndOfList);
 
   if (!Requirements.empty())
     EndLoc = Requirements.back().getSourceRange().End;

lib/Parse/ParsePattern.cpp

@@ -192,7 +192,7 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
   // Parse the parameter list.
   bool isClosure = paramContext == ParameterContextKind::Closure;
   return parseList(tok::r_paren, leftParenLoc, rightParenLoc,
-                      /*AllowSepAfterLast=*/false,
+                      /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
                       diag::expected_rparen_parameter,
                       [&]() -> ParserStatus {
     ParsedParameter param;
@@ -1279,7 +1279,7 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
   SmallVector<TuplePatternElt, 8> elts;
   ParserStatus ListStatus =
     parseList(tok::r_paren, LPLoc, RPLoc,
-              /*AllowSepAfterLast=*/false,
+              /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
               diag::expected_rparen_tuple_pattern_list,
               [&] () -> ParserStatus {
     // Parse the pattern tuple element.

lib/Parse/ParseStmt.cpp

@@ -1519,6 +1519,11 @@ Parser::parseAvailabilitySpecList(SmallVectorImpl<AvailabilitySpec *> &Specs,
       consumeToken();
       Status.setIsParseError();
     } else if (consumeIf(tok::comma)) {
+      // End of list with a trailing comma.
+      if (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+          Tok.is(tok::r_paren)) {
+        break;
+      }
       // There is more to parse in this list.
 
       // Before continuing to parse the next specification, we check that it's
@@ -1833,6 +1838,74 @@ Parser::parseStmtConditionElement(SmallVectorImpl<StmtConditionElement> &result,
   return Status;
 }
 
+/// Returns `true` if the current token represents the start of a conditional
+/// statement body.
+bool Parser::isStartOfConditionalStmtBody() {
+  if (!Tok.is(tok::l_brace)) {
+    // Statement bodies always start with a '{'. If there is no '{', we can't be
+    // at the statement body.
+    return false;
+  }
+
+  Parser::BacktrackingScope Backtrack(*this);
+
+  skipSingle();
+
+  if (Tok.is(tok::eof)) {
+    // There's nothing else in the source file that could be the statement body,
+    // so this must be it.
+    return true;
+  }
+
+  if (Tok.is(tok::semi)) {
+    // We can't have a semicolon between the condition and the statement body,
+    // so this must be the statement body.
+    return true;
+  }
+
+  if (Tok.is(tok::kw_else)) {
+    // If the current token is an `else` keyword, this must be the statement
+    // body of an `if` statement since conditions can't be followed by `else`.
+    return true;
+  }
+
+  if (Tok.is(tok::r_brace) || Tok.is(tok::r_paren)) {
+    // A right brace or parenthesis cannot start a statement body, nor can the
+    // condition list continue afterwards. So, this must be the statement body.
+    // This covers cases like `if true, { if true, { } }` or `( if true, {
+    // print(0) } )`. While the latter is not valid code, it improves
+    // diagnostics.
+    return true;
+  }
+
+  if (Tok.isAtStartOfLine()) {
+    // If the current token is at the start of a line, it is most likely a
+    // statement body. The only exceptions are:
+    if (Tok.is(tok::comma)) {
+      // If newline begins with ',' it must be a condition trailing comma, so
+      // this can't be the statement body, e.g. if true, { true } , true {
+      // print("body") }
+      return false;
+    }
+    if (Tok.is(tok::oper_binary_spaced)) {
+      // If current token is a binary operator this can't be the statement body
+      // since an `if` expression can't be the left-hand side of an operator,
+      // e.g. if true, { true }
+      // != nil
+      // {
+      //   print("body")
+      // }
+      return false;
+    }
+    // Excluded the above exceptions, this must be the statement body.
+    return true;
+  } else {
+    // If the current token isn't at the start of a line and isn't `EOF`, `;`,
+    // `else`, `)` or `}` this can't be the statement body.
+    return false;
+  }
+}
+
 /// Parse the condition of an 'if' or 'while'.
 ///
 ///   condition:
@@ -1861,6 +1934,20 @@ ParserStatus Parser::parseStmtCondition(StmtCondition &Condition,
   // a variety of common errors situations (including migrating from Swift 2
   // syntax).
   while (true) {
+
+    if (Context.LangOpts.hasFeature(Feature::TrailingComma)) {
+      // Condition terminator is `else` for `guard` statements.
+      if (ParentKind == StmtKind::Guard && Tok.is(tok::kw_else)) {
+        break;
+      }
+      // Condition terminator is start of statement body for `if` or `while`
+      // statements. Missing `else` is a common mistake for `guard` statements
+      // so we fall back to lookahead for a body.
+      if (isStartOfConditionalStmtBody()) {
+        break;
+      }
+    }
+
     Status |= parseStmtConditionElement(result, DefaultID, ParentKind,
                                         BindingKindStr);
     if (Status.isErrorOrHasCompletion())
@@ -2634,6 +2721,11 @@ static ParserStatus parseStmtCase(Parser &P, SourceLoc &CaseLoc,
       if (!P.consumeIf(tok::comma))
         break;
 
+      if (P.Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+          P.Tok.is(tok::colon)) {
+        break;
+      }
+
       if (P.Tok.is(tok::kw_case)) {
         P.diagnose(P.Tok, diag::extra_case_keyword)
           .fixItRemove(SourceRange(P.Tok.getLoc()));

test/Parse/trailing-comma-conditions.swift

@@ -0,0 +1,51 @@
+// RUN: %target-typecheck-verify-swift -disable-experimental-parser-round-trip -enable-experimental-feature TrailingComma
+
+func f(_ block: (Bool) -> Bool) -> Bool { block(true) }
+
+func testConditionListTrailingComma() {
+    if true, { }
+
+    if true, { }; { }()
+
+    if true, { print("if-body") } else { print("else-body") }
+
+    if true, { print("if-body") } else if true, { print("else-if-body") } else { print("else-body") }
+
+    if true, { if true { { } } } // expected-error {{closure expression is unused}} expected-note {{did you mean to use a 'do' statement?}}
+
+    { if true, { print(0) } } // expected-error {{closure expression is unused}} expected-note {{did you mean to use a 'do' statement?}}
+
+    ( if true, { print(0) } ) // expected-error {{'if' may only be used as expression in return, throw, or as the source of an assignment}} expected-error {{'if' must have an unconditional 'else' to be used as expression}}
+
+    if true, {true}(), { }
+
+    if true, { true }, { print(0) } // expected-error {{function produces expected type 'Bool'; did you mean to call it with '()'?}}
+
+    if true, { print(0) }
+    { }()
+
+    if true, { true } // expected-error {{function produces expected type 'Bool'; did you mean to call it with '()'?}}
+    ,{ print(0) }
+
+    if true, { (x: () -> Void) in true } != nil { print(0) } // expected-warning {{comparing non-optional value of type '(() -> Void) -> Bool' to 'nil' always returns true}}
+
+    if true, { (x: () -> Void) in true }
+    != nil // expected-warning {{comparing non-optional value of type '(() -> Void) -> Bool' to 'nil' always returns true}}
+    {
+    print(0)
+    }
+
+    if { } // expected-error {{missing condition in 'if' statement}} 
+
+    if , { } // expected-error {{expected expression, var, or let in 'if' condition}}
+
+    guard else { return } // expected-error {{missing condition in 'guard' statement}} 
+
+    guard , else { return } // expected-error {{expected expression, var, let or case in 'guard' condition}} 
+
+    guard true, else { return }
+
+    guard true, , else { return } // expected-error {{expected expression in conditional}}
+
+    guard true, { return } // expected-error {{expected 'else' after 'guard' condition}}
+}