mprobst created this revision.
mprobst added reviewers: krasimir, MyDeveloperDay.
Herald added a project: clang.
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).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73354
Files:
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTestJS.cpp
Index: clang/unittests/Format/FormatTestJS.cpp
===================================================================
--- clang/unittests/Format/FormatTestJS.cpp
+++ 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<SomeThing> =\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<Y> = Z<Y>;");
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`<ul>`;");
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 = <Bar[]>[\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<string>\n"
+ " aaaaaa?: List<string>,\n"
"}) {}");
}
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -788,6 +788,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 +945,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 +1466,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<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->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 +2495,11 @@
return Formatter(Env, Expanded, Status).process();
});
+ if (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);
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
