diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -544,6 +544,17 @@ /// \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. + 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 diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -157,6 +157,13 @@ } }; +template <> struct ScalarEnumerationTraits { + 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 { static void enumeration(IO &IO, FormatStyle::BinaryOperatorStyle &Value) { IO.enumCase(Value, "All", FormatStyle::BOS_All); @@ -486,6 +493,7 @@ 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); @@ -788,6 +796,7 @@ 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; @@ -944,6 +953,7 @@ // taze:, triple slash directives (`/// <...`), tslint:, and @see, which is // commonly followed by overlong URLs. GoogleStyle.CommentPragmas = "(taze:|^/[ \t]*<|tslint:|@see)"; + GoogleStyle.InsertTrailingCommas = FormatStyle::TCS_Wrapped; GoogleStyle.MaxEmptyLinesToKeep = 3; GoogleStyle.NamespaceIndentation = FormatStyle::NI_All; GoogleStyle.SpacesInContainerLiterals = false; @@ -1464,6 +1474,66 @@ FormattingAttemptStatus *Status; }; +/// TrailingCommaInserter inserts trailing commas into container literals. +/// E.g.: +/// const x = [ +/// 1, +/// ]; +class TrailingCommaInserter : public TokenAnalyzer { +public: + TrailingCommaInserter(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &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 &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->Previous) + 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()); + auto Err = Result.add( + tooling::Replacement(Env.getSourceManager(), Start, 0, ",")); + // FIXME: handle error. For now, print error message and skip the + // replacement for release version. + if (Err) { + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(false); + } + } + } + } +}; + // This class clean up the erroneous/redundant code around the given ranges in // file. class Cleaner : public TokenAnalyzer { @@ -2433,6 +2503,12 @@ 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(Code, FileName, Ranges, FirstStartColumn, NextStartColumn, LastStartColumn); diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp --- a/clang/unittests/Format/FormatTestJS.cpp +++ b/clang/unittests/Format/FormatTestJS.cpp @@ -340,7 +340,7 @@ "}\n"); verifyFormat("const Axis = {\n" " for: 'for',\n" - " x: 'x'\n" + " x: 'x',\n" "};", "const Axis = {for: 'for', x: 'x'};"); } @@ -405,18 +405,18 @@ verifyFormat("var x = {\n" " y: function(a) {\n" " return a;\n" - " }\n" + " },\n" "};"); verifyFormat("return {\n" " link: function() {\n" " f(); //\n" - " }\n" + " },\n" "};"); verifyFormat("return {\n" " a: a,\n" " link: function() {\n" " f(); //\n" - " }\n" + " },\n" "};"); verifyFormat("return {\n" " a: a,\n" @@ -425,7 +425,7 @@ " },\n" " link: function() {\n" " f(); //\n" - " }\n" + " },\n" "};"); verifyFormat("var stuff = {\n" " // comment for update\n" @@ -433,23 +433,27 @@ " // comment for modules\n" " modules: false,\n" " // comment for tasks\n" - " tasks: false\n" + " tasks: false,\n" "};"); verifyFormat("return {\n" " 'finish':\n" " //\n" - " a\n" + " a,\n" "};"); verifyFormat("var obj = {\n" " fooooooooo: function(x) {\n" " return x.zIsTooLongForOneLineWithTheDeclarationLine();\n" - " }\n" + " },\n" "};"); // Simple object literal, as opposed to enum style below. verifyFormat("var obj = {a: 123};"); // Enum style top level assignment. - verifyFormat("X = {\n a: 123\n};"); - verifyFormat("X.Y = {\n a: 123\n};"); + verifyFormat("X = {\n" + " a: 123,\n" + "};"); + verifyFormat("X.Y = {\n" + " a: 123,\n" + "};"); // But only on the top level, otherwise its a plain object literal assignment. verifyFormat("function x() {\n" " y = {z: 1};\n" @@ -460,7 +464,7 @@ verifyFormat("var x = {\n" " y: (a) => {\n" " return a;\n" - " }\n" + " },\n" "};"); verifyFormat("var x = {y: (a) => a};"); @@ -468,12 +472,12 @@ verifyFormat("var x = {\n" " y(a: string): number {\n" " return a;\n" - " }\n" + " },\n" "};"); verifyFormat("var x = {\n" " y(a: string) {\n" " return a;\n" - " }\n" + " },\n" "};"); // Computed keys. @@ -514,19 +518,19 @@ " value: 'test',\n" " get value() { // getter\n" " return this.value;\n" - " }\n" + " },\n" "};"); verifyFormat("var o = {\n" " value: 'test',\n" " set value(val) { // setter\n" " this.value = val;\n" - " }\n" + " },\n" "};"); verifyFormat("var o = {\n" " value: 'test',\n" " someMethod(val) { // method\n" " doSomething(this.value + val);\n" - " }\n" + " },\n" "};"); verifyFormat("var o = {\n" " someMethod(val) { // method\n" @@ -534,7 +538,7 @@ " },\n" " someOtherMethod(val) { // method\n" " doSomething(this.value + val);\n" - " }\n" + " },\n" "};"); } @@ -563,7 +567,7 @@ verifyFormat("var object_literal_with_long_name = {\n" " a: 'aaaaaaaaaaaaaaaaaa',\n" - " b: 'bbbbbbbbbbbbbbbbbb'\n" + " b: 'bbbbbbbbbbbbbbbbbb',\n" "};"); verifyFormat("f({a: 1, b: 2, c: 3});", @@ -730,7 +734,7 @@ verifyFormat("var x = {\n" " a: function*() {\n" " //\n" - " }\n" + " },\n" "}\n"); } @@ -824,12 +828,18 @@ } TEST_F(FormatTestJS, ArrayLiterals) { + // Several tests for bin-packing arrays below do not make sense with trailing + // comma insertion. + FormatStyle NoCommaInsertion = getGoogleStyle(FormatStyle::LK_JavaScript); + NoCommaInsertion.InsertTrailingCommas = FormatStyle::TCS_None; + verifyFormat("var aaaaa: List =\n" " [new SomeThingAAAAAAAAAAAA(), new SomeThingBBBBBBBBB()];"); verifyFormat("return [\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbb,\n" " ccccccccccccccccccccccccccc\n" - "];"); + "];", + NoCommaInsertion); verifyFormat("return [\n" " aaaa().bbbbbbbb('A'),\n" " aaaa().bbbbbbbb('B'),\n" @@ -838,7 +848,8 @@ verifyFormat("var someVariable = SomeFunction([\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbb,\n" " ccccccccccccccccccccccccccc\n" - "]);"); + "]);", + NoCommaInsertion); verifyFormat("var someVariable = SomeFunction([\n" " [aaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbb],\n" "]);", @@ -846,14 +857,16 @@ verifyFormat("var someVariable = SomeFunction(aaaa, [\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbb,\n" " ccccccccccccccccccccccccccc\n" - "]);"); + "]);", + NoCommaInsertion); verifyFormat("var someVariable = SomeFunction(\n" " aaaa,\n" " [\n" " aaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbb,\n" " cccccccccccccccccccccccccc\n" " ],\n" - " aaaa);"); + " aaaa);", + NoCommaInsertion); verifyFormat("var aaaa = aaaaa || // wrap\n" " [];"); @@ -892,8 +905,8 @@ " body: {\n" " setAttribute: function(key, val) { this[key] = val; },\n" " getAttribute: function(key) { return this[key]; },\n" - " style: {direction: ''}\n" - " }\n" + " style: {direction: ''},\n" + " },\n" "};", Style); verifyFormat("abc = xyz ? function() {\n" @@ -919,7 +932,7 @@ " return function() {\n" " f(); //\n" " };\n" - " }\n" + " },\n" "};"); verifyFormat("{\n" " var someVariable = function(x) {\n" @@ -937,7 +950,7 @@ " a: function SomeFunction() {\n" " // ...\n" " return 1;\n" - " }\n" + " },\n" "};"); verifyFormat("this.someObject.doSomething(aaaaaaaaaaaaaaaaaaaaaaaaaa)\n" " .then(goog.bind(function(aaaaaaaaaaa) {\n" @@ -975,7 +988,7 @@ verifyFormat("f({a: function() { return 1; }});", Style); Style.ColumnLimit = 32; verifyFormat("f({\n" - " a: function() { return 1; }\n" + " a: function() { return 1; },\n" "});", Style); @@ -1186,7 +1199,7 @@ verifyFormat("aaaaaaaaa++;", getGoogleJSStyleWithColumns(10)); verifyFormat("aaaaaaaaa--;", getGoogleJSStyleWithColumns(10)); verifyFormat("return [\n" - " aaa\n" + " aaa,\n" "];", getGoogleJSStyleWithColumns(12)); verifyFormat("class X {\n" @@ -1213,7 +1226,7 @@ verifyFormat("function f() {\n" " return foo.bar(\n" " (param): param is {\n" - " a: SomeType\n" + " a: SomeType;\n" " }&ABC => 1)\n" "}", getGoogleJSStyleWithColumns(25)); @@ -1224,7 +1237,7 @@ // key, on a newline. verifyFormat("Polymer({\n" " is: '', //\n" - " rest: 1\n" + " rest: 1,\n" "});", getGoogleJSStyleWithColumns(20)); } @@ -1294,7 +1307,7 @@ // Below "class Y {}" should ideally be on its own line. verifyFormat( "x = {\n" - " a: 1\n" + " a: 1,\n" "} class Y {}", " x = {a : 1}\n" " class Y { }"); @@ -1496,12 +1509,12 @@ verifyFormat("var x = {\n" " y: function(): z {\n" " return 1;\n" - " }\n" + " },\n" "};"); verifyFormat("var x = {\n" " y: function(): {a: number} {\n" " return 1;\n" - " }\n" + " },\n" "};"); verifyFormat("function someFunc(args: string[]):\n" " {longReturnValue: string[]} {}", @@ -1512,7 +1525,7 @@ verifyFormat("const xIsALongIdent:\n"" YJustBarelyFitsLinex[];", getGoogleJSStyleWithColumns(20)); verifyFormat("const x = {\n" - " y: 1\n" + " y: 1,\n" "} as const;"); } @@ -1638,24 +1651,24 @@ TEST_F(FormatTestJS, EnumDeclarations) { verifyFormat("enum Foo {\n" " A = 1,\n" - " B\n" + " B,\n" "}"); verifyFormat("export /* somecomment*/ enum Foo {\n" " A = 1,\n" - " B\n" + " B,\n" "}"); verifyFormat("enum Foo {\n" " A = 1, // comment\n" - " B\n" + " B,\n" "}\n" "var x = 1;"); verifyFormat("const enum Foo {\n" " A = 1,\n" - " B\n" + " B,\n" "}"); verifyFormat("export const enum Foo {\n" " A = 1,\n" - " B\n" + " B,\n" "}"); } @@ -1685,7 +1698,7 @@ "class C {}"); verifyFormat("type X = Z;"); verifyFormat("type X = {\n" - " y: number\n" + " y: number;\n" "};\n" "class C {}"); verifyFormat("export type X = {\n" @@ -1754,7 +1767,7 @@ verifyFormat("export {\n" " from as from,\n" " someSurprisinglyLongVariable as\n" - " from\n" + " from,\n" "};", getGoogleJSStyleWithColumns(20)); verifyFormat("export class C {\n" @@ -1778,14 +1791,18 @@ verifyFormat("export var x: number = 12;"); verifyFormat("export const y = {\n" " a: 1,\n" - " b: 2\n" + " b: 2,\n" "};"); verifyFormat("export enum Foo {\n" " BAR,\n" " // adsdasd\n" - " BAZ\n" + " BAZ,\n" "}"); verifyFormat("export default [\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,\n" + "];", + "export default [\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n" "];"); @@ -1998,10 +2015,11 @@ verifyFormat("var x = html`
    `;"); verifyFormat("yield `hello`;"); verifyFormat("var f = {\n" - " param: longTagName`This is a ${\n" - " 'really'} long line`\n" + " param:\n" + " longTagName`This is a ${\n" + " 'really'} long line!`,\n" "};", - "var f = {param: longTagName`This is a ${'really'} long line`};", + "var f = {param: longTagName`This is a ${'really'} long line!`};", getGoogleJSStyleWithColumns(40)); } @@ -2013,7 +2031,7 @@ getGoogleJSStyleWithColumns(20)); verifyFormat("foo = [\n" " 1, //\n" - " 2\n" + " 2,\n" "];"); verifyFormat("var x = [{x: 1} as type];"); verifyFormat("x = x as [a, b];"); @@ -2063,7 +2081,7 @@ " aa?: string,\n" " aaaaaaaa?: string,\n" " aaaaaaaaaaaaaaa?: boolean,\n" - " aaaaaa?: List\n" + " aaaaaa?: List,\n" "}) {}"); }