[clang-tools-extra] 1a8dd74 - [include-cleaner] clang-include-cleaner can print/apply edits
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 2 04:17:06 PST 2022
Author: Sam McCall
Date: 2022-12-02T13:16:58+01:00
New Revision: 1a8dd7425873fae55a38cbdb41cccb1f17c82e8c
URL: https://github.com/llvm/llvm-project/commit/1a8dd7425873fae55a38cbdb41cccb1f17c82e8c
DIFF: https://github.com/llvm/llvm-project/commit/1a8dd7425873fae55a38cbdb41cccb1f17c82e8c.diff
LOG: [include-cleaner] clang-include-cleaner can print/apply edits
This adds command-line flags to the tool:
+ -print: prints changed source code
+ -print=changes: prints headers added/removed
+ -edit: rewrites code in place
+ -insert=0/-remove=0: disables additions/deletions for the above
These are supported by a couple of new functions dumped into Analysis:
analyze() sits on top of walkUsed and makes used/unused decisions for
Includes. fixIncludes() applies those results to source code.
Differential Revision: https://reviews.llvm.org/D139013
Added:
clang-tools-extra/include-cleaner/test/Inputs/foobar.h
clang-tools-extra/include-cleaner/test/tool.cpp
Modified:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang/lib/Format/Format.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 31ae7592ceec0..146c652f730de 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -13,14 +13,21 @@
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
+#include "clang/Format/Format.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/Support/MemoryBufferRef.h"
#include <variant>
namespace clang {
class SourceLocation;
class Decl;
class FileEntry;
+class HeaderSearch;
+namespace tooling {
+class Replacements;
+struct IncludeStyle;
+} // namespace tooling
namespace include_cleaner {
/// A UsedSymbolCB is a callback invoked for each symbol reference seen.
@@ -47,6 +54,24 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
llvm::ArrayRef<SymbolReference> MacroRefs,
const PragmaIncludes *PI, const SourceManager &, UsedSymbolCB CB);
+struct AnalysisResults {
+ std::vector<const Include *> Unused;
+ std::vector<std::string> Missing; // Spellings, like "<vector>"
+};
+
+/// Determine which headers should be inserted or removed from the main file.
+/// This exposes conclusions but not reasons: use lower-level walkUsed for that.
+AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
+ llvm::ArrayRef<SymbolReference> MacroRefs,
+ const Includes &I, const PragmaIncludes *PI,
+ const SourceManager &SM, HeaderSearch &HS);
+
+/// Removes unused includes and inserts missing ones in the main file.
+/// Returns the modified main-file code.
+/// The FormatStyle must be C++ or ObjC (to support include ordering).
+std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
+ const format::FormatStyle &IncludeStyle);
+
} // namespace include_cleaner
} // namespace clang
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 0f96ae26f51c5..c83b4d5a25be0 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -11,6 +11,10 @@
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
@@ -42,4 +46,69 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
}
}
+static std::string spellHeader(const Header &H, HeaderSearch &HS,
+ const FileEntry *Main) {
+ switch (H.kind()) {
+ case Header::Physical: {
+ bool IsSystem = false;
+ std::string Path = HS.suggestPathToFileForDiagnostics(
+ H.physical(), Main->tryGetRealPathName(), &IsSystem);
+ return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
+ }
+ case Header::Standard:
+ return H.standard().name().str();
+ case Header::Verbatim:
+ return H.verbatim().str();
+ }
+ llvm_unreachable("Unknown Header kind");
+}
+
+AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
+ llvm::ArrayRef<SymbolReference> MacroRefs,
+ const Includes &Inc, const PragmaIncludes *PI,
+ const SourceManager &SM, HeaderSearch &HS) {
+ const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+ llvm::DenseSet<const Include *> Used;
+ llvm::StringSet<> Missing;
+ walkUsed(ASTRoots, MacroRefs, PI, SM,
+ [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
+ bool Satisfied = false;
+ for (const Header &H : Providers) {
+ if (H.kind() == Header::Physical && H.physical() == MainFile)
+ Satisfied = true;
+ for (const Include *I : Inc.match(H)) {
+ Used.insert(I);
+ Satisfied = true;
+ }
+ }
+ if (!Satisfied && !Providers.empty() &&
+ Ref.RT == RefType::Explicit)
+ Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+ });
+
+ AnalysisResults Results;
+ for (const Include &I : Inc.all())
+ if (!Used.contains(&I))
+ Results.Unused.push_back(&I);
+ for (llvm::StringRef S : Missing.keys())
+ Results.Missing.push_back(S.str());
+ llvm::sort(Results.Missing);
+ return Results;
+}
+
+std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
+ const format::FormatStyle &Style) {
+ assert(Style.isCpp() && "Only C++ style supports include insertions!");
+ tooling::Replacements R;
+ // Encode insertions/deletions in the magic way clang-format understands.
+ for (const Include *I : Results.Unused)
+ cantFail(R.add(tooling::Replacement("input", UINT_MAX, 1, I->quote())));
+ for (llvm::StringRef Spelled : Results.Missing)
+ cantFail(R.add(tooling::Replacement("input", UINT_MAX, 0,
+ ("#include " + Spelled).str())));
+ // "cleanup" actually turns the UINT_MAX replacements into concrete edits.
+ auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
+ return cantFail(tooling::applyAllReplacements(Code, Positioned));
+}
+
} // namespace clang::include_cleaner
diff --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index 99f73836a5ad3..5c83e8fdc1515 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -14,6 +14,7 @@ clang_target_link_libraries(clangIncludeCleaner
PRIVATE
clangAST
clangBasic
+ clangFormat
clangLex
clangToolingInclusions
clangToolingInclusionsStdlib
diff --git a/clang-tools-extra/include-cleaner/test/Inputs/foobar.h b/clang-tools-extra/include-cleaner/test/Inputs/foobar.h
new file mode 100644
index 0000000000000..da3706c67dbed
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/test/Inputs/foobar.h
@@ -0,0 +1,3 @@
+#pragma once
+#include "bar.h"
+#include "foo.h"
diff --git a/clang-tools-extra/include-cleaner/test/tool.cpp b/clang-tools-extra/include-cleaner/test/tool.cpp
new file mode 100644
index 0000000000000..937099a39ce98
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/test/tool.cpp
@@ -0,0 +1,25 @@
+#include "foobar.h"
+
+int x = foo();
+
+// RUN: clang-include-cleaner -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGE %s
+// CHANGE: - "foobar.h"
+// CHANGE-NEXT: + "foo.h"
+
+// RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
+// INSERT-NOT: - "foobar.h"
+// INSERT: + "foo.h"
+
+// RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
+// REMOVE: - "foobar.h"
+// REMOVE-NOT: + "foo.h"
+
+// RUN: clang-include-cleaner -print %s -- -I%S/Inputs/ | FileCheck --match-full-lines --check-prefix=PRINT %s
+// PRINT: #include "foo.h"
+// PRINT-NOT: {{^}}#include "foobar.h"{{$}}
+
+// RUN: cp %s %t.cpp
+// RUN: clang-include-cleaner -edit %t.cpp -- -I%S/Inputs/
+// RUN: FileCheck --match-full-lines --check-prefix=EDIT %s < %t.cpp
+// EDIT: #include "foo.h"
+// EDIT-NOT: {{^}}#include "foobar.h"{{$}}
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 80450c86e3f30..0a920f6e9ca0a 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Record.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
@@ -46,7 +47,48 @@ cl::opt<std::string> HTMLReportPath{
cl::cat(IncludeCleaner),
};
-class HTMLReportAction : public clang::ASTFrontendAction {
+enum class PrintStyle { Changes, Final };
+cl::opt<PrintStyle> Print{
+ "print",
+ cl::values(
+ clEnumValN(PrintStyle::Changes, "changes", "Print symbolic changes"),
+ clEnumValN(PrintStyle::Final, "", "Print final code")),
+ cl::ValueOptional,
+ cl::init(PrintStyle::Final),
+ cl::desc("Print the list of headers to insert and remove"),
+ cl::cat(IncludeCleaner),
+};
+
+cl::opt<bool> Edit{
+ "edit",
+ cl::desc("Apply edits to analyzed source files"),
+ cl::cat(IncludeCleaner),
+};
+
+cl::opt<bool> Insert{
+ "insert",
+ cl::desc("Allow header insertions"),
+ cl::init(true),
+};
+cl::opt<bool> Remove{
+ "remove",
+ cl::desc("Allow header removals"),
+ cl::init(true),
+};
+
+std::atomic<unsigned> Errors = ATOMIC_VAR_INIT(0);
+
+format::FormatStyle getStyle(llvm::StringRef Filename) {
+ auto S = format::getStyle(format::DefaultFormatStyle, Filename,
+ format::DefaultFallbackStyle);
+ if (!S || !S->isCpp()) {
+ consumeError(S.takeError());
+ return format::getLLVMStyle();
+ }
+ return std::move(*S);
+}
+
+class Action : public clang::ASTFrontendAction {
RecordedAST AST;
RecordedPP PP;
PragmaIncludes PI;
@@ -64,12 +106,59 @@ class HTMLReportAction : public clang::ASTFrontendAction {
}
void EndSourceFile() override {
+ if (!HTMLReportPath.empty())
+ writeHTML();
+
+ const auto &SM = getCompilerInstance().getSourceManager();
+ auto &HS = getCompilerInstance().getPreprocessor().getHeaderSearchInfo();
+ llvm::StringRef Path =
+ SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+ assert(!Path.empty() && "Main file path not known?");
+ llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());
+
+ auto Results =
+ analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS);
+ if (!Insert)
+ Results.Missing.clear();
+ if (!Remove)
+ Results.Unused.clear();
+ std::string Final = fixIncludes(Results, Code, getStyle(Path));
+
+ if (Print.getNumOccurrences()) {
+ switch (Print) {
+ case PrintStyle::Changes:
+ for (const Include *I : Results.Unused)
+ llvm::outs() << "- " << I->quote() << "\n";
+ for (const auto &I : Results.Missing)
+ llvm::outs() << "+ " << I << "\n";
+ break;
+ case PrintStyle::Final:
+ llvm::outs() << Final;
+ break;
+ }
+ }
+
+ if (Edit) {
+ if (auto Err = llvm::writeToOutput(
+ Path, [&](llvm::raw_ostream &OS) -> llvm::Error {
+ OS << Final;
+ return llvm::Error::success();
+ })) {
+ llvm::errs() << "Failed to apply edits to " << Path << ": "
+ << toString(std::move(Err)) << "\n";
+ ++Errors;
+ }
+ }
+ }
+
+ void writeHTML() {
std::error_code EC;
llvm::raw_fd_ostream OS(HTMLReportPath, EC);
if (EC) {
llvm::errs() << "Unable to write HTML report to " << HTMLReportPath
<< ": " << EC.message() << "\n";
- exit(1);
+ ++Errors;
+ return;
}
writeHTMLReport(
AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
@@ -93,20 +182,17 @@ int main(int argc, const char **argv) {
return 1;
}
- std::unique_ptr<clang::tooling::FrontendActionFactory> Factory;
- if (HTMLReportPath.getNumOccurrences()) {
- if (OptionsParser->getSourcePathList().size() != 1) {
- llvm::errs() << "-" << HTMLReportPath.ArgStr
- << " requires a single input file";
+ if (OptionsParser->getSourcePathList().size() != 1) {
+ std::vector<cl::Option *> IncompatibleFlags = {&HTMLReportPath, &Print};
+ for (const auto *Flag : IncompatibleFlags) {
+ if (Flag->getNumOccurrences())
+ llvm::errs() << "-" << Flag->ArgStr << " requires a single input file";
return 1;
}
- Factory = clang::tooling::newFrontendActionFactory<HTMLReportAction>();
- } else {
- llvm::errs() << "Unimplemented\n";
- return 1;
}
-
+ auto Factory = clang::tooling::newFrontendActionFactory<Action>();
return clang::tooling::ClangTool(OptionsParser->getCompilations(),
OptionsParser->getSourcePathList())
- .run(Factory.get());
+ .run(Factory.get()) ||
+ Errors != 0;
}
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 2149800c966d1..c6b4801bc2fb9 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -18,6 +18,7 @@
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Testing/Support/Annotations.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -25,6 +26,7 @@
namespace clang::include_cleaner {
namespace {
+using testing::ElementsAre;
using testing::Pair;
using testing::UnorderedElementsAre;
@@ -134,6 +136,83 @@ TEST(WalkUsed, MacroRefs) {
UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile))));
}
+TEST(Analyze, Basic) {
+ TestInputs Inputs;
+ Inputs.Code = R"cpp(
+#include "a.h"
+#include "b.h"
+
+int x = a + c;
+)cpp";
+ Inputs.ExtraFiles["a.h"] = guard("int a;");
+ Inputs.ExtraFiles["b.h"] = guard(R"cpp(
+ #include "c.h"
+ int b;
+ )cpp");
+ Inputs.ExtraFiles["c.h"] = guard("int c;");
+
+ RecordedPP PP;
+ Inputs.MakeAction = [&PP] {
+ struct Hook : public SyntaxOnlyAction {
+ public:
+ Hook(RecordedPP &PP) : PP(PP) {}
+ bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+ CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+ return true;
+ }
+
+ RecordedPP &PP;
+ };
+ return std::make_unique<Hook>(PP);
+ };
+
+ TestAST AST(Inputs);
+ auto Decls = AST.context().getTranslationUnitDecl()->decls();
+ auto Results =
+ analyze(std::vector<Decl *>{Decls.begin(), Decls.end()},
+ PP.MacroReferences, PP.Includes, /*PragmaIncludes=*/nullptr,
+ AST.sourceManager(), AST.preprocessor().getHeaderSearchInfo());
+
+ const Include *B = PP.Includes.atLine(3);
+ ASSERT_EQ(B->Spelled, "b.h");
+ EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
+ EXPECT_THAT(Results.Unused, ElementsAre(B));
+}
+
+TEST(FixIncludes, Basic) {
+ llvm::StringRef Code = R"cpp(
+#include "a.h"
+#include "b.h"
+#include <c.h>
+)cpp";
+
+ Includes Inc;
+ Include I;
+ I.Spelled = "a.h";
+ I.Line = 2;
+ Inc.add(I);
+ I.Spelled = "b.h";
+ I.Line = 3;
+ Inc.add(I);
+ I.Spelled = "c.h";
+ I.Line = 4;
+ I.Angled = true;
+ Inc.add(I);
+
+ AnalysisResults Results;
+ Results.Missing.push_back("\"aa.h\"");
+ Results.Missing.push_back("\"ab.h\"");
+ Results.Missing.push_back("<e.h>");
+ Results.Unused.push_back(Inc.atLine(3));
+ Results.Unused.push_back(Inc.atLine(4));
+
+ EXPECT_EQ(fixIncludes(Results, Code, format::getLLVMStyle()), R"cpp(
+#include "a.h"
+#include "aa.h"
+#include "ab.h"
+#include <e.h>
+)cpp");
+}
} // namespace
} // namespace clang::include_cleaner
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 3e82014566c4c..bfc2741ea96ae 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3333,7 +3333,7 @@ cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
// Make header insertion replacements insert new headers into correct blocks.
tooling::Replacements NewReplaces =
fixCppIncludeInsertions(Code, Replaces, Style);
- return processReplacements(Cleanup, Code, NewReplaces, Style);
+ return cantFail(processReplacements(Cleanup, Code, NewReplaces, Style));
}
namespace internal {
More information about the cfe-commits
mailing list