clang-format Test: Split BackwardCompatibility out of ParsesConfiguration

[jeanphilippeD]

Summary

Add some missing test to ParsesConfiguration and ParsesConfigurationBools.
Make explicit the backward compatibility test following the example of SpaceBeforeParens.

Split ConstructorInitializerIndentWidth into BreakConstructorInitializersBeforeComma.
This should ease a possible extension of the supported style for the Initializers.

Updated lines were ran through clang-format.
Built debug 'FormatTest' with VS2013 express and ran the test.

[djasper]

I like breaking out the BreakConstructorInitializerBeforeComma tests and of course the missing tests, but I don't think it is a good idea to split out the backward-compatibility tests. I'd rather have all the tests for a single style flag in a single place.

[jeanphilippeD]

Thank you for your feedback.

The following can only be added in ParsesConfigurationBools under DerivePointerAlignment if the CHECK_PARSE macro is moved above.

Style.DerivePointerAlignment = true;
CHECK_PARSE("DerivePointerBinding: false", DerivePointerAlignment, false);
CHECK_PARSE("DerivePointerBinding: true", DerivePointerAlignment, true);

In order to keep all the test for a single flag in the same place, would it be better define a second macro instead?CHECK_PARSE_BOOL_COMPATIBLE (or CHECK_PARSE_BOOL_LEGACY) defined and undefined with CHECK_PARSE_BOOL.

Would it also make sense to have a similar set of function for ParsesConfiguration?
CHECK_PARSE_ITEM and CHECK_PARSE to have:

Style.PointerAlignment = FormatStyle::PAS_Middle;
CHECK_PARSE_ITEM("Left", PointerAlignment, FormatStyle::PAS_Left);
CHECK_PARSE_ITEM("Right", PointerAlignment, FormatStyle::PAS_Right);
CHECK_PARSE_ITEM("Middle", PointerAlignment, FormatStyle::PAS_Middle);
// For backward compatibility:
CHECK_PARSE("PointerBindsToType: Left", PointerAlignment,
            FormatStyle::PAS_Left);
CHECK_PARSE("PointerBindsToType: Right", PointerAlignment,
            FormatStyle::PAS_Right);
CHECK_PARSE("PointerBindsToType: Middle", PointerAlignment,
            FormatStyle::PAS_Middle);

Since CHECK_PARSE is also used in ParsesConfigurationWithLanguages, I would avoid rename it to CHECK_PARSE_ITEM_COMPATIBLE.

I'll work on addressing the issue this evening.

[jeanphilippeD]

Updated patch as per review comments: Keept backward compatible tests with their flag tests.
Rebased patch on top of snv:r217804 and merge.
Compile and run FormatTest.exe.

[djasper]

Looks good. Do you have commit access or do you want me to submit this for you?

[jeanphilippeD]

This is my first patch, I do not have commit access, I would prefer if you
submit it for me.
Thank you very much.

2014-09-16 13:52 GMT+01:00 Daniel Jasper <[e-mail]>:

Looks good. Do you have commit access or do you want me to submit this for
you?

http://reviews.llvm.org/D5346

[djasper]

Submitted as r217880. Thank you!

unittests/Format/FormatTest.cpp

 
 #define CHECK_PARSE_BOOL(FIELD)                                                \
   Style.FIELD = false;                                                         \
   EXPECT_EQ(0, parseConfiguration(#FIELD ": true", &Style).value());           \
   EXPECT_TRUE(Style.FIELD);                                                    \
   EXPECT_EQ(0, parseConfiguration(#FIELD ": false", &Style).value());          \
   EXPECT_FALSE(Style.FIELD);
 
+#define CHECK_PARSE(TEXT, FIELD, VALUE)                                        \
+  EXPECT_NE(VALUE, Style.FIELD);                                               \
+  EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value());                      \
+  EXPECT_EQ(VALUE, Style.FIELD)
+
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
   CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(AlwaysBreakAfterDefinitionReturnType);
   CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
+  // For backward compatibility:
+  CHECK_PARSE("DerivePointerBinding: true", DerivePointerAlignment, true);
+  CHECK_PARSE("DerivePointerBinding: false", DerivePointerAlignment, false);
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
 }
 
 #undef CHECK_PARSE_BOOL
 
-#define CHECK_PARSE(TEXT, FIELD, VALUE)                                        \
-  EXPECT_NE(VALUE, Style.FIELD);                                               \
-  EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value());                      \
-  EXPECT_EQ(VALUE, Style.FIELD)
-
 TEST_F(FormatTest, ParsesConfiguration) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset, -1234);
   CHECK_PARSE("ConstructorInitializerIndentWidth: 1234",
               ConstructorInitializerIndentWidth, 1234u);
   CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
   CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
TEST_F(FormatTest, ParsesConfiguration) {
 
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
               FormatStyle::PAS_Left);
   CHECK_PARSE("PointerAlignment: Right", PointerAlignment,
               FormatStyle::PAS_Right);
   CHECK_PARSE("PointerAlignment: Middle", PointerAlignment,
               FormatStyle::PAS_Middle);
+  // For backward compatibility:
+  CHECK_PARSE("PointerBindsToType: Left", PointerAlignment,
+              FormatStyle::PAS_Left);
+  CHECK_PARSE("PointerBindsToType: Right", PointerAlignment,
+              FormatStyle::PAS_Right);
+  CHECK_PARSE("PointerBindsToType: Middle", PointerAlignment,
+              FormatStyle::PAS_Middle);
 
   Style.Standard = FormatStyle::LS_Auto;
   CHECK_PARSE("Standard: Cpp03", Standard, FormatStyle::LS_Cpp03);
   CHECK_PARSE("Standard: Cpp11", Standard, FormatStyle::LS_Cpp11);
   CHECK_PARSE("Standard: C++03", Standard, FormatStyle::LS_Cpp03);
   CHECK_PARSE("Standard: C++11", Standard, FormatStyle::LS_Cpp11);
   CHECK_PARSE("Standard: Auto", Standard, FormatStyle::LS_Auto);
 
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
-  CHECK_PARSE("BreakBeforeBinaryOperators: false", BreakBeforeBinaryOperators,
-              FormatStyle::BOS_None);
   CHECK_PARSE("BreakBeforeBinaryOperators: NonAssignment",
               BreakBeforeBinaryOperators, FormatStyle::BOS_NonAssignment);
-  CHECK_PARSE("BreakBeforeBinaryOperators: true", BreakBeforeBinaryOperators,
-              FormatStyle::BOS_All);
   CHECK_PARSE("BreakBeforeBinaryOperators: None", BreakBeforeBinaryOperators,
               FormatStyle::BOS_None);
   CHECK_PARSE("BreakBeforeBinaryOperators: All", BreakBeforeBinaryOperators,
               FormatStyle::BOS_All);
+  // For backward compatibility:
+  CHECK_PARSE("BreakBeforeBinaryOperators: false", BreakBeforeBinaryOperators,
+              FormatStyle::BOS_None);
+  CHECK_PARSE("BreakBeforeBinaryOperators: true", BreakBeforeBinaryOperators,
+              FormatStyle::BOS_All);
 
   Style.UseTab = FormatStyle::UT_ForIndentation;
-  CHECK_PARSE("UseTab: false", UseTab, FormatStyle::UT_Never);
-  CHECK_PARSE("UseTab: true", UseTab, FormatStyle::UT_Always);
   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.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
-  CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
-              AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
-  CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
-              AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: None",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: Inline",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Inline);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
+              AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
+  CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
+              AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
               FormatStyle::SBPO_Never);
   CHECK_PARSE("SpaceBeforeParens: Always", SpaceBeforeParens,
               FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
               FormatStyle::SBPO_ControlStatements);
verifyFormat(
       Style);
 
   Style.ConstructorInitializerIndentWidth = 0;
   verifyFormat(
       "SomeClass::Constructor()\n"
       ": aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
       "  aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}",
       Style);
+}
 
+TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {
+  FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializersBeforeComma = true;
+
   Style.ConstructorInitializerIndentWidth = 4;
   verifyFormat("SomeClass::Constructor()\n"
                "    : a(a)\n"
                "    , b(b)\n"
                "    , c(c) {}",
                Style);
   verifyFormat("SomeClass::Constructor()\n"
                "    : a(a) {}",