[clang-format] Do not format likely xml

[krasimir]

Summary

This patch detects the leading '<' in likely xml files and stops formatting in
that case. A recent use of a Qt xml file with a .ts extension triggered this:
http://doc.qt.io/qt-4.8/linguist-ts-file-format.html

[djasper]

Just a few minor comments, otherwise looks good.

lib/Format/Format.cpp

1542 ↗

(On Diff #112669)

Rename to isLikelyXml.

1552 ↗

(On Diff #112669)

Just add a separate if statement. Or merge with previous one, but I think three separate if statements is actually easiest to read.

1902 ↗

(On Diff #112669)

Same here.

[krasimir]

• Address review comments

cfe/trunk/lib/Format/Format.cpp

 
 bool isMpegTS(StringRef Code) {
   // MPEG transport streams use the ".ts" file extension. clang-format should
   // not attempt to format those. MPEG TS' frame format starts with 0x47 every
   // 189 bytes - detect that and return.
   return Code.size() > 188 && Code[0] == 0x47 && Code[188] == 0x47;
 }
 
+bool likelyXml(StringRef Code) {
+  return Code.ltrim().startswith("<");
+}
+
 tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
                                    ArrayRef<tooling::Range> Ranges,
                                    StringRef FileName, unsigned *Cursor) {
   tooling::Replacements Replaces;
   if (!Style.SortIncludes)
     return Replaces;
-  if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
-      isMpegTS(Code))
+  if (likelyXml(Code) ||
+      (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
+       isMpegTS(Code)))
     return Replaces;
   if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript)
     return sortJavaScriptImports(Style, Code, Ranges, FileName);
   sortCppIncludes(Style, Code, Ranges, FileName, Replaces, Cursor);
   return Replaces;
 }
 
 template <typename T>

 
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
                                ArrayRef<tooling::Range> Ranges,
                                StringRef FileName,
                                FormattingAttemptStatus *Status) {
   FormatStyle Expanded = expandPresets(Style);
   if (Expanded.DisableFormat)
     return tooling::Replacements();
-  if (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code))
+  if (likelyXml(Code) ||
+      (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code)))
     return tooling::Replacements();
 
   typedef std::function<tooling::Replacements(const Environment &)>
       AnalyzerPass;
   SmallVector<AnalyzerPass, 4> Passes;
 
   if (Style.Language == FormatStyle::LK_Cpp) {
     if (Style.FixNamespaceComments)

cfe/trunk/unittests/Format/FormatTest.cpp

 }
 
 TEST_F(FormatTest, UTF8CharacterLiteralCpp11) {
   // u8'a' is a C++17 feature, utf8 literal character, LS_Cpp11 covers
   // all modes, including C++11, C++14 and C++17
   EXPECT_EQ("auto c = u8'a';", format("auto c = u8'a';"));
 }
 
+TEST_F(FormatTest, DoNotFormatLikelyXml) {
+  EXPECT_EQ("<!-- ;> -->",
+            format("<!-- ;> -->", getGoogleStyle()));
+  EXPECT_EQ(" <!-- >; -->",
+            format(" <!-- >; -->", getGoogleStyle()));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang

cfe/trunk/unittests/Format/SortIncludesTest.cpp

TEST_F(SortIncludesTest, ValidAffactedRangesAfterDeduplicatingIncludes) {
   std::vector<tooling::Range> Ranges = {tooling::Range(0, 52)};
   auto Replaces = sortIncludes(Style, Code, Ranges, "input.cpp");
   Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
   EXPECT_EQ(1u, Ranges.size());
   EXPECT_EQ(0u, Ranges[0].getOffset());
   EXPECT_EQ(26u, Ranges[0].getLength());
 }
 
+TEST_F(SortIncludesTest, DoNotSortLikelyXml) {
+  EXPECT_EQ("<!--;\n"
+            "#include <b>\n"
+            "#include <a>\n"
+            "-->",
+            sort("<!--;\n"
+                 "#include <b>\n"
+                 "#include <a>\n"
+                 "-->"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang