clang-format: insert trailing commas into containers.

[mprobst]

Summary

This change adds an option to insert trailing commas into container
literals. For example, in JavaScript:

const x = [
  a,
  b,
   ^~~~~ inserted if missing.
]

This is implemented as a seperate post-processing pass after formatting
(because formatting might change whether the container literal does or
does not wrap). This keeps the code relatively simple and orthogonal,
though it has the notable drawback that the newly inserted comma is not
taken into account for formatting decisions (e.g. it might exceed the 80
char limit).

[mprobst]

This is required by JavaScript style, but this change as is optimistically runs the tool for any language. I think that's probably not what we want here initially, just throwing it out there. WDYT?

[mprobst]

• - only run comma insertion for JavaScript.

[krasimir]

General comment:

I think this is useful and could also be useful for other languages too (some folks have asked for this for some C++ code).
As this is a non-whitespace only change, it's a bit more risky than whitespace-only changes.

I think we need to establish a policy on such more riskier style options, at least have a convenient way for users to to opt-out of such and be extra careful including them by default in default styles.
This is tricky because some style guides explicitly recommend such changes.

[sammccall]

For rolling this out, I think the best path is:

• check in the option, but don't turn it on with any styles yet
• test it by taking a large codebase, formatting it (normal options), format it (with comma insertion), look at the diffs (internally google3diff can do this)
• turn it on for -style=google

This could all be in one commit, but that means coupling review of the code to getting results.

clang/include/clang/Format/Format.h

557

Hadn't occurred to me that this will interact with bin-packing. That seems to increase the chances of going over the column limit :(

clang/lib/Format/Format.cpp

1488

Inlining this in format.cpp seems worse than having passes in their own files, but is the pattern so far. Ugh, up to you.

1493

Worth a comment (somewhere) about the fact that this never rewraps, in particular if the line is long, for simplicity.

Another policy that occurred to me: don't insert the comma if you're exactly at the line limit.

1522

You're checking for the existence of a previous token, and below you use getPreviousNonComment() without a null-check.

I think you want to either assume that the existence of a match means there's a prev token (and remove the check here), or you want to verify in which case I think you also need to guard against it being a comment (by checking Prev below instead)

1537

this is a can't-happen case, right? (comma-insertions can't conflict with each other, and this pass gets its own Replacements).

use cantFail(Result.add(...))

clang/unittests/Format/FormatTestJS.cpp

343

AFAICT one-arg verifyFormat doesn't actually test this change - the new tests with commas should pass both before/after this change.
So there's just one testcase, I think.

1864

Pretty hard to spot the change here, add a comment or change the a/bs to a description?

I'd also place this next to a test that verifies that when you put the same construct on one line, no comma is inserted (even if the test is redundant), and include a case with a dict/map/hash/thing :-)

[Bouska]

This comment was removed by Bouska.

[Bouska]

I have an issue with this change. Currently (at least for C++), the presence of a trailing comma is used as a formatting hint to put all the element in one line or one per line as below:

enum test1 = {ONE, TWO, THREE};

enum test2 = {
    ONE,
    TWO,
    THREE,
};

As your change come after the formatting, clang-format won't be idempotent anymore. You are going to have the following if the container is bin packed (I did not actually test this):

First run:  
enum test1 = {
    ONE, TWO,
    THREE,};

Second run:
enum test1 = {
    ONE,
    TWO,
    THREE,
};

So I definitely think this change should be done before formatting and not after.

[sammccall]

In D73354#1838964, @Bouska wrote:

I have an issue with this change. Currently (at least for C++), the presence of a trailing comma is used as a formatting hint to put all the element in one line or one per line as below:

Good point about the interaction with bin-packing.
Do the style guides that want this actually endorse bin-packing containers? Insertion + bin-packing + comma-as-hint basically can't coexist - we could turn off the hint or make this option mutually exclusive with bin-packing.

...
So I definitely think this change should be done before formatting and not after.

I don't think this is a solution - we might wrap a container and fail to insert the comma :-)

[MyDeveloperDay]

I'm a little uncomfortable about all the tests changing, and I'm unsure if we should be changing the default behaviour.

The rules in the past here was to show that this is part of a public style guide. The assumption here is google style wants this. Could you please point to that documentation so at least there is some comeback when we break the world.

clang/lib/Format/Format.cpp

960

Are you sure the right decision is to make this on by default for something that's going to insert the comma? is this in google's javascript style guide?

I think this could cause clang-format to suddenly start adding lost of commas (as we see with the tests)

1488

Actually I think there is precedent to put TokenAnalyzers in their own class, I don't think it should be in Format.cpp

clang/unittests/Format/FormatTestJS.cpp

1290

is this correct?

1762

is this correct?

[krasimir]

An alternative approach (I'm not endorsing this) that would *in theory* circumvent non-idempotency issue (but would be more fiddly~fiddly to implement and spill across different layers of the formatter):
This is just a rough idea and can have multiple issues in itself. In general the trick is that if we can teach the token penalty computer and the format outputter about this, we should be able to use it together with other formatting decisions.

• add a new synthetic (annotated) token type, something like TT_PossiblyMissingTrailingComma, that would capture the length-0 text range where this could be inserted;
• use the algorithm here to splay it into the annotated token stream;
• mark it so that no newline can be inserted before such a token
• modify the penalty calculation algorithm to penalize for inserting a newline after this token: if we're inserting a newline after this token and if this token was inserted at a position at or past the maximal number of columns, add a PenaltyExcessCharacter for it.
• during outputting, if this token is followed by a newline, output it as ,; otherwise output it as an empty string or do not output it.

[MyDeveloperDay]

FYI maybe related D33029: [clang-format] add option for dangling parenthesis

[krasimir]

clang/lib/Format/Format.cpp

1493

+1 for "Another policy that occurred to me: don't insert the comma if you're exactly at the line limit." (or over the column limit). Seems like that would be enough to keep it idempotent.

[mprobst]

• - only run comma insertion for JavaScript.
• review fixes
• Fix col limit
• test for comma insertion

[mprobst]

clang/include/clang/Format/Format.h

557

Uh. Turns out we don't have a separate option for bin packing containers, only for bin packing arguments and parameters.

Should we add that option to make this setting work?
Should we have this setting imply bin packing containers (as opposed to parameters) is disabled?

clang/lib/Format/Format.cpp

960

It matches the style guide: https://google.github.io/styleguide/jsguide.html#features-arrays-trailing-comma

But it's indeed not entirely clear re bin packing, and certainly a good idea to split adding the option vs landing it.

1488

Re inlining vs separate file, idk. On one hand, this file is on the large side, otoh these ~60 LOC don't carry their own weight in a separate file. Maybe in a follow up we should extract all passes into a separate file (maybe all passes except the actual indenter?).

1493

Done re comment, and also done re policy, good idea.

I've also added a comment that this doesn't work together with bin packing.

1522

Good catch. I think the right fix is to check if there is a previous non comment token, as that's what I'm using below.

clang/unittests/Format/FormatTestJS.cpp

343

Ha, I just adjusted the test cases to my change, but forgot uploading the patch with the tests.

I've reverted all these changes (because I'm splitting flipping the default from this), but added an explicit test.

1290

Yes, type definitions should use ;

1762

Yes.

[MyDeveloperDay]

Nit: Could you add an entry for ReleaseNotes.rst and regenerate the ClangFormatStyleOption.rst with the docs/tools/dump_style.py

clang/unittests/Format/FormatTestJS.cpp

1290

if you hadn't added this to the test would it have added a "," ?

[mprobst]

PTAL.

clang/include/clang/Format/Format.h

557

OK, so I added a bit of validation logic to reject bin packing arguments together with TSC_Wrapped. We can figure out in a next step whether we want to expose an option for bin packing containers, or whether we can just disable bin packing for those people who want trailing commas.

clang/unittests/Format/FormatTestJS.cpp

1290

More precisely:
you can use either ; or , to separate entries in object literal types. So using ; is correct here, but c-f with this option would insert a ,, which is correct as well. I've added a test.

[mprobst]

• - only run comma insertion for JavaScript.
• review fixes
• Fix col limit
• test for comma insertion
• - validate options, reject bin packing + trailing commas

[MyDeveloperDay]

clang/lib/Format/Format.cpp

2531

Ok, this comment is more a discussion point rather than a review comment. Was there a reason you needed to put the TrailingCommaInserter pass after the Formatter pass? (maybe you needed the Tokens annotated?)

From my understanding of some of the other conversations, it seemed the reason for ignoring the Column limit was because the "," insertion came after it had been formatted (is that correct?)

If there was a good reason that's also fine, I was just interested to learn if there was some part of the Formatter that perhaps needs to be pulled out into its own separate pass.

[mprobst]

clang/lib/Format/Format.cpp

2531

The problem with inserting the comma during formatting is that it'd need to be inserted during the state exploration as part of Dijkstra's implemented in clang-format. I've considered that, but it'd be complex (if we make formatting decision X, we now add a token, which might invalidate the formatting decision). Keeping it as a separate pass has drawbacks, such as potentially not ending up with perfect formatting, thus the backing off to insert over ColumnLimit, but seems overall simpler.

[sammccall]

(Maybe we should have a basic test for C++, but I'm not sure how this usually goes)

BTW it occurs to me that turning this on by default may be a surprising breakage for:

• people who use clang-format to format JSON, where the comma is illegal.
• people who care about IE8 or something

clang/include/clang/Format/Format.h

42

Back->Pack?

clang/lib/Format/Format.cpp

2531

Ok, this comment is more a discussion point rather than a review comment. Was there a reason you needed to put the TrailingCommaInserter pass after the Formatter pass? (maybe you needed the Tokens annotated?)

The comma must only be inserted when the items are wrapped one-per-line (i.e. the last item is not on the same line as the closing bracket). This wrapping is a decision taken by the formatter, so we need to run it first to know.
(Apologies if this is obvious, but maybe missed)

[mprobst]

clang/include/clang/Format/Format.h

42

That's what you get when you fix my auto-complete: consistent misspelling.

[MyDeveloperDay]

I'm happy if you are, I think given that you've really been one of the main contributors of clang-format [JS] for the last number of years you are the best person to probably assess the impact on Javascript code.

Thanks for the patch, it LGTM and good luck!

[MyDeveloperDay]

Nit: don't forget the ClangFormatStyleOption.rst and ReleaseNote.rst changes

[mprobst]

This has landed as https://reviews.llvm.org/rGa324fcf1ae6 (not sure why this hasn't closed automatically).

[MyDeveloperDay]

In D73354#1854115, @mprobst wrote:

This has landed as https://reviews.llvm.org/rGa324fcf1ae6 (not sure why this hasn't closed automatically).

Thank you,

as for not autoclosing my guess it because perhaps you dropped the "Differential Revision" line from the commit message. I know there was some discussion about people not keeping all the Phabricator tags (Subscribers etc..), but I think if the commit message doesn't have the Differential Revision you won't get the AutoClose behaviour

[MyDeveloperDay]

clang/include/clang/Format/Format.h

563

I think we need to add some text in here to ensure it gets documented automatically in ClangFormatStyleOptions.rst rather than the by hand version done in D73768: clang-format: [JS] document InsertTrailingCommas. otherwise revisions like D78982: [clang-format] Fix Microsoft style for enums will end up removing it..

clang/include/clang/Format/Format.h

 namespace clang {
 
 class Lexer;
 class SourceManager;
 class DiagnosticConsumer;
 
 namespace format {
 
-enum class ParseError { Success = 0, Error, Unsuitable };
+enum class ParseError {
+  Success = 0,
+  Error,
+  Unsuitable,
+  BinBackTrailingCommaConflict
+};
 class ParseErrorCategory final : public std::error_category {
 public:
   const char *name() const noexcept override;
   std::string message(int EV) const override;
 };
 const std::error_category &getParseCategory();
 std::error_code make_error_code(ParseError e);
 
struct FormatStyle {
   ///   void f() {
   ///     f(aaaaaaaaaaaaaaaaaaaa,
   ///       aaaaaaaaaaaaaaaaaaaa,
   ///       aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);
   ///   }
   /// \endcode
   bool BinPackArguments;
 
+  /// The style of inserting trailing commas into container literals.
+  enum TrailingCommaStyle {
+    /// Do not insert trailing commas.
+    TCS_None,
+    /// Insert trailing commas in container literals that were wrapped over
+    /// multiple lines. Note that this is conceptually incompatible with
+    /// bin-packing, because the trailing comma is used as an indicator
+    /// that a container should be formatted one-per-line (i.e. not bin-packed).
+    /// So inserting a trailing comma counteracts bin-packing.
+    TCS_Wrapped,
+  };
+
+  TrailingCommaStyle InsertTrailingCommas;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line each.
   /// \code
   ///   true:
   ///   void f(int aaaaaaaaaaaaaaaaaaaa, int aaaaaaaaaaaaaaaaaaaa,
   ///          int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}
   ///
   ///   false:

clang/lib/Format/Format.cpp

 template <> struct ScalarEnumerationTraits<FormatStyle::BinPackStyle> {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
     IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
     IO.enumCase(Value, "Always", FormatStyle::BPS_Always);
     IO.enumCase(Value, "Never", FormatStyle::BPS_Never);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::TrailingCommaStyle> {
+  static void enumeration(IO &IO, FormatStyle::TrailingCommaStyle &Value) {
+    IO.enumCase(Value, "None", FormatStyle::TCS_None);
+    IO.enumCase(Value, "Wrapped", FormatStyle::TCS_Wrapped);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::BinaryOperatorStyle> {
   static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) {
     IO.enumCase(Value, "All", FormatStyle::BOS_All);
     IO.enumCase(Value, "true", FormatStyle::BOS_All);
     IO.enumCase(Value, "None", FormatStyle::BOS_None);
     IO.enumCase(Value, "false", FormatStyle::BOS_None);
     IO.enumCase(Value, "NonAssignment", FormatStyle::BOS_NonAssignment);
   }
IO.mapOptional("IncludeIsMainSourceRegex",
                    Style.IncludeStyle.IncludeIsMainSourceRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
     IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
     IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
+    IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
     IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
     IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);

 std::string ParseErrorCategory::message(int EV) const {
   switch (static_cast<ParseError>(EV)) {
   case ParseError::Success:
     return "Success";
   case ParseError::Error:
     return "Invalid argument";
   case ParseError::Unsuitable:
     return "Unsuitable";
+  case ParseError::BinBackTrailingCommaConflict:
+    return "trailing comma insertion cannot be used with bin packing";
   }
   llvm_unreachable("unexpected parse error");
 }
 
 static FormatStyle expandPresets(const FormatStyle &Style) {
   if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
     return Style;
   FormatStyle Expanded = Style;
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.IndentCaseBlocks = false;
   LLVMStyle.IndentGotoLabels = true;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
+  LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
if (Language == FormatStyle::LK_Java) {
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     // TODO: still under discussion whether to switch to SLS_All.
     GoogleStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
     GoogleStyle.BreakBeforeTernaryOperators = false;
     // taze:, triple slash directives (`/// <...`), tslint:, and @see, which is
     // commonly followed by overlong URLs.
     GoogleStyle.CommentPragmas = "(taze:|^/[ \t]*<|tslint:|@see)";
+    // TODO: enable once decided, in particular re disabling bin packing.
+    // https://google.github.io/styleguide/jsguide.html#features-arrays-trailing-comma
+    // GoogleStyle.InsertTrailingCommas = FormatStyle::TCS_Wrapped;
     GoogleStyle.MaxEmptyLinesToKeep = 3;
     GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
     GoogleStyle.SpacesInContainerLiterals = false;
     GoogleStyle.JavaScriptQuotes = FormatStyle::JSQS_Single;
     GoogleStyle.JavaScriptWrapImports = false;
   } else if (Language == FormatStyle::LK_Proto) {
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
std::error_code parseConfiguration(StringRef Text, FormatStyle *Style) {
   if (!LanguageFound) {
     if (Styles.empty() || Styles[0].Language != FormatStyle::LK_None)
       return make_error_code(ParseError::Unsuitable);
     FormatStyle DefaultStyle = Styles[0];
     DefaultStyle.Language = Language;
     StyleSet.Add(std::move(DefaultStyle));
   }
   *Style = *StyleSet.Get(Language);
+  if (Style->InsertTrailingCommas != FormatStyle::TCS_None &&
+      Style->BinPackArguments) {
+    // See comment on FormatStyle::TSC_Wrapped.
+    return make_error_code(ParseError::BinBackTrailingCommaConflict);
+  }
   return make_error_code(ParseError::Success);
 }
 
 std::string configurationAsText(const FormatStyle &Style) {
   std::string Text;
   llvm::raw_string_ostream Stream(Text);
   llvm::yaml::Output Output(Stream);
   // We use the same mapping method for input and output, so we need a non-const
deriveLocalStyle(const SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
     BinPackInconclusiveFunctions =
         HasBinPackedFunction || !HasOnePerLineFunction;
   }
 
   bool BinPackInconclusiveFunctions;
   FormattingAttemptStatus *Status;
 };
 
+/// TrailingCommaInserter inserts trailing commas into container literals.
+/// E.g.:
+///     const x = [
+///       1,
+///     ];
+/// TrailingCommaInserter runs after formatting. To avoid causing a required
+/// reformatting (and thus reflow), it never inserts a comma that'd exceed the
+/// ColumnLimit.
+///
+/// Because trailing commas disable binpacking of arrays, TrailingCommaInserter
+/// is conceptually incompatible with bin packing.
+class TrailingCommaInserter : public TokenAnalyzer {
+public:
+  TrailingCommaInserter(const Environment &Env, const FormatStyle &Style)
+      : TokenAnalyzer(Env, Style) {}
+
+  std::pair<tooling::Replacements, unsigned>
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens) override {
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+    tooling::Replacements Result;
+    insertTrailingCommas(AnnotatedLines, Result);
+    return {Result, 0};
+  }
+
+private:
+  /// Inserts trailing commas in [] and {} initializers if they wrap over
+  /// multiple lines.
+  void insertTrailingCommas(SmallVectorImpl<AnnotatedLine *> &Lines,
+                            tooling::Replacements &Result) {
+    for (AnnotatedLine *Line : Lines) {
+      insertTrailingCommas(Line->Children, Result);
+      if (!Line->Affected)
+        continue;
+      for (FormatToken *FormatTok = Line->First; FormatTok;
+           FormatTok = FormatTok->Next) {
+        if (FormatTok->NewlinesBefore == 0)
+          continue;
+        FormatToken *Matching = FormatTok->MatchingParen;
+        if (!Matching || !FormatTok->getPreviousNonComment())
+          continue;
+        if (!(FormatTok->is(tok::r_square) &&
+              Matching->is(TT_ArrayInitializerLSquare)) &&
+            !(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral)))
+          continue;
+        FormatToken *Prev = FormatTok->getPreviousNonComment();
+        if (Prev->is(tok::comma) || Prev->is(tok::semi))
+          continue;
+        // getEndLoc is not reliably set during re-lexing, use text length
+        // instead.
+        SourceLocation Start =
+            Prev->Tok.getLocation().getLocWithOffset(Prev->TokenText.size());
+        // If inserting a comma would push the code over the column limit, skip
+        // this location - it'd introduce an unstable formatting due to the
+        // required reflow.
+        unsigned ColumnNumber =
+            Env.getSourceManager().getSpellingColumnNumber(Start);
+        if (ColumnNumber > Style.ColumnLimit)
+          continue;
+        // Comma insertions cannot conflict with each other, and this pass has a
+        // clean set of Replacements, so the operation below cannot fail.
+        cantFail(Result.add(
+            tooling::Replacement(Env.getSourceManager(), Start, 0, ",")));
+      }
+    }
+  }
+};
+
 // This class clean up the erroneous/redundant code around the given ranges in
 // file.
 class Cleaner : public TokenAnalyzer {
 public:
   Cleaner(const Environment &Env, const FormatStyle &Style)
       : TokenAnalyzer(Env, Style),
         DeletedTokens(FormatTokenLess(Env.getSourceManager())) {}
 
if (Style.Language == FormatStyle::LK_JavaScript &&
     Passes.emplace_back([&](const Environment &Env) {
       return JavaScriptRequoter(Env, Expanded).process();
     });
 
   Passes.emplace_back([&](const Environment &Env) {
     return Formatter(Env, Expanded, Status).process();
   });
 
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+      Style.InsertTrailingCommas == FormatStyle::TCS_Wrapped)
+    Passes.emplace_back([&](const Environment &Env) {
+      return TrailingCommaInserter(Env, Expanded).process();
+    });
+
   auto Env =
       std::make_unique<Environment>(Code, FileName, Ranges, FirstStartColumn,
                                     NextStartColumn, LastStartColumn);
   llvm::Optional<std::string> CurrentCode = None;
   tooling::Replacements Fixes;
   unsigned Penalty = 0;
   for (size_t I = 0, E = Passes.size(); I < E; ++I) {
     std::pair<tooling::Replacements, unsigned> PassFixes = Passes[I](*Env);

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ParsesConfigurationWithLanguages) {
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE("Language: Cpp\n"
               "IndentWidth: 12",
               IndentWidth, 12u);
   EXPECT_EQ(parseConfiguration("Language: JavaScript\n"
                                "IndentWidth: 34",
                                &Style),
             ParseError::Unsuitable);
+  FormatStyle BinPackedTCS = {};
+  BinPackedTCS.Language = FormatStyle::LK_JavaScript;
+  EXPECT_EQ(parseConfiguration("BinPackArguments: true\n"
+                               "InsertTrailingCommas: Wrapped",
+                               &BinPackedTCS),
+            ParseError::BinBackTrailingCommaConflict);
   EXPECT_EQ(12u, Style.IndentWidth);
   CHECK_PARSE("IndentWidth: 56", IndentWidth, 56u);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language);
 
   Style.Language = FormatStyle::LK_JavaScript;
   CHECK_PARSE("Language: JavaScript\n"
               "IndentWidth: 12",
               IndentWidth, 12u);

clang/unittests/Format/FormatTestJS.cpp

TEST_F(FormatTestJS, ReservedWords) {
   verifyFormat("interface Test {\n"
                "  x: string;\n"
                "  switch: string;\n"
                "  case: string;\n"
                "  default: string;\n"
                "}\n");
   verifyFormat("const Axis = {\n"
                "  for: 'for',\n"
                "  x: 'x'\n"
                "};",
                "const Axis = {for: 'for', x:   'x'};");
 }
 
 TEST_F(FormatTestJS, ReservedWordsMethods) {
   verifyFormat(
       "class X {\n"
       "  delete() {\n"
verifyFormat("var array = [\n"
                "  a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,\n"
                "];");
   verifyFormat("var array = someFunction([\n"
                "  a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,\n"
                "  a, a, a, a, a, a, a, a, a, a, a, a, a, a, a,\n"
                "]);");
 }
 
+TEST_F(FormatTestJS, TrailingCommaInsertion) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.InsertTrailingCommas = FormatStyle::TCS_Wrapped;
+  // Insert comma in wrapped array.
+  verifyFormat("const x = [\n"
+               "  1,  //\n"
+               "  2,\n"
+               "];",
+               "const x = [\n"
+               "  1,  //\n"
+               "  2];",
+               Style);
+  // Insert comma in newly wrapped array.
+  Style.ColumnLimit = 30;
+  verifyFormat("const x = [\n"
+               "  aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
+               "];",
+               "const x = [aaaaaaaaaaaaaaaaaaaaaaaaa];", Style);
+  // Do not insert trailing commas if they'd exceed the colum limit
+  verifyFormat("const x = [\n"
+               "  aaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "];",
+               "const x = [aaaaaaaaaaaaaaaaaaaaaaaaaaaa];", Style);
+  // Object literals.
+  verifyFormat("const x = {\n"
+               "  a: aaaaaaaaaaaaaaaaa,\n"
+               "};",
+               "const x = {a: aaaaaaaaaaaaaaaaa};", Style);
+  verifyFormat("const x = {\n"
+               "  a: aaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "};",
+               "const x = {a: aaaaaaaaaaaaaaaaaaaaaaaaa};", Style);
+  // Object literal types.
+  verifyFormat("let x: {\n"
+               "  a: aaaaaaaaaaaaaaaaaaaaa,\n"
+               "};",
+               "let x: {a: aaaaaaaaaaaaaaaaaaaaa};", Style);
+}
+
 TEST_F(FormatTestJS, FunctionLiterals) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   verifyFormat("doFoo(function() {});");
   verifyFormat("doFoo(function() { return 1; });", Style);
   verifyFormat("var func = function() {\n"
                "  return 1;\n"
                "};");
TEST_F(FormatTestJS, WrapRespectsAutomaticSemicolonInsertion) {
   verifyFormat("await theReckoning;", getGoogleJSStyleWithColumns(10));
   verifyFormat("some['a']['b']", getGoogleJSStyleWithColumns(10));
   verifyFormat("x = (a['a']\n"
                "      ['b']);",
                getGoogleJSStyleWithColumns(10));
   verifyFormat("function f() {\n"
                "  return foo.bar(\n"
                "      (param): param is {\n"
                "        a: SomeType\n"
                "      }&ABC => 1)\n"
                "}",
                getGoogleJSStyleWithColumns(25));
 }
 
 TEST_F(FormatTestJS, AddsIsTheDictKeyOnNewline) {
   // Do not confuse is, the dict key with is, the type matcher. Put is, the dict
   // key, on a newline.
verifyFormat("class X {\n"
                "}\n");
 }
 
 TEST_F(FormatTestJS, TypeAliases) {
   verifyFormat("type X = number;\n"
                "class C {}");
   verifyFormat("type X<Y> = Z<Y>;");
   verifyFormat("type X = {\n"
                "  y: number\n"
                "};\n"
                "class C {}");
   verifyFormat("export type X = {\n"
                "  a: string,\n"
                "  b?: string,\n"
                "};\n");
 }
 
verifyFormat("export const y = {\n"
                "  a: 1,\n"
                "  b: 2\n"
                "};");
   verifyFormat("export enum Foo {\n"
                "  BAR,\n"
                "  // adsdasd\n"
                "  BAZ\n"
                "}");
   verifyFormat("export default [\n"
                "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
                "  bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n"
                "];");
   verifyFormat("export default [];");
   verifyFormat("export default () => {};");
   verifyFormat("export default () => {\n"
                "  x;\n"
                "  x;\n"