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 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h @@ -15,12 +15,18 @@ #include "clang-include-cleaner/Types.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLFunctionalExtras.h" +#include "llvm/Support/MemoryBufferRef.h" #include 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 +53,24 @@ llvm::ArrayRef MacroRefs, const PragmaIncludes *PI, const SourceManager &, UsedSymbolCB CB); +struct AnalysisResults { + std::vector Unused; + std::vector Missing; // Spellings, like "" +}; + +/// 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 ASTRoots, + llvm::ArrayRef MacroRefs, + const Includes &, 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. +std::string fixIncludes(const AnalysisResults &Results, + const llvm::MemoryBufferRef MainFile, + const tooling::IncludeStyle &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 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -8,9 +8,13 @@ #include "clang-include-cleaner/Analysis.h" #include "AnalysisInternal.h" +#include "clang-include-cleaner/Record.h" // FIXME: move/rename RecordedIncludes #include "clang-include-cleaner/Types.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/SourceManager.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,90 @@ } } +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 ASTRoots, + llvm::ArrayRef MacroRefs, + const Includes &Inc, const PragmaIncludes *PI, + const SourceManager &SM, HeaderSearch &HS) { + const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); + llvm::DenseSet Used; + llvm::StringSet<> Missing; + walkUsed(ASTRoots, MacroRefs, PI, SM, + [&](const SymbolReference &Ref, llvm::ArrayRef
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, + const llvm::MemoryBufferRef MainFile, + const tooling::IncludeStyle &IncludeStyle) { + tooling::HeaderIncludes HI(MainFile.getBufferIdentifier(), + MainFile.getBuffer(), IncludeStyle); + // tooling::Replacements has an irritating "order-independent" invariant + // that forces us to pre-merge any insertions at the same point. + tooling::Replacements Result; + llvm::Optional PendingInsert; + for (llvm::StringRef Insert : Results.Missing) { + if (auto Edit = HI.insert(Insert.trim("<>\""), Insert.starts_with("<"))) { + if (PendingInsert && Edit->getOffset() == PendingInsert->getOffset()) { + PendingInsert = tooling::Replacement( + PendingInsert->getFilePath(), PendingInsert->getOffset(), + PendingInsert->getLength(), + (PendingInsert->getReplacementText() + Edit->getReplacementText()) + .str()); + } else { + if (PendingInsert) + cantFail(Result.add(std::move(*PendingInsert))); + PendingInsert = std::move(Edit); + } + } + } + if (PendingInsert) + cantFail(Result.add(std::move(*PendingInsert))); + for (const Include *Unused : Results.Unused) { + for (const auto &Edit : HI.remove(Unused->Spelled, Unused->Angled)) + // HeaderIncludes::remove is by name rather than line number, so we + // could remove the same header multiple times. Ignore that. + consumeError(Result.add(Edit)); + } + + return cantFail(tooling::applyAllReplacements(MainFile.getBuffer(), Result)); +} + } // namespace clang::include_cleaner 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 --- /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 --- /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 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -7,11 +7,13 @@ //===----------------------------------------------------------------------===// #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" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CommonOptionsParser.h" +#include "clang/Tooling/Inclusions/IncludeStyle.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/CommandLine.h" @@ -46,7 +48,38 @@ cl::cat(IncludeCleaner), }; -class HTMLReportAction : public clang::ASTFrontendAction { +enum class PrintStyle { Changes, Final }; +cl::opt 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 Edit{ + "edit", + cl::desc("Apply edits to analyzed source files"), + cl::cat(IncludeCleaner), +}; + +cl::opt Insert{ + "insert", + cl::desc("Allow header insertions"), + cl::init(true), +}; +cl::opt Remove{ + "remove", + cl::desc("Allow header removals"), + cl::init(true), +}; + +std::atomic Errors = ATOMIC_VAR_INIT(0); + +class Action : public clang::ASTFrontendAction { RecordedAST AST; RecordedPP PP; PragmaIncludes PI; @@ -64,12 +97,61 @@ } void EndSourceFile() override { + if (!HTMLReportPath.empty()) + writeHTML(); + + const auto &SM = getCompilerInstance().getSourceManager(); + auto &HS = getCompilerInstance().getPreprocessor().getHeaderSearchInfo(); + auto Results = + analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS); + if (!Insert) + Results.Missing.clear(); + if (!Remove) + Results.Unused.clear(); + + auto Main = SM.getBufferOrFake(SM.getMainFileID()); + tooling::IncludeStyle IncludeStyle; // FIXME: determine properly somehow + std::string Final = fixIncludes(Results, Main, IncludeStyle); + + 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) { + llvm::StringRef Path = + SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName(); + assert(!Path.empty() && "Main file path not known?"); + 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 " + << Main.getBufferIdentifier() << ": " + << 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 +175,17 @@ return 1; } - std::unique_ptr 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 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(); - } else { - llvm::errs() << "Unimplemented\n"; - return 1; } - + auto Factory = clang::tooling::newFrontendActionFactory(); 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 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -15,9 +15,11 @@ #include "clang/Basic/SourceManager.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Testing/TestAST.h" +#include "clang/Tooling/Inclusions/IncludeStyle.h" #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 +27,7 @@ namespace clang::include_cleaner { namespace { +using testing::ElementsAre; using testing::Pair; using testing::UnorderedElementsAre; @@ -134,6 +137,91 @@ 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"] = R"cpp( + #pragma once + int a; + )cpp"; + Inputs.ExtraFiles["b.h"] = R"cpp( + #pragma once + #include "c.h" + int b; + )cpp"; + Inputs.ExtraFiles["c.h"] = R"cpp( + #pragma once + int c; + )cpp"; + + 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(PP); + }; + + TestAST AST(Inputs); + auto Decls = AST.context().getTranslationUnitDecl()->decls(); + auto Results = + analyze(std::vector{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) { + auto Code = llvm::MemoryBuffer::getMemBufferCopy(R"cpp( +#include "a.h" +#include "b.h" +#include +)cpp", + "test.cc"); + + 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(""); + Results.Unused.push_back(Inc.atLine(3)); + Results.Unused.push_back(Inc.atLine(4)); + + EXPECT_EQ(fixIncludes(Results, *Code, tooling::IncludeStyle()), R"cpp( +#include "a.h" +#include "aa.h" +#include "ab.h" +#include +)cpp"); +} } // namespace } // namespace clang::include_cleaner