[clang-format] Preserve include blocks in ObjC Google style

[krasimir]

Summary

r357567 started to regroup include block for Google style; it was meant to apply
only for C++. This patch reverts this for ObjC.

[ioeric]

lib/Format/Format.cpp

787

maybe we should also only use regroup for cpp? regroup is only supported in sortCppIncludes after all.

[krasimir]

lib/Format/Format.cpp

787

Yeah, but they are just not used for unrelated languages. This argument applies to the other fields in the IncludeStyle group above this. I'd lean on keeping this as-is.

[thakis]

Why would we want this to be different in Obj-C and C++?

[krasimir]

In D60263#1454812, @thakis wrote:

Why would we want this to be different in Obj-C and C++?

Only the C++ Style Guide had been updated.
Practically, clang-format would need to be updated to recognize ObjC-specific code patterns better before being able to apply regrouping to ObjC code in the wild.

I'd expect that sometime in the future the ObjC-style catches up.

[krasimir]

• Add a note about intent

[thakis]

Please don't land changes with open discussion.

Objective-C++ basically is C++ code. I don't think it makes sense to make a distinction here.

[stephanemoore]

Looks good to me.

cfe/trunk/lib/Format/Format.cpp

881 ↗

(On Diff #193700)

nit: "headers"

lib/Format/Format.cpp

FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
   GoogleStyle.DerivePointerAlignment = true;
   GoogleStyle.IncludeStyle.IncludeCategories = {
       {"^<ext/.*\\.h>", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
   GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   GoogleStyle.ObjCSpaceAfterProperty = false;
   GoogleStyle.ObjCSpaceBeforeProtocolList = true;
   GoogleStyle.PointerAlignment = FormatStyle::PAS_Left;
   GoogleStyle.RawStringFormats = {
       {
if (Language == FormatStyle::LK_Java) {
     // Text protos are currently mostly formatted inside C++ raw string literals
     // and often the current breaking behavior of string literals is not
     // beneficial there. Investigate turning this on once proper string reflow
     // has been implemented.
     GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
     GoogleStyle.ColumnLimit = 100;
+    // "Regroup" doesn't work well for ObjC yet (main header heuristic,
+    // relationship between ObjC standard library headers and other heades,
+    // #imports, etc.)
+    GoogleStyle.IncludeStyle.IncludeBlocks =
+        tooling::IncludeStyle::IBS_Preserve;
   }
 
   return GoogleStyle;
 }
 
 FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
   FormatStyle ChromiumStyle = getGoogleStyle(Language);
   if (Language == FormatStyle::LK_Java) {

unittests/Format/SortIncludesTest.cpp

TEST_F(SortIncludesTest, DoNotOutputReplacementsForSortedBlocksWithRegrouping) {
   std::string Code = R"(
 #include "b.h"
 
 #include <a.h>
 )";
   EXPECT_EQ(Code, sort(Code, "input.h", 0));
 }
 
+
+TEST_F(SortIncludesTest, DoNotRegroupGroupsInGoogleObjCStyle) {
+  FmtStyle = getGoogleStyle(FormatStyle::LK_ObjC);
+
+  EXPECT_EQ("#include <a.h>\n"
+            "#include <b.h>\n"
+            "#include \"a.h\"",
+            sort("#include <b.h>\n"
+                 "#include <a.h>\n"
+                 "#include \"a.h\""));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang