[include-cleaner] Attach Header to AnalysisResults for misisng headers (#110272)

Currently callers of analyze can't get detailed information about a
missing header, e.g. resolve path. Only way to get at this is to use low
level walkUsed funciton, which is way more complicated than just calling
analyze.

This enables further analysis, e.g. when includes are spelled relative
to inner directories, caller can still know their path relative to
repository root.
This commit is contained in:
kadir çetinkaya
2024-09-30 04:57:19 +02:00
committed by GitHub
parent 64f2bff12b
commit 4ef77d61b2
4 changed files with 24 additions and 18 deletions

View File

@@ -21,6 +21,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <string>
#include <utility>
namespace clang {
class SourceLocation;
@@ -62,7 +63,8 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
struct AnalysisResults {
std::vector<const Include *> Unused;
std::vector<std::string> Missing; // Spellings, like "<vector>"
// Spellings, like "<vector>" paired with the Header that generated it.
std::vector<std::pair<std::string, Header>> Missing;
};
/// Determine which headers should be inserted or removed from the main file.

View File

@@ -26,8 +26,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
@@ -84,7 +84,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
auto &SM = PP.getSourceManager();
const auto MainFile = *SM.getFileEntryRefForID(SM.getMainFileID());
llvm::DenseSet<const Include *> Used;
llvm::StringSet<> Missing;
llvm::StringMap<Header> Missing;
if (!HeaderFilter)
HeaderFilter = [](llvm::StringRef) { return false; };
OptionalDirectoryEntryRef ResourceDir =
@@ -119,7 +119,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
Satisfied = true;
}
if (!Satisfied)
Missing.insert(std::move(Spelling));
Missing.try_emplace(std::move(Spelling), Providers.front());
});
AnalysisResults Results;
@@ -144,8 +144,8 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
}
Results.Unused.push_back(&I);
}
for (llvm::StringRef S : Missing.keys())
Results.Missing.push_back(S.str());
for (auto &E : Missing)
Results.Missing.emplace_back(E.first().str(), E.second);
llvm::sort(Results.Missing);
return Results;
}
@@ -158,9 +158,9 @@ std::string fixIncludes(const AnalysisResults &Results,
// Encode insertions/deletions in the magic way clang-format understands.
for (const Include *I : Results.Unused)
cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 1, I->quote())));
for (llvm::StringRef Spelled : Results.Missing)
cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 0,
("#include " + Spelled).str())));
for (auto &[Spelled, _] : Results.Missing)
cantFail(R.add(
tooling::Replacement(FileName, UINT_MAX, 0, "#include " + Spelled)));
// "cleanup" actually turns the UINT_MAX replacements into concrete edits.
auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
return cantFail(tooling::applyAllReplacements(Code, Positioned));

View File

@@ -192,7 +192,7 @@ private:
case PrintStyle::Changes:
for (const Include *I : Results.Unused)
llvm::outs() << "- " << I->quote() << " @Line:" << I->Line << "\n";
for (const auto &I : Results.Missing)
for (const auto &[I, _] : Results.Missing)
llvm::outs() << "+ " << I << "\n";
break;
case PrintStyle::Final:

View File

@@ -25,6 +25,7 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/VirtualFileSystem.h"
@@ -39,6 +40,7 @@
namespace clang::include_cleaner {
namespace {
using testing::_;
using testing::AllOf;
using testing::Contains;
using testing::ElementsAre;
@@ -262,10 +264,12 @@ int x = a + c;
auto Results =
analyze(std::vector<Decl *>{Decls.begin(), Decls.end()},
PP.MacroReferences, PP.Includes, &PI, AST.preprocessor());
auto CHeader = llvm::cantFail(
AST.context().getSourceManager().getFileManager().getFileRef("c.h"));
const Include *B = PP.Includes.atLine(3);
ASSERT_EQ(B->Spelled, "b.h");
EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
EXPECT_THAT(Results.Missing, ElementsAre(Pair("\"c.h\"", Header(CHeader))));
EXPECT_THAT(Results.Unused, ElementsAre(B));
}
@@ -370,7 +374,7 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
auto Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor());
// Check that we're spelling header using the symlink, and not underlying
// path.
EXPECT_THAT(Results.Missing, testing::ElementsAre("\"inner.h\""));
EXPECT_THAT(Results.Missing, testing::ElementsAre(Pair("\"inner.h\"", _)));
// header.h should be unused.
EXPECT_THAT(Results.Unused, Not(testing::IsEmpty()));
@@ -379,7 +383,7 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
auto HeaderFilter = [](llvm::StringRef Path) { return Path == "inner.h"; };
Results = analyze(DeclsInTU, {}, PP.Includes, &PI, AST.preprocessor(),
HeaderFilter);
EXPECT_THAT(Results.Missing, testing::ElementsAre("\"inner.h\""));
EXPECT_THAT(Results.Missing, testing::ElementsAre(Pair("\"inner.h\"", _)));
// header.h should be unused.
EXPECT_THAT(Results.Unused, Not(testing::IsEmpty()));
}
@@ -389,7 +393,7 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
HeaderFilter);
// header.h should be ignored now.
EXPECT_THAT(Results.Unused, Not(testing::IsEmpty()));
EXPECT_THAT(Results.Missing, testing::ElementsAre("\"inner.h\""));
EXPECT_THAT(Results.Missing, testing::ElementsAre(Pair("\"inner.h\"", _)));
}
}
@@ -414,9 +418,9 @@ TEST(FixIncludes, Basic) {
Inc.add(I);
AnalysisResults Results;
Results.Missing.push_back("\"aa.h\"");
Results.Missing.push_back("\"ab.h\"");
Results.Missing.push_back("<e.h>");
Results.Missing.emplace_back("\"aa.h\"", Header(""));
Results.Missing.emplace_back("\"ab.h\"", Header(""));
Results.Missing.emplace_back("<e.h>", Header(""));
Results.Unused.push_back(Inc.atLine(3));
Results.Unused.push_back(Inc.atLine(4));
@@ -429,7 +433,7 @@ R"cpp(#include "d.h"
)cpp");
Results = {};
Results.Missing.push_back("\"d.h\"");
Results.Missing.emplace_back("\"d.h\"", Header(""));
Code = R"cpp(#include "a.h")cpp";
EXPECT_EQ(fixIncludes(Results, "d.cc", Code, format::getLLVMStyle()),
R"cpp(#include "d.h"