[clang-tools-extra] 2eb5ac9 - Revert "[clangd] Add support for missing includes analysis."
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 7 19:14:30 PST 2023
Author: Nico Weber
Date: 2023-03-07T22:14:11-05:00
New Revision: 2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd
URL: https://github.com/llvm/llvm-project/commit/2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd
DIFF: https://github.com/llvm/llvm-project/commit/2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd.diff
LOG: Revert "[clangd] Add support for missing includes analysis."
This reverts commit 38b9fb5a129db3e086610d53b534833273c5b4d0.
Breaks tests on Windows, see comments on https://reviews.llvm.org/D143496
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index ab346aab0e8a..dffd54b01c45 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -88,12 +88,11 @@ struct Config {
bool StandardLibrary = true;
} Index;
- enum class IncludesPolicy {
- /// Diagnose missing and unused includes.
+ enum class UnusedIncludesPolicy {
+ /// Diagnose unused includes.
Strict,
None,
- /// The same as Strict, but using the include-cleaner library for
- /// unused includes.
+ /// The same as Strict, but using the include-cleaner library.
Experiment,
};
/// Controls warnings and errors when parsing code.
@@ -108,12 +107,11 @@ struct Config {
llvm::StringMap<std::string> CheckOptions;
} ClangTidy;
+ UnusedIncludesPolicy UnusedIncludes = UnusedIncludesPolicy::None;
+
/// Enable emitting diagnostics using stale preambles.
bool AllowStalePreamble = false;
- IncludesPolicy UnusedIncludes = IncludesPolicy::None;
- IncludesPolicy MissingIncludes = IncludesPolicy::None;
-
/// IncludeCleaner will not diagnose usages of these headers matched by
/// these regexes.
struct {
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 2f0ef892131c..18de6e4d5c3b 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -431,16 +431,16 @@ struct FragmentCompiler {
});
if (F.UnusedIncludes)
- if (auto Val = compileEnum<Config::IncludesPolicy>("UnusedIncludes",
- **F.UnusedIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
- .map("None", Config::IncludesPolicy::None)
- .value())
+ if (auto Val =
+ compileEnum<Config::UnusedIncludesPolicy>("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::UnusedIncludesPolicy::Strict)
+ .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
+ .map("None", Config::UnusedIncludesPolicy::None)
+ .value())
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
-
if (F.AllowStalePreamble) {
if (auto Val = F.AllowStalePreamble)
Out.Apply.push_back([Val](const Params &, Config &C) {
@@ -448,16 +448,6 @@ struct FragmentCompiler {
});
}
- if (F.MissingIncludes)
- if (auto Val = compileEnum<Config::IncludesPolicy>("MissingIncludes",
- **F.MissingIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("None", Config::IncludesPolicy::None)
- .value())
- Out.Apply.push_back([Val](const Params &, Config &C) {
- C.Diagnostics.MissingIncludes = *Val;
- });
-
compile(std::move(F.Includes));
compile(std::move(F.ClangTidy));
}
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 956e8a859944..a5597596196f 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -221,7 +221,7 @@ struct Fragment {
/// This often has other advantages, such as skipping some analysis.
std::vector<Located<std::string>> Suppress;
- /// Controls how clangd will correct "unnecessary" #include directives.
+ /// Controls how clangd will correct "unnecessary #include directives.
/// clangd can warn if a header is `#include`d but not used, and suggest
/// removing it.
//
@@ -236,23 +236,9 @@ struct Fragment {
/// - None
std::optional<Located<std::string>> UnusedIncludes;
-
/// Enable emitting diagnostics using stale preambles.
std::optional<Located<bool>> AllowStalePreamble;
- /// Controls if clangd should analyze missing #include directives.
- /// clangd will warn if no header providing a symbol is `#include`d
- /// (missing) directly, and suggest adding it.
- ///
- /// Strict means a header providing a symbol is missing if it is not
- /// *directly #include'd. The file might still compile if the header is
- /// included transitively.
- ///
- /// Valid values are:
- /// - Strict
- /// - None
- std::optional<Located<std::string>> MissingIncludes;
-
/// Controls IncludeCleaner diagnostics.
struct IncludesBlock {
/// Regexes that will be used to avoid diagnosing certain includes as
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 84559f5c44f8..0ec0239fc71e 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -128,9 +128,6 @@ class Parser {
Dict.handle("UnusedIncludes", [&](Node &N) {
F.UnusedIncludes = scalarValue(N, "UnusedIncludes");
});
- Dict.handle("MissingIncludes", [&](Node &N) {
- F.MissingIncludes = scalarValue(N, "MissingIncludes");
- });
Dict.handle("Includes", [&](Node &N) { parse(F.Includes, N); });
Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); });
Dict.handle("AllowStalePreamble", [&](Node &N) {
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 0d74b888557e..7d523d9a6364 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,55 +8,32 @@
#include "IncludeCleaner.h"
#include "Config.h"
-#include "Diagnostics.h"
#include "Headers.h"
#include "ParsedAST.h"
#include "Protocol.h"
#include "SourceCode.h"
-#include "URI.h"
#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "support/Logger.h"
-#include "support/Path.h"
#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/AST/TemplateName.h"
-#include "clang/AST/Type.h"
-#include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
-#include "clang/Format/Format.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
-#include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Inclusions/HeaderIncludes.h"
-#include "clang/Tooling/Inclusions/IncludeStyle.h"
-#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
-#include <iterator>
+#include <functional>
#include <optional>
-#include <string>
-#include <vector>
namespace clang {
namespace clangd {
@@ -281,17 +258,6 @@ void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
}
}
-bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) {
- // Convert the path to Unix slashes and try to match against the filter.
- llvm::SmallString<64> NormalizedPath(HeaderPath);
- llvm::sys::path::native(NormalizedPath, llvm::sys::path::Style::posix);
- for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
- if (Filter(NormalizedPath))
- return true;
- }
- return false;
-}
-
static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
const Config &Cfg) {
if (Inc.BehindPragmaKeep)
@@ -322,10 +288,14 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
FE->getName());
return false;
}
-
- if (isFilteredByConfig(Cfg, Inc.Resolved)) {
- dlog("{0} header is filtered out by the configuration", FE->getName());
- return false;
+ for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
+ // Convert the path to Unix slashes and try to match against the filter.
+ llvm::SmallString<64> Path(Inc.Resolved);
+ llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+ if (Filter(Path)) {
+ dlog("{0} header is filtered out by the configuration", FE->getName());
+ return false;
+ }
}
return true;
}
@@ -355,195 +325,6 @@ FileID headerResponsible(FileID ID, const SourceManager &SM,
return ID;
}
-include_cleaner::Includes
-convertIncludes(const SourceManager &SM,
- const llvm::ArrayRef<Inclusion> MainFileIncludes) {
- include_cleaner::Includes Includes;
- for (const Inclusion &Inc : MainFileIncludes) {
- include_cleaner::Include TransformedInc;
- llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
- TransformedInc.Spelled = WrittenRef.trim("\"<>");
- TransformedInc.HashLocation =
- SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
- TransformedInc.Line = Inc.HashLine + 1;
- TransformedInc.Angled = WrittenRef.starts_with("<");
- auto FE = SM.getFileManager().getFile(Inc.Resolved);
- if (!FE) {
- elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
- Inc.Resolved, FE.getError().message());
- continue;
- }
- TransformedInc.Resolved = *FE;
- Includes.add(std::move(TransformedInc));
- }
- return Includes;
-}
-
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
- include_cleaner::Header Provider) {
- if (Provider.kind() == include_cleaner::Header::Physical) {
- if (auto CanonicalPath =
- getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
- std::string SpelledHeader =
- llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
- if (!SpelledHeader.empty())
- return SpelledHeader;
- }
- }
- return include_cleaner::spellHeader(
- Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
-}
-
-std::vector<include_cleaner::SymbolReference>
-collectMacroReferences(ParsedAST &AST) {
- const auto &SM = AST.getSourceManager();
- // FIXME: !!this is a hacky way to collect macro references.
- std::vector<include_cleaner::SymbolReference> Macros;
- auto &PP = AST.getPreprocessor();
- for (const syntax::Token &Tok :
- AST.getTokens().spelledTokens(SM.getMainFileID())) {
- auto Macro = locateMacroAt(Tok, PP);
- if (!Macro)
- continue;
- if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
- Macros.push_back(
- {Tok.location(),
- include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
- DefLoc},
- include_cleaner::RefType::Explicit});
- }
- return Macros;
-}
-
-llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) {
- switch (SymProvider.kind()) {
- case include_cleaner::Header::Physical:
- return SymProvider.physical()->tryGetRealPathName();
- case include_cleaner::Header::Standard:
- return SymProvider.standard().name().trim("<>\"");
- case include_cleaner::Header::Verbatim:
- return SymProvider.verbatim().trim("<>\"");
- }
- llvm_unreachable("Unknown header kind");
-}
-
-std::string getSymbolName(const include_cleaner::Symbol &Sym) {
- switch (Sym.kind()) {
- case include_cleaner::Symbol::Macro:
- return Sym.macro().Name->getName().str();
- case include_cleaner::Symbol::Declaration:
- return llvm::dyn_cast<NamedDecl>(&Sym.declaration())
- ->getQualifiedNameAsString();
- }
- llvm_unreachable("Unknown symbol kind");
-}
-
-std::vector<Diag> generateMissingIncludeDiagnostics(
- ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
- llvm::StringRef Code) {
- std::vector<Diag> Result;
- const Config &Cfg = Config::current();
- if (Cfg.Diagnostics.MissingIncludes != Config::IncludesPolicy::Strict ||
- Cfg.Diagnostics.SuppressAll ||
- Cfg.Diagnostics.Suppress.contains("missing-includes")) {
- return Result;
- }
-
- const SourceManager &SM = AST.getSourceManager();
- const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
-
- auto FileStyle = format::getStyle(
- format::DefaultFormatStyle, AST.tuPath(), format::DefaultFallbackStyle,
- Code, &SM.getFileManager().getVirtualFileSystem());
- if (!FileStyle) {
- elog("Couldn't infer style", FileStyle.takeError());
- FileStyle = format::getLLVMStyle();
- }
-
- tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code,
- FileStyle->IncludeStyle);
- for (const auto &SymbolWithMissingInclude : MissingIncludes) {
- llvm::StringRef ResolvedPath =
- getResolvedPath(SymbolWithMissingInclude.Providers.front());
- if (isFilteredByConfig(Cfg, ResolvedPath)) {
- dlog("IncludeCleaner: not diagnosing missing include {0}, filtered by "
- "config",
- ResolvedPath);
- continue;
- }
-
- std::string Spelling =
- spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front());
- llvm::StringRef HeaderRef{Spelling};
- bool Angled = HeaderRef.starts_with("<");
- // We might suggest insertion of an existing include in edge cases, e.g.,
- // include is present in a PP-disabled region, or spelling of the header
- // turns out to be the same as one of the unresolved includes in the
- // main file.
- std::optional<tooling::Replacement> Replacement = HeaderIncludes.insert(
- HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include);
- if (!Replacement.has_value())
- continue;
-
- Diag &D = Result.emplace_back();
- D.Message =
- llvm::formatv("No header providing \"{0}\" is directly included",
- getSymbolName(SymbolWithMissingInclude.Symbol));
- D.Name = "missing-includes";
- D.Source = Diag::DiagSource::Clangd;
- D.File = AST.tuPath();
- D.InsideMainFile = true;
- D.Severity = DiagnosticsEngine::Warning;
- D.Range = clangd::Range{
- offsetToPosition(Code,
- SymbolWithMissingInclude.SymRefRange.beginOffset()),
- offsetToPosition(Code,
- SymbolWithMissingInclude.SymRefRange.endOffset())};
- auto &F = D.Fixes.emplace_back();
- F.Message = "#include " + Spelling;
- TextEdit Edit = replacementToEdit(Code, *Replacement);
- F.Edits.emplace_back(std::move(Edit));
- }
- return Result;
-}
-
-std::vector<Diag> generateUnusedIncludeDiagnostics(
- PathRef FileName, llvm::ArrayRef<const Inclusion *> UnusedIncludes,
- llvm::StringRef Code) {
- std::vector<Diag> Result;
- const Config &Cfg = Config::current();
- if (Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None ||
- Cfg.Diagnostics.SuppressAll ||
- Cfg.Diagnostics.Suppress.contains("unused-includes")) {
- return Result;
- }
- for (const auto *Inc : UnusedIncludes) {
- Diag &D = Result.emplace_back();
- D.Message =
- llvm::formatv("included header {0} is not used directly",
- llvm::sys::path::filename(
- Inc->Written.substr(1, Inc->Written.size() - 2),
- llvm::sys::path::Style::posix));
- D.Name = "unused-includes";
- D.Source = Diag::DiagSource::Clangd;
- D.File = FileName;
- D.InsideMainFile = true;
- D.Severity = DiagnosticsEngine::Warning;
- D.Tags.push_back(Unnecessary);
- D.Range = getDiagnosticRange(Code, Inc->HashOffset);
- // FIXME(kirillbobyrev): Removing inclusion might break the code if the
- // used headers are only reachable transitively through this one. Suggest
- // including them directly instead.
- // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
- // (keep/export) remove the warning once we support IWYU pragmas.
- auto &F = D.Fixes.emplace_back();
- F.Message = "remove #include directive";
- F.Edits.emplace_back();
- F.Edits.back().range.start.line = Inc->HashLine;
- F.Edits.back().range.end.line = Inc->HashLine + 1;
- }
- return Result;
-}
} // namespace
ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP,
@@ -693,85 +474,105 @@ std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM);
return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas);
}
-
-IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
+std::vector<const Inclusion *>
+computeUnusedIncludesExperimental(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
const auto &Includes = AST.getIncludeStructure();
- include_cleaner::Includes ConvertedIncludes =
- convertIncludes(SM, Includes.MainFileIncludes);
- const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
- std::vector<include_cleaner::SymbolReference> Macros =
- collectMacroReferences(AST);
- std::vector<MissingIncludeDiagInfo> MissingIncludes;
+ // FIXME: !!this is a hacky way to collect macro references.
+ std::vector<include_cleaner::SymbolReference> Macros;
+ auto &PP = AST.getPreprocessor();
+ for (const syntax::Token &Tok :
+ AST.getTokens().spelledTokens(SM.getMainFileID())) {
+ auto Macro = locateMacroAt(Tok, PP);
+ if (!Macro)
+ continue;
+ if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
+ Macros.push_back(
+ {Tok.location(),
+ include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
+ DefLoc},
+ include_cleaner::RefType::Explicit});
+ }
llvm::DenseSet<IncludeStructure::HeaderID> Used;
- trace::Span Tracer("include_cleaner::walkUsed");
include_cleaner::walkUsed(
AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
AST.getPragmaIncludes(), SM,
[&](const include_cleaner::SymbolReference &Ref,
llvm::ArrayRef<include_cleaner::Header> Providers) {
- bool Satisfied = false;
for (const auto &H : Providers) {
- if (H.kind() == include_cleaner::Header::Physical &&
- H.physical() == MainFile) {
- Satisfied = true;
- continue;
- }
- for (auto *Inc : ConvertedIncludes.match(H)) {
- Satisfied = true;
- auto HeaderID = Includes.getID(Inc->Resolved);
- assert(HeaderID.has_value() &&
- "ConvertedIncludes only contains resolved includes.");
- Used.insert(*HeaderID);
+ switch (H.kind()) {
+ case include_cleaner::Header::Physical:
+ if (auto HeaderID = Includes.getID(H.physical()))
+ Used.insert(*HeaderID);
+ break;
+ case include_cleaner::Header::Standard:
+ for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard()))
+ Used.insert(HeaderID);
+ break;
+ case include_cleaner::Header::Verbatim:
+ for (auto *Inc :
+ Includes.mainFileIncludesWithSpelling(H.verbatim())) {
+ if (!Inc->HeaderID.has_value())
+ continue;
+ IncludeStructure::HeaderID ID =
+ static_cast<IncludeStructure::HeaderID>(*Inc->HeaderID);
+ Used.insert(ID);
+ }
+ break;
}
}
-
- if (Satisfied || Providers.empty() ||
- Ref.RT != include_cleaner::RefType::Explicit)
- return;
-
- auto &Tokens = AST.getTokens();
- auto SpelledForExpanded =
- Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation));
- if (!SpelledForExpanded)
- return;
-
- auto Range = syntax::Token::range(SM, SpelledForExpanded->front(),
- SpelledForExpanded->back());
- MissingIncludeDiagInfo DiagInfo{Ref.Target, Range, Providers};
- MissingIncludes.push_back(std::move(DiagInfo));
});
- std::vector<const Inclusion *> UnusedIncludes =
- getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
- return {std::move(UnusedIncludes), std::move(MissingIncludes)};
+ return getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
}
-std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
llvm::StringRef Code) {
+ const Config &Cfg = Config::current();
+ if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None ||
+ Cfg.Diagnostics.SuppressAll ||
+ Cfg.Diagnostics.Suppress.contains("unused-includes"))
+ return {};
// Interaction is only polished for C/CPP.
if (AST.getLangOpts().ObjC)
return {};
-
- trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
-
- const Config &Cfg = Config::current();
- IncludeCleanerFindings Findings;
- if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict ||
- Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) {
- // will need include-cleaner results, call it once
- Findings = computeIncludeCleanerFindings(AST);
+ trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
+ std::vector<Diag> Result;
+ std::string FileName =
+ AST.getSourceManager()
+ .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
+ ->getName()
+ .str();
+ const auto &UnusedIncludes =
+ Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment
+ ? computeUnusedIncludesExperimental(AST)
+ : computeUnusedIncludes(AST);
+ for (const auto *Inc : UnusedIncludes) {
+ Diag D;
+ D.Message =
+ llvm::formatv("included header {0} is not used directly",
+ llvm::sys::path::filename(
+ Inc->Written.substr(1, Inc->Written.size() - 2),
+ llvm::sys::path::Style::posix));
+ D.Name = "unused-includes";
+ D.Source = Diag::DiagSource::Clangd;
+ D.File = FileName;
+ D.Severity = DiagnosticsEngine::Warning;
+ D.Tags.push_back(Unnecessary);
+ D.Range = getDiagnosticRange(Code, Inc->HashOffset);
+ // FIXME(kirillbobyrev): Removing inclusion might break the code if the
+ // used headers are only reachable transitively through this one. Suggest
+ // including them directly instead.
+ // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
+ // (keep/export) remove the warning once we support IWYU pragmas.
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "remove #include directive";
+ D.Fixes.back().Edits.emplace_back();
+ D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
+ D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+ D.InsideMainFile = true;
+ Result.push_back(std::move(D));
}
-
- std::vector<Diag> Result = generateUnusedIncludeDiagnostics(
- AST.tuPath(),
- Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
- ? computeUnusedIncludes(AST)
- : Findings.UnusedIncludes,
- Code);
- llvm::move(
- generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code),
- std::back_inserter(Result));
return Result;
}
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index 989067e3d84f..a7beb9c3c9d4 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -20,38 +20,18 @@
#include "Headers.h"
#include "ParsedAST.h"
-#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/StringSet.h"
#include <optional>
-#include <tuple>
#include <vector>
namespace clang {
namespace clangd {
-// Data needed for missing include diagnostics.
-struct MissingIncludeDiagInfo {
- include_cleaner::Symbol Symbol;
- syntax::FileRange SymRefRange;
- std::vector<include_cleaner::Header> Providers;
-
- bool operator==(const MissingIncludeDiagInfo &Other) const {
- return std::tie(SymRefRange, Providers, Symbol) ==
- std::tie(Other.SymRefRange, Other.Providers, Other.Symbol);
- }
-};
-
-struct IncludeCleanerFindings {
- std::vector<const Inclusion *> UnusedIncludes;
- std::vector<MissingIncludeDiagInfo> MissingIncludes;
-};
-
struct ReferencedLocations {
llvm::DenseSet<SourceLocation> User;
llvm::DenseSet<tooling::stdlib::Symbol> Stdlib;
@@ -116,10 +96,13 @@ getUnused(ParsedAST &AST,
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
const llvm::StringSet<> &ReferencedPublicHeaders);
-IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST);
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
+// The same as computeUnusedIncludes, but it is an experimental and
+// include-cleaner-lib-based implementation.
+std::vector<const Inclusion *>
+computeUnusedIncludesExperimental(ParsedAST &AST);
-std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
+std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
llvm::StringRef Code);
/// Affects whether standard library includes should be considered for
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 640b112ebe71..54a0f979c730 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -687,9 +687,13 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::move(Macros), std::move(Marks), std::move(ParsedDecls),
std::move(Diags), std::move(Includes),
std::move(CanonIncludes));
- if (Result.Diags)
- llvm::move(issueIncludeCleanerDiagnostics(Result, Inputs.Contents),
- std::back_inserter(*Result.Diags));
+ if (Result.Diags) {
+ auto UnusedHeadersDiags =
+ issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
+ Result.Diags->insert(Result.Diags->end(),
+ make_move_iterator(UnusedHeadersDiags.begin()),
+ make_move_iterator(UnusedHeadersDiags.end()));
+ }
return std::move(Result);
}
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index c4aec747acc0..fceea309710b 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -128,9 +128,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
if (Config::current().Diagnostics.UnusedIncludes ==
- Config::IncludesPolicy::Experiment ||
- Config::current().Diagnostics.MissingIncludes ==
- Config::IncludesPolicy::Strict)
+ Config::UnusedIncludesPolicy::Experiment)
Pragmas.record(CI);
if (BeforeExecuteCallback)
BeforeExecuteCallback(CI);
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 45dacba5822b..014db131c622 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -249,19 +249,19 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
// Defaults to None.
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::None);
+ Config::UnusedIncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("None");
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::None);
+ Config::UnusedIncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("Strict");
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::Strict);
+ Config::UnusedIncludesPolicy::Strict);
Frag = {};
EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 289767b0dec0..a20be39722ec 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1900,7 +1900,7 @@ TEST(DiagnosticsTest, IncludeCleaner) {
// Off by default.
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
Config Cfg;
- Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
// Set filtering.
Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back(
[](llvm::StringRef Header) { return Header.endswith("ignore.h"); });
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 464c43ea48ef..01b8e6a05dab 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,58 +8,26 @@
#include "Annotations.h"
#include "Config.h"
-#include "Diagnostics.h"
#include "IncludeCleaner.h"
-#include "ParsedAST.h"
#include "SourceCode.h"
#include "TestFS.h"
#include "TestTU.h"
-#include "clang-include-cleaner/Analysis.h"
-#include "clang-include-cleaner/Types.h"
#include "support/Context.h"
-#include "clang/AST/DeclBase.h"
-#include "clang/Basic/SourceManager.h"
-#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
-#include <cstddef>
-#include <string>
-#include <vector>
namespace clang {
namespace clangd {
namespace {
-using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
using ::testing::IsEmpty;
-using ::testing::Matcher;
using ::testing::Pointee;
using ::testing::UnorderedElementsAre;
-Matcher<const Diag &> withFix(::testing::Matcher<Fix> FixMatcher) {
- return Field(&Diag::Fixes, ElementsAre(FixMatcher));
-}
-
-MATCHER_P2(Diag, Range, Message,
- "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
- return arg.Range == Range && arg.Message == Message;
-}
-
-MATCHER_P3(Fix, Range, Replacement, Message,
- "Fix " + llvm::to_string(Range) + " => " +
- ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
- return arg.Message == Message && arg.Edits.size() == 1 &&
- arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
-}
-
std::string guard(llvm::StringRef Code) {
return "#pragma once\n" + Code.str();
}
@@ -374,8 +342,7 @@ TEST(IncludeCleaner, StdlibUnused) {
auto AST = TU.build();
EXPECT_THAT(computeUnusedIncludes(AST),
ElementsAre(Pointee(writtenInclusion("<queue>"))));
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- EXPECT_THAT(Findings.UnusedIncludes,
+ EXPECT_THAT(computeUnusedIncludesExperimental(AST),
ElementsAre(Pointee(writtenInclusion("<queue>"))));
}
@@ -412,138 +379,12 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
computeUnusedIncludes(AST),
UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
Pointee(writtenInclusion("\"dir/unused.h\""))));
-
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(
- Findings.UnusedIncludes,
+ computeUnusedIncludesExperimental(AST),
UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
Pointee(writtenInclusion("\"dir/unused.h\""))));
}
-TEST(IncludeCleaner, ComputeMissingHeaders) {
- Annotations MainFile(R"cpp(
- #include "a.h"
-
- void foo() {
- $b[[b]]();
- })cpp");
- TestTU TU;
- TU.Filename = "foo.cpp";
- TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
- TU.AdditionalFiles["b.h"] = guard("void b();");
-
- TU.Code = MainFile.code();
- ParsedAST AST = TU.build();
-
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- const SourceManager &SM = AST.getSourceManager();
- const NamedDecl *BDecl = nullptr;
- for (Decl *D : AST.getASTContext().getTranslationUnitDecl()->decls()) {
- const NamedDecl *CandidateDecl = llvm::dyn_cast<NamedDecl>(D);
- std::string Name = CandidateDecl->getQualifiedNameAsString();
- if (Name != "b")
- continue;
- BDecl = CandidateDecl;
- }
- ASSERT_TRUE(BDecl);
- include_cleaner::Symbol B{*BDecl};
- auto Range = MainFile.range("b");
- size_t Start = llvm::cantFail(positionToOffset(MainFile.code(), Range.start));
- size_t End = llvm::cantFail(positionToOffset(MainFile.code(), Range.end));
- syntax::FileRange BRange{SM.getMainFileID(), static_cast<unsigned int>(Start),
- static_cast<unsigned int>(End)};
- include_cleaner::Header Header{*SM.getFileManager().getFile("b.h")};
- MissingIncludeDiagInfo BInfo{B, BRange, {Header}};
- EXPECT_THAT(Findings.MissingIncludes, ElementsAre(BInfo));
-}
-
-TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
- Config Cfg;
- Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
- Cfg.Diagnostics.Includes.IgnoreHeader = {
- [](llvm::StringRef Header) { return Header == testPath("buzz.h"); }};
- WithContextValue Ctx(Config::Key, std::move(Cfg));
- Annotations MainFile(R"cpp(
-#include "a.h"
-$insert_b[[]]#include "baz.h"
-#include "dir/c.h"
-$insert_d[[]]#include "fuzz.h"
-#include "header.h"
-$insert_foobar[[]]#include <e.h>
-$insert_f[[]]$insert_vector[[]]
-
- void foo() {
- $b[[b]]();
-
- ns::$bar[[Bar]] bar;
- bar.d();
- $f[[f]]();
-
- // this should not be diagnosed, because it's ignored in the config
- buzz();
-
- $foobar[[foobar]]();
-
- std::$vector[[vector]] v;
- })cpp");
-
- TestTU TU;
- TU.Filename = "foo.cpp";
- TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
- TU.AdditionalFiles["b.h"] = guard("void b();");
-
- TU.AdditionalFiles["dir/c.h"] = guard("#include \"d.h\"");
- TU.AdditionalFiles["dir/d.h"] =
- guard("namespace ns { struct Bar { void d(); }; }");
-
- TU.AdditionalFiles["system/e.h"] = guard("#include <f.h>");
- TU.AdditionalFiles["system/f.h"] = guard("void f();");
- TU.ExtraArgs.push_back("-isystem" + testPath("system"));
-
- TU.AdditionalFiles["fuzz.h"] = guard("#include \"buzz.h\"");
- TU.AdditionalFiles["buzz.h"] = guard("void buzz();");
-
- TU.AdditionalFiles["baz.h"] = guard("#include \"private.h\"");
- TU.AdditionalFiles["private.h"] = guard(R"cpp(
- // IWYU pragma: private, include "public.h"
- void foobar();
- )cpp");
- TU.AdditionalFiles["header.h"] = guard(R"cpp(
- namespace std { class vector {}; }
- )cpp");
-
- TU.Code = MainFile.code();
- ParsedAST AST = TU.build();
-
- std::vector<clangd::Diag> Diags =
- issueIncludeCleanerDiagnostics(AST, TU.Code);
- EXPECT_THAT(
- Diags,
- UnorderedElementsAre(
- AllOf(Diag(MainFile.range("b"),
- "No header providing \"b\" is directly included"),
- withFix(Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
- "#include \"b.h\""))),
- AllOf(Diag(MainFile.range("bar"),
- "No header providing \"ns::Bar\" is directly included"),
- withFix(Fix(MainFile.range("insert_d"),
- "#include \"dir/d.h\"\n", "#include \"dir/d.h\""))),
- AllOf(Diag(MainFile.range("f"),
- "No header providing \"f\" is directly included"),
- withFix(Fix(MainFile.range("insert_f"), "#include <f.h>\n",
- "#include <f.h>"))),
- AllOf(
- Diag(MainFile.range("foobar"),
- "No header providing \"foobar\" is directly included"),
- withFix(Fix(MainFile.range("insert_foobar"),
- "#include \"public.h\"\n", "#include \"public.h\""))),
- AllOf(
- Diag(MainFile.range("vector"),
- "No header providing \"std::vector\" is directly included"),
- withFix(Fix(MainFile.range("insert_vector"),
- "#include <vector>\n", "#include <vector>")))));
-}
-
TEST(IncludeCleaner, VirtualBuffers) {
TestTU TU;
TU.Code = R"cpp(
@@ -697,7 +538,7 @@ TEST(IncludeCleaner, IWYUPragmas) {
void foo() {}
)cpp");
Config Cfg;
- Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Experiment;
+ Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Experiment;
WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
@@ -713,8 +554,7 @@ TEST(IncludeCleaner, IWYUPragmas) {
ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
}
TEST(IncludeCleaner, RecursiveInclusion) {
@@ -743,8 +583,7 @@ TEST(IncludeCleaner, RecursiveInclusion) {
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
}
TEST(IncludeCleaner, IWYUPragmaExport) {
@@ -769,8 +608,7 @@ TEST(IncludeCleaner, IWYUPragmaExport) {
// FIXME: This is not correct: foo.h is unused but is not diagnosed as such
// because we ignore headers with IWYU export pragmas for now.
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
}
TEST(IncludeCleaner, NoDiagsForObjC) {
@@ -789,9 +627,7 @@ TEST(IncludeCleaner, NoDiagsForObjC) {
TU.ExtraArgs.emplace_back("-xobjective-c");
Config Cfg;
-
- Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
- Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index bee22c8901ea..94870abf1569 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@ TEST(PreamblePatch, DiagnosticsFromMainASTAreInRightPlace) {
TEST(PreamblePatch, DiagnosticsToPreamble) {
Config Cfg;
Cfg.Diagnostics.AllowStalePreamble = true;
- Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
llvm::StringMap<std::string> AdditionalFiles;
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 cd1170054807..f6afaff09cfd 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
@@ -73,8 +73,6 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
const format::FormatStyle &IncludeStyle);
-std::string spellHeader(const Header &H, HeaderSearch &HS,
- const FileEntry *Main);
} // 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 6237bdb46bab..c5559db57e14 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
}
}
-std::string spellHeader(const Header &H, HeaderSearch &HS,
- const FileEntry *Main) {
+static std::string spellHeader(const Header &H, HeaderSearch &HS,
+ const FileEntry *Main) {
switch (H.kind()) {
case Header::Physical: {
bool IsSystem = false;
More information about the cfe-commits
mailing list