mprobst updated this revision to Diff 240194.
mprobst added a comment.
- - only run comma insertion for JavaScript.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73354/new/
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
@@ -157,6 +157,13 @@
}
};
+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);
@@ -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<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 +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<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
