[clang-format] support header deletion in cleanupAroundReplacemnts.

[ioeric]

Summary

• If a replacement has offset UINT_MAX, length 0, and a replacement text that is an #include directive, this will insert the #include into the correct block in the \p Code.
• If a replacement has offset UINT_MAX, length 1, and a replacement text that is the name of the header to be removed, the header will be removed from \p Code if it exists.

[djasper]

Looks good.

lib/Format/Format.cpp

1557 ↗

(On Diff #72175)

return HeaderToDelete.find(HeaderName) != HeadersToDelete.end() ||
       HeaderToDelete.find(HeaderName.trim("\"<>")) != HeadersToDelete.end()

1623 ↗

(On Diff #72175)

quotes.

1634 ↗

(On Diff #72175)

Maybe:

unsigned Length = std::min(Line.size(), Code.size() - Offset - 1);

[ioeric]

• Addressed comments.

cfe/trunk/include/clang/Format/Format.h

 /// llvm::StringError.
 llvm::Expected<tooling::Replacements>
 formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
                    const FormatStyle &Style);
 
 /// \brief Returns the replacements corresponding to applying \p Replaces and
 /// cleaning up the code after that on success; otherwise, return an llvm::Error
 /// carrying llvm::StringError.
-/// This also inserts a C++ #include directive into the correct block if the
-/// replacement corresponding to the header insertion has offset UINT_MAX.
+/// This also supports inserting/deleting C++ #include directives:
+/// - If a replacement has offset UINT_MAX, length 0, and a replacement text
+///   that is an #include directive, this will insert the #include into the
+///   correct block in the \p Code.
+/// - If a replacement has offset UINT_MAX, length 1, and a replacement text
+///   that is the name of the header to be removed, the header will be removed
+///   from \p Code if it exists.
 llvm::Expected<tooling::Replacements>
 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
                           const FormatStyle &Style);
 
 /// \brief Reformats the given \p Ranges in the file \p ID.
 ///
 /// Each range is extended on either end to its next bigger logic unit, i.e.
 /// everything that might influence its formatting or might be influenced by its

cfe/trunk/lib/Format/Format.cpp

auto Reformat = [](const FormatStyle &Style, StringRef Code,
     return reformat(Style, Code, Ranges, FileName);
   };
   return processReplacements(Reformat, Code, *SortedReplaces, Style);
 }
 
 namespace {
 
 inline bool isHeaderInsertion(const tooling::Replacement &Replace) {
-  return Replace.getOffset() == UINT_MAX &&
+  return Replace.getOffset() == UINT_MAX && Replace.getLength() == 0 &&
          llvm::Regex(IncludeRegexPattern).match(Replace.getReplacementText());
 }
 
+inline bool isHeaderDeletion(const tooling::Replacement &Replace) {
+  return Replace.getOffset() == UINT_MAX && Replace.getLength() == 1;
+}
+
 void skipComments(Lexer &Lex, Token &Tok) {
   while (Tok.is(tok::comment))
     if (Lex.LexFromRawLexer(Tok))
       return;
 }
 
 // Check if a sequence of tokens is like "#<Name> <raw_identifier>". If it is,
 // \p Tok will be the token after this directive; otherwise, it can be any token
unsigned getOffsetAfterHeaderGuardsAndComments(StringRef FileName,
   if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
     skipComments(Lex, Tok);
     if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
       return SourceMgr.getFileOffset(Tok.getLocation());
   }
   return AfterComments;
 }
 
+bool isDeletedHeader(llvm::StringRef HeaderName,
+                     const std::set<llvm::StringRef> HeadersToDelete) {
+  return HeadersToDelete.find(HeaderName) != HeadersToDelete.end() ||
+         HeadersToDelete.find(HeaderName.trim("\"<>")) != HeadersToDelete.end();
+}
+
 // FIXME: we also need to insert a '\n' at the end of the code if we have an
 // insertion with offset Code.size(), and there is no '\n' at the end of the
 // code.
 // FIXME: do not insert headers into conditional #include blocks, e.g. #includes
 // surrounded by compile condition "#if...".
 // FIXME: insert empty lines between newly created blocks.
 tooling::Replacements
 fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
                         const FormatStyle &Style) {
   if (Style.Language != FormatStyle::LanguageKind::LK_Cpp)
     return Replaces;
 
   tooling::Replacements HeaderInsertions;
+  std::set<llvm::StringRef> HeadersToDelete;
   tooling::Replacements Result;
   for (const auto &R : Replaces) {
     if (isHeaderInsertion(R)) {
       // Replacements from \p Replaces must be conflict-free already, so we can
       // simply consume the error.
       llvm::consumeError(HeaderInsertions.add(R));
+    } else if (isHeaderDeletion(R)) {
+      HeadersToDelete.insert(R.getReplacementText());
     } else if (R.getOffset() == UINT_MAX) {
       llvm::errs() << "Insertions other than header #include insertion are "
                       "not supported! "
                    << R.getReplacementText() << "\n";
     } else {
       llvm::consumeError(Result.add(R));
     }
   }
-  if (HeaderInsertions.empty())
+  if (HeaderInsertions.empty() && HeadersToDelete.empty())
     return Replaces;
 
   llvm::Regex IncludeRegex(IncludeRegexPattern);
   llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)");
   SmallVector<StringRef, 4> Matches;
 
   StringRef FileName = Replaces.begin()->getFilePath();
   IncludeCategoryManager Categories(Style, FileName);
fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
   SmallVector<StringRef, 32> Lines;
   TrimmedCode.split(Lines, '\n');
   unsigned Offset = MinInsertOffset;
   unsigned NextLineOffset;
   std::set<StringRef> ExistingIncludes;
   for (auto Line : Lines) {
     NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
     if (IncludeRegex.match(Line, &Matches)) {
+      // The header name with quotes or angle brackets.
       StringRef IncludeName = Matches[2];
       ExistingIncludes.insert(IncludeName);
       int Category = Categories.getIncludePriority(
           IncludeName, /*CheckMainHeader=*/FirstIncludeOffset < 0);
       CategoryEndOffsets[Category] = NextLineOffset;
       if (FirstIncludeOffset < 0)
         FirstIncludeOffset = Offset;
+      if (isDeletedHeader(IncludeName, HeadersToDelete)) {
+        // If this is the last line without trailing newline, we need to make
+        // sure we don't delete across the file boundary.
+        unsigned Length = std::min(Line.size() + 1, Code.size() - Offset);
+        llvm::Error Err =
+            Result.add(tooling::Replacement(FileName, Offset, Length, ""));
+        if (Err) {
+          // Ignore the deletion on conflict.
+          llvm::errs() << "Failed to add header deletion replacement for "
+                       << IncludeName << ": " << llvm::toString(std::move(Err))
+                       << "\n";
+        }
+      }
     }
     Offset = NextLineOffset;
   }
 
   // Populate CategoryEndOfssets:
   // - Ensure that CategoryEndOffset[Highest] is always populated.
   // - If CategoryEndOffset[Priority] isn't set, use the next higher value that
   //   is set, up to CategoryEndOffset[Highest].

cfe/trunk/unittests/Format/CleanupTest.cpp

 
 class CleanUpReplacementsTest : public ReplacementTest {
 protected:
   tooling::Replacement createReplacement(unsigned Offset, unsigned Length,
                                          StringRef Text) {
     return tooling::Replacement(FileName, Offset, Length, Text);
   }
 
-  tooling::Replacement createInsertion(StringRef HeaderName) {
-    return createReplacement(UINT_MAX, 0, HeaderName);
+  tooling::Replacement createInsertion(StringRef IncludeDirective) {
+    return createReplacement(UINT_MAX, 0, IncludeDirective);
+  }
+
+  tooling::Replacement createDeletion(StringRef HeaderName) {
+    return createReplacement(UINT_MAX, 1, HeaderName);
   }
 
   inline std::string apply(StringRef Code,
                            const tooling::Replacements Replaces) {
     auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
     EXPECT_TRUE(static_cast<bool>(CleanReplaces))
         << llvm::toString(CleanReplaces.takeError()) << "\n";
     auto Result = applyAllReplacements(Code, *CleanReplaces);
std::string Expected = "#include \"a.h\"\n"
                          "#include <vector>\n"
                          "#include <a.h>\n";
   tooling::Replacements Replaces =
       toReplacements({createInsertion("#include \"vector\""),
                       createInsertion("#include <a.h>")});
   EXPECT_EQ(Expected, apply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, SimpleDeleteIncludes) {
+  std::string Code = "#include \"abc.h\"\n"
+                     "#include \"xyz.h\" // comment\n"
+                     "#include \"xyz\"\n"
+                     "int x;\n";
+  std::string Expected = "#include \"xyz\"\n"
+                         "int x;\n";
+  tooling::Replacements Replaces =
+      toReplacements({createDeletion("abc.h"), createDeletion("xyz.h")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, DeleteAllCode) {
+  std::string Code = "#include \"xyz.h\"\n"
+                     "#include <xyz.h>";
+  std::string Expected = "";
+  tooling::Replacements Replaces = toReplacements({createDeletion("xyz.h")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, DeleteAllIncludesWithSameNameIfNoType) {
+  std::string Code = "#include \"xyz.h\"\n"
+                     "#include \"xyz\"\n"
+                     "#include <xyz.h>\n";
+  std::string Expected = "#include \"xyz\"\n";
+  tooling::Replacements Replaces = toReplacements({createDeletion("xyz.h")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, OnlyDeleteHeaderWithType) {
+  std::string Code = "#include \"xyz.h\"\n"
+                     "#include \"xyz\"\n"
+                     "#include <xyz.h>";
+  std::string Expected = "#include \"xyz.h\"\n"
+                         "#include \"xyz\"\n";
+  tooling::Replacements Replaces = toReplacements({createDeletion("<xyz.h>")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertionAndDeleteHeader) {
+  std::string Code = "#include \"a.h\"\n"
+                     "\n"
+                     "#include <vector>\n";
+  std::string Expected = "#include \"a.h\"\n"
+                         "\n"
+                         "#include <map>\n";
+  tooling::Replacements Replaces = toReplacements(
+      {createDeletion("<vector>"), createInsertion("#include <map>")});
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang