llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

[clang-format] Skip block commented out includes when sorting them

Open owenca opened this issue 1 year ago • 1 comments

Fixes #97539.

owenca avatar Jul 05 '24 04:07 owenca

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #97539.


Full diff: https://github.com/llvm/llvm-project/pull/97787.diff

2 Files Affected:

  • (modified) clang/lib/Format/Format.cpp (+11-4)
  • (modified) clang/unittests/Format/SortIncludesTest.cpp (+9)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 6a8883b77a730..7fd42e46e0ccb 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3222,10 +3222,16 @@ tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
     if (Trimmed.contains(RawStringTermination))
       FormattingOff = false;
 
-    if (isClangFormatOff(Trimmed))
+    bool IsBlockComment = false;
+
+    if (isClangFormatOff(Trimmed)) {
       FormattingOff = true;
-    else if (isClangFormatOn(Trimmed))
+    } else if (isClangFormatOn(Trimmed)) {
       FormattingOff = false;
+    } else if (Trimmed.starts_with("/*")) {
+      IsBlockComment = true;
+      Pos = Code.find("*/", SearchFrom + 2);
+    }
 
     const bool EmptyLineSkipped =
         Trimmed.empty() &&
@@ -3235,9 +3241,10 @@ tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
 
     bool MergeWithNextLine = Trimmed.ends_with("\\");
     if (!FormattingOff && !MergeWithNextLine) {
-      if (tooling::HeaderIncludes::IncludeRegex.match(Line, &Matches)) {
+      if (!IsBlockComment &&
+          tooling::HeaderIncludes::IncludeRegex.match(Trimmed, &Matches)) {
         StringRef IncludeName = Matches[2];
-        if (Line.contains("/*") && !Line.contains("*/")) {
+        if (Trimmed.contains("/*") && !Trimmed.contains("*/")) {
           // #include with a start of a block comment, but without the end.
           // Need to keep all the lines until the end of the comment together.
           // FIXME: This is somehow simplified check that probably does not work
diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp
index 2eeb16b4ab9f5..3175382564637 100644
--- a/clang/unittests/Format/SortIncludesTest.cpp
+++ b/clang/unittests/Format/SortIncludesTest.cpp
@@ -1455,6 +1455,15 @@ TEST_F(SortIncludesTest, DisableRawStringLiteralSorting) {
 #undef X
 }
 
+TEST_F(SortIncludesTest, BlockCommentedOutIncludes) {
+  StringRef Code{"/* #include \"foo.h\"\n"
+                 "#include \"bar.h\" */\n"
+                 "#include <chrono>"};
+
+  FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
+  verifyFormat(Code, sort(Code, "input.cpp", 0));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang

llvmbot avatar Jul 05 '24 04:07 llvmbot