clang-format: [JS] Option for top-level dict literals on a single line

[ono]

Summary

Previously r223367 introduced stylistic change, where all top-level dict
literals will never be compacted to single line. Since this changes formatting,
some who want previous formatting now can bring it back setting
AllowTopLevelDictLiteralsOnASingleLine: true.

[djasper]

This needs tests.

Also, I am wondering whether we should straight away introduce this as "AllowDictLiteralsOnASingleLine" with an enum value of (None, All, TopLevel). What do you think?

[ono]

Yeah, but I think this should be rather None, All, SubLevel, since SubLevel will represent current behavior and All will represent previous behavior.

Preparing updated patch.

[djasper]

Right. SubLevel or Nested.

[ono]

• Introduced SingleLineStyle enum of None, Nested, All
• Replaced previous setting with AllowDictLiteralsOnASingleLine with value of enum above
• Added unit tests for this setting

[djasper]

Looks good. Do you have commit access?

[ono]

In D7200#156887, @djasper wrote:

Looks good. Do you have commit access?

Okie, need to rebase or merged okay with latest HEAD? And nope - no commit access.

[djasper]

Unfortunately, this interacts non-trivially with http://reviews.llvm.org/rL232320. I can fix the merge, but I am not sure what the proposed behavior is. Could you outline what you would want the flag to do in these cases? Or does the other commit possibly fix your use case?

unittests/Format/FormatTestJS.cpp

161–162

We probably should be testing 'nested' here, too.

[ono]

In D7200#157425, @djasper wrote:

(...) Or does the other commit possibly fix your use case?

I think the mentioned commit obsoletes this change. So I am marking this to be abandoned.

include/clang/Format/Format.h

struct FormatStyle {
 
   /// \brief If \c true, <tt>while (true) continue;</tt> can be put on a
   /// single line.
   bool AllowShortLoopsOnASingleLine;
 
   /// \brief If \c true, short case labels will be contracted to a single line.
   bool AllowShortCaseLabelsOnASingleLine;
 
+  /// \brief Different styles for merging structures into a single line.
+  enum SingleLineStyle {
+    /// \brief Never merge into a single line.
+    SLS_None,
+    /// \brief Only merge nested structures.
+    SLS_Nested,
+    /// \brief Try to merge all.
+    SLS_All,
+  };
+
+  /// \brief Dependent on the value, dict literals will be contracted to
+  /// a single line.
+  SingleLineStyle AllowDictLiteralsOnASingleLine;
+
   /// \brief Different styles for merging short functions containing at most one
   /// statement.
   enum ShortFunctionStyle {
     /// \brief Never merge functions into a single line.
     SFS_None,
     /// \brief Only merge functions defined inside a class.
     SFS_Inline,
     /// \brief Only merge empty functions.
struct FormatStyle {
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
            AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
            AlignOperands == R.AlignOperands &&
            AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&
            AlignTrailingComments == R.AlignTrailingComments &&
            AllowAllParametersOfDeclarationOnNextLine ==
                R.AllowAllParametersOfDeclarationOnNextLine &&
+           AllowDictLiteralsOnASingleLine == R.AllowDictLiteralsOnASingleLine &&
            AllowShortFunctionsOnASingleLine ==
                R.AllowShortFunctionsOnASingleLine &&
            AllowShortBlocksOnASingleLine == R.AllowShortBlocksOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==
                R.AllowShortIfStatementsOnASingleLine &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
            AlwaysBreakAfterDefinitionReturnType ==
                R.AlwaysBreakAfterDefinitionReturnType &&

lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::UseTabStyle &Value) {
     IO.enumCase(Value, "Never", FormatStyle::UT_Never);
     IO.enumCase(Value, "false", FormatStyle::UT_Never);
     IO.enumCase(Value, "Always", FormatStyle::UT_Always);
     IO.enumCase(Value, "true", FormatStyle::UT_Always);
     IO.enumCase(Value, "ForIndentation", FormatStyle::UT_ForIndentation);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::SingleLineStyle> {
+  static void enumeration(IO &IO, FormatStyle::SingleLineStyle &Value) {
+    IO.enumCase(Value, "None", FormatStyle::SLS_None);
+    IO.enumCase(Value, "false", FormatStyle::SLS_None);
+    IO.enumCase(Value, "All", FormatStyle::SLS_All);
+    IO.enumCase(Value, "true", FormatStyle::SLS_All);
+    IO.enumCase(Value, "Nested", FormatStyle::SLS_Nested);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
   static void enumeration(IO &IO, FormatStyle::ShortFunctionStyle &Value) {
     IO.enumCase(Value, "None", FormatStyle::SFS_None);
     IO.enumCase(Value, "false", FormatStyle::SFS_None);
     IO.enumCase(Value, "All", FormatStyle::SFS_All);
     IO.enumCase(Value, "true", FormatStyle::SFS_All);
     IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
     IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
static void mapping(IO &IO, FormatStyle &Style) {
 
     IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
     IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
     IO.mapOptional("AlignEscapedNewlinesLeft", Style.AlignEscapedNewlinesLeft);
     IO.mapOptional("AlignOperands", Style.AlignOperands);
     IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
     IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
                    Style.AllowAllParametersOfDeclarationOnNextLine);
+    IO.mapOptional("AllowDictLiteralsOnASingleLine",
+                   Style.AllowDictLiteralsOnASingleLine);
     IO.mapOptional("AllowShortBlocksOnASingleLine",
                    Style.AllowShortBlocksOnASingleLine);
     IO.mapOptional("AllowShortCaseLabelsOnASingleLine",
                    Style.AllowShortCaseLabelsOnASingleLine);
     IO.mapOptional("AllowShortIfStatementsOnASingleLine",
                    Style.AllowShortIfStatementsOnASingleLine);
     IO.mapOptional("AllowShortLoopsOnASingleLine",
                    Style.AllowShortLoopsOnASingleLine);
FormatStyle getLLVMStyle() {
   FormatStyle LLVMStyle;
   LLVMStyle.Language = FormatStyle::LK_Cpp;
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlinesLeft = false;
   LLVMStyle.AlignAfterOpenBracket = true;
   LLVMStyle.AlignOperands = true;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
+  LLVMStyle.AllowDictLiteralsOnASingleLine = FormatStyle::SLS_Nested;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortBlocksOnASingleLine = false;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;

lib/Format/TokenAnnotator.cpp

if (Right.is(tok::lessless) && Left.is(tok::identifier) &&
     return true;
 
   if (Style.Language == FormatStyle::LK_JavaScript) {
     // FIXME: This might apply to other languages and token kinds.
     if (Right.is(tok::char_constant) && Left.is(tok::plus) && Left.Previous &&
         Left.Previous->is(tok::char_constant))
       return true;
     if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace) &&
-        Left.NestingLevel == 0)
+        (Style.AllowDictLiteralsOnASingleLine == FormatStyle::SLS_None ||
+         (Style.AllowDictLiteralsOnASingleLine == FormatStyle::SLS_Nested &&
+          Left.NestingLevel == 0)))
       return true;
   } else if (Style.Language == FormatStyle::LK_Java) {
     if (Left.is(TT_LeadingJavaAnnotation) &&
         Right.isNot(TT_LeadingJavaAnnotation) && Right.isNot(tok::l_paren) &&
         Line.Last->is(tok::l_brace))
       return true;
     if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
         Right.Next->is(tok::string_literal))

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ParsesConfiguration) {
   Style.UseTab = FormatStyle::UT_ForIndentation;
   CHECK_PARSE("UseTab: Never", UseTab, FormatStyle::UT_Never);
   CHECK_PARSE("UseTab: ForIndentation", UseTab, FormatStyle::UT_ForIndentation);
   CHECK_PARSE("UseTab: Always", UseTab, FormatStyle::UT_Always);
   // For backward compatibility:
   CHECK_PARSE("UseTab: false", UseTab, FormatStyle::UT_Never);
   CHECK_PARSE("UseTab: true", UseTab, FormatStyle::UT_Always);
 
+  Style.AllowDictLiteralsOnASingleLine = FormatStyle::SLS_Nested;
+  CHECK_PARSE("AllowDictLiteralsOnASingleLine: None",
+              AllowDictLiteralsOnASingleLine, FormatStyle::SLS_None);
+  CHECK_PARSE("AllowDictLiteralsOnASingleLine: Nested",
+              AllowDictLiteralsOnASingleLine, FormatStyle::SLS_Nested);
+  CHECK_PARSE("AllowDictLiteralsOnASingleLine: All",
+              AllowDictLiteralsOnASingleLine, FormatStyle::SLS_All);
+  // For backward compatibility:
+  CHECK_PARSE("AllowDictLiteralsOnASingleLine: false",
+              AllowDictLiteralsOnASingleLine, FormatStyle::SLS_None);
+  CHECK_PARSE("AllowDictLiteralsOnASingleLine: true",
+              AllowDictLiteralsOnASingleLine, FormatStyle::SLS_All);
+
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: None",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: Inline",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Inline);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: Empty",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Empty);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",

unittests/Format/FormatTestJS.cpp

protected:
   }
 
   static FormatStyle getGoogleJSStyleWithColumns(unsigned ColumnLimit) {
     FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
     Style.ColumnLimit = ColumnLimit;
     return Style;
   }
 
+  static FormatStyle singleLineDictLiteralsStyle(
+      FormatStyle::SingleLineStyle SingleLineStyle) {
+    FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+    Style.AllowDictLiteralsOnASingleLine = SingleLineStyle;
+    return Style;
+  }
+
   static void verifyFormat(
       llvm::StringRef Code,
       const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_JavaScript)) {
     EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 };
 
 TEST_F(FormatTestJS, UnderstandsJavaScriptOperators) {

 }
 
 TEST_F(FormatTestJS, ES6DestructuringAssignment) {
   verifyFormat("var [a, b, c] = [1, 2, 3];");
   verifyFormat("var {a, b} = {\n"
                "  a: 1,\n"
                "  b: 2\n"
                "};");
+  verifyFormat("var {a, b} = {\n"
+               "  a: 1,\n"
+               "  b: 2\n"
+               "};",
+               singleLineDictLiteralsStyle(FormatStyle::SLS_None));
+  verifyFormat("var {a, b} = {a: 1, b: 2};",
+               singleLineDictLiteralsStyle(FormatStyle::SLS_All));
 }
 
 TEST_F(FormatTestJS, ContainerLiterals) {
   verifyFormat("return {\n"
                "  link: function() {\n"
                "    f();  //\n"
                "  }\n"
                "};");
verifyFormat("var obj = {\n"
                "    return x.zIsTooLongForOneLineWithTheDeclarationLine();\n"
                "  }\n"
                "};");
 }
 
 TEST_F(FormatTestJS, SpacesInContainerLiterals) {
   verifyFormat("var arr = [1, 2, 3];");
   verifyFormat("f({a: 1, b: 2, c: 3});");
+  verifyFormat("f({a: 1, b: 2, c: 3});",
+               singleLineDictLiteralsStyle(FormatStyle::SLS_All));
+  verifyFormat("f({\n"
+               "  a: 1,\n"
+               "  b: 2,\n"
+               "  c: 3\n"
+               "});",
+               singleLineDictLiteralsStyle(FormatStyle::SLS_None));
 
   verifyFormat("var object_literal_with_long_name = {\n"
                "  a: 'aaaaaaaaaaaaaaaaaa',\n"
                "  b: 'bbbbbbbbbbbbbbbbbb'\n"
                "};");
 
   verifyFormat("f({a: 1, b: 2, c: 3});",
                getChromiumStyle(FormatStyle::LK_JavaScript));