[clangd] Full support for #import insertions
These are still disabled by default, but will work in ObjC code if you enable the `-import-insertions` flag. Completion requires ASTSignals to be available; before ASTSignals are available, we will always use #include. Once they are available, the behavior varies as follows: - For source files, use #import if the ObjC language flag is enabled - For header files: - If the ObjC language flag is disabled, use #include - If the header file contains any #imports, use #import - If the header file references any ObjC decls, use #import - Otherwise, use #include IncludeFixer support is similar, but it does not rely upon ASTSignals, instead it does the above checks excluding the scan for ObjC symbols. Differential Revision: https://reviews.llvm.org/D139458
This commit is contained in:
parent
814c0bb316
commit
042dd99484
16 changed files with 154 additions and 16 deletions
|
|
@ -376,6 +376,38 @@ const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D) {
|
|||
return nullptr;
|
||||
}
|
||||
|
||||
Symbol::IncludeDirective
|
||||
preferredIncludeDirective(llvm::StringRef FileName, const LangOptions &LangOpts,
|
||||
ArrayRef<Inclusion> MainFileIncludes,
|
||||
ArrayRef<const Decl *> TopLevelDecls) {
|
||||
// Always prefer #include for non-ObjC code.
|
||||
if (!LangOpts.ObjC)
|
||||
return Symbol::IncludeDirective::Include;
|
||||
// If this is not a header file and has ObjC set as the language, prefer
|
||||
// #import.
|
||||
if (!isHeaderFile(FileName, LangOpts))
|
||||
return Symbol::IncludeDirective::Import;
|
||||
|
||||
// Headers lack proper compile flags most of the time, so we might treat a
|
||||
// header as ObjC accidentally. Perform some extra checks to make sure this
|
||||
// works.
|
||||
|
||||
// Any file with a #import, should keep #import-ing.
|
||||
for (auto &Inc : MainFileIncludes)
|
||||
if (Inc.Directive == tok::pp_import)
|
||||
return Symbol::IncludeDirective::Import;
|
||||
|
||||
// Any file declaring an ObjC decl should also be #import-ing.
|
||||
// No need to look over the references, as the file doesn't have any #imports,
|
||||
// it must be declaring interesting ObjC-like decls.
|
||||
for (const Decl *D : TopLevelDecls)
|
||||
if (isa<ObjCContainerDecl, ObjCIvarDecl, ObjCMethodDecl, ObjCPropertyDecl>(
|
||||
D))
|
||||
return Symbol::IncludeDirective::Import;
|
||||
|
||||
return Symbol::IncludeDirective::Include;
|
||||
}
|
||||
|
||||
std::string printType(const QualType QT, const DeclContext &CurContext,
|
||||
const llvm::StringRef Placeholder) {
|
||||
std::string Result;
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@
|
|||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
|
||||
|
||||
#include "Headers.h"
|
||||
#include "index/Symbol.h"
|
||||
#include "index/SymbolID.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
#include "clang/AST/DeclObjC.h"
|
||||
|
|
@ -118,6 +120,18 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
|
|||
/// return nullptr as protocols don't have an implementation.
|
||||
const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D);
|
||||
|
||||
/// Infer the include directive to use for the given \p FileName. It aims for
|
||||
/// #import for ObjC files and #include for the rest.
|
||||
///
|
||||
/// - For source files we use LangOpts directly to infer ObjC-ness.
|
||||
/// - For header files we also check for symbols declared by the file and
|
||||
/// existing include directives, as the language can be set to ObjC++ as a
|
||||
/// fallback in the absence of compile flags.
|
||||
Symbol::IncludeDirective
|
||||
preferredIncludeDirective(llvm::StringRef FileName, const LangOptions &LangOpts,
|
||||
ArrayRef<Inclusion> MainFileIncludes,
|
||||
ArrayRef<const Decl *> TopLevelDecls);
|
||||
|
||||
/// Returns a QualType as string. The result doesn't contain unwritten scopes
|
||||
/// like anonymous/inline namespace.
|
||||
std::string printType(const QualType QT, const DeclContext &CurContext,
|
||||
|
|
|
|||
|
|
@ -9,13 +9,18 @@
|
|||
#include "ASTSignals.h"
|
||||
#include "AST.h"
|
||||
#include "FindTarget.h"
|
||||
#include "Headers.h"
|
||||
#include "support/Trace.h"
|
||||
#include "clang/AST/DeclObjC.h"
|
||||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
ASTSignals ASTSignals::derive(const ParsedAST &AST) {
|
||||
trace::Span Span("ASTSignals::derive");
|
||||
ASTSignals Signals;
|
||||
Signals.InsertionDirective = preferredIncludeDirective(
|
||||
AST.tuPath(), AST.getLangOpts(),
|
||||
AST.getIncludeStructure().MainFileIncludes, AST.getLocalTopLevelDecls());
|
||||
const SourceManager &SM = AST.getSourceManager();
|
||||
findExplicitReferences(
|
||||
AST.getASTContext(),
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_ASTSIGNALS_H
|
||||
|
||||
#include "ParsedAST.h"
|
||||
#include "index/Symbol.h"
|
||||
#include "index/SymbolID.h"
|
||||
#include "llvm/ADT/DenseMap.h"
|
||||
#include "llvm/ADT/StringMap.h"
|
||||
|
|
@ -29,6 +30,9 @@ struct ASTSignals {
|
|||
/// Namespaces whose symbols are used in the file, and the number of such
|
||||
/// distinct symbols.
|
||||
llvm::StringMap<unsigned> RelatedNamespaces;
|
||||
/// Preferred preprocessor directive to use for inclusions by the file.
|
||||
Symbol::IncludeDirective InsertionDirective =
|
||||
Symbol::IncludeDirective::Include;
|
||||
|
||||
static ASTSignals derive(const ParsedAST &AST);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -188,6 +188,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
|
|||
UseDirtyHeaders(Opts.UseDirtyHeaders),
|
||||
LineFoldingOnly(Opts.LineFoldingOnly),
|
||||
PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions),
|
||||
ImportInsertions(Opts.ImportInsertions),
|
||||
WorkspaceRoot(Opts.WorkspaceRoot),
|
||||
Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
|
||||
: TUScheduler::NoInvalidation),
|
||||
|
|
@ -261,6 +262,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
|
|||
std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents);
|
||||
ParseOptions Opts;
|
||||
Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
|
||||
Opts.ImportInsertions = ImportInsertions;
|
||||
|
||||
// Compile command is set asynchronously during update, as it can be slow.
|
||||
ParseInputs Inputs;
|
||||
|
|
|
|||
|
|
@ -171,6 +171,10 @@ public:
|
|||
// If true, parse emplace-like functions in the preamble.
|
||||
bool PreambleParseForwardingFunctions = false;
|
||||
|
||||
/// Whether include fixer insertions for Objective-C code should use #import
|
||||
/// instead of #include.
|
||||
bool ImportInsertions = false;
|
||||
|
||||
explicit operator TUScheduler::Options() const;
|
||||
};
|
||||
// Sensible default options for use in tests.
|
||||
|
|
@ -434,6 +438,8 @@ private:
|
|||
|
||||
bool PreambleParseForwardingFunctions = false;
|
||||
|
||||
bool ImportInsertions = false;
|
||||
|
||||
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
|
||||
llvm::StringMap<std::optional<FuzzyFindRequest>>
|
||||
CachedCompletionFuzzyFindRequestByFile;
|
||||
|
|
|
|||
|
|
@ -187,6 +187,12 @@ MarkupContent renderDoc(const markup::Document &Doc, MarkupKind Kind) {
|
|||
return Result;
|
||||
}
|
||||
|
||||
Symbol::IncludeDirective insertionDirective(const CodeCompleteOptions &Opts) {
|
||||
if (!Opts.ImportInsertions || !Opts.MainFileSignals)
|
||||
return Symbol::IncludeDirective::Include;
|
||||
return Opts.MainFileSignals->InsertionDirective;
|
||||
}
|
||||
|
||||
// Identifier code completion result.
|
||||
struct RawIdentifier {
|
||||
llvm::StringRef Name;
|
||||
|
|
@ -277,9 +283,9 @@ struct CompletionCandidate {
|
|||
if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc())))
|
||||
return std::nullopt;
|
||||
}
|
||||
Symbol::IncludeDirective Directive = insertionDirective(Opts);
|
||||
for (const auto &Inc : RankedIncludeHeaders)
|
||||
// FIXME: We should support #import directives here.
|
||||
if ((Inc.Directive & clang::clangd::Symbol::Include) != 0)
|
||||
if ((Inc.Directive & Directive) != 0)
|
||||
return Inc.Header;
|
||||
return std::nullopt;
|
||||
}
|
||||
|
|
@ -395,10 +401,10 @@ struct CodeCompletionBuilder {
|
|||
Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
|
||||
};
|
||||
bool ShouldInsert = C.headerToInsertIfAllowed(Opts).has_value();
|
||||
Symbol::IncludeDirective Directive = insertionDirective(Opts);
|
||||
// Calculate include paths and edits for all possible headers.
|
||||
for (const auto &Inc : C.RankedIncludeHeaders) {
|
||||
// FIXME: We should support #import directives here.
|
||||
if ((Inc.Directive & clang::clangd::Symbol::Include) == 0)
|
||||
if ((Inc.Directive & Directive) == 0)
|
||||
continue;
|
||||
|
||||
if (auto ToInclude = Inserted(Inc.Header)) {
|
||||
|
|
@ -406,7 +412,9 @@ struct CodeCompletionBuilder {
|
|||
Include.Header = ToInclude->first;
|
||||
if (ToInclude->second && ShouldInsert)
|
||||
Include.Insertion = Includes.insert(
|
||||
ToInclude->first, tooling::IncludeDirective::Include);
|
||||
ToInclude->first, Directive == Symbol::Import
|
||||
? tooling::IncludeDirective::Import
|
||||
: tooling::IncludeDirective::Include);
|
||||
Completion.Includes.push_back(std::move(Include));
|
||||
} else
|
||||
log("Failed to generate include insertion edits for adding header "
|
||||
|
|
|
|||
|
|
@ -40,6 +40,8 @@ public:
|
|||
// Options to run clang e.g. when parsing AST.
|
||||
struct ParseOptions {
|
||||
bool PreambleParseForwardingFunctions = false;
|
||||
|
||||
bool ImportInsertions = false;
|
||||
};
|
||||
|
||||
/// Information required to run clang, e.g. to parse AST or do code completion.
|
||||
|
|
|
|||
|
|
@ -320,8 +320,7 @@ std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
|
|||
llvm::StringSet<> InsertedHeaders;
|
||||
for (const auto &Sym : Syms) {
|
||||
for (const auto &Inc : getRankedIncludes(Sym)) {
|
||||
// FIXME: We should support #import directives here.
|
||||
if ((Inc.Directive & clang::clangd::Symbol::Include) == 0)
|
||||
if ((Inc.Directive & Directive) == 0)
|
||||
continue;
|
||||
if (auto ToInclude = Inserted(Sym, Inc.Header)) {
|
||||
if (ToInclude->second) {
|
||||
|
|
@ -329,7 +328,9 @@ std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
|
|||
continue;
|
||||
if (auto Fix =
|
||||
insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str(),
|
||||
tooling::IncludeDirective::Include))
|
||||
Directive == Symbol::Import
|
||||
? tooling::IncludeDirective::Import
|
||||
: tooling::IncludeDirective::Include))
|
||||
Fixes.push_back(std::move(*Fix));
|
||||
}
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -34,9 +34,10 @@ namespace clangd {
|
|||
class IncludeFixer {
|
||||
public:
|
||||
IncludeFixer(llvm::StringRef File, std::shared_ptr<IncludeInserter> Inserter,
|
||||
const SymbolIndex &Index, unsigned IndexRequestLimit)
|
||||
const SymbolIndex &Index, unsigned IndexRequestLimit,
|
||||
Symbol::IncludeDirective Directive)
|
||||
: File(File), Inserter(std::move(Inserter)), Index(Index),
|
||||
IndexRequestLimit(IndexRequestLimit) {}
|
||||
IndexRequestLimit(IndexRequestLimit), Directive(Directive) {}
|
||||
|
||||
/// Returns include insertions that can potentially recover the diagnostic.
|
||||
/// If Info is a note and fixes are returned, they should *replace* the note.
|
||||
|
|
@ -80,6 +81,7 @@ private:
|
|||
const SymbolIndex &Index;
|
||||
const unsigned IndexRequestLimit; // Make at most 5 index requests.
|
||||
mutable unsigned IndexRequestCount = 0;
|
||||
const Symbol::IncludeDirective Directive;
|
||||
|
||||
// These collect the last unresolved name so that we can associate it with the
|
||||
// diagnostic.
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@
|
|||
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
|
||||
#include "../clang-tidy/ClangTidyModuleRegistry.h"
|
||||
#include "AST.h"
|
||||
#include "ASTSignals.h"
|
||||
#include "Compiler.h"
|
||||
#include "Config.h"
|
||||
#include "Diagnostics.h"
|
||||
|
|
@ -25,6 +26,7 @@
|
|||
#include "TidyProvider.h"
|
||||
#include "index/CanonicalIncludes.h"
|
||||
#include "index/Index.h"
|
||||
#include "index/Symbol.h"
|
||||
#include "support/Logger.h"
|
||||
#include "support/Trace.h"
|
||||
#include "clang/AST/ASTContext.h"
|
||||
|
|
@ -560,12 +562,21 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
|
|||
auto Inserter = std::make_shared<IncludeInserter>(
|
||||
Filename, Inputs.Contents, Style, BuildDir.get(),
|
||||
&Clang->getPreprocessor().getHeaderSearchInfo());
|
||||
ArrayRef<Inclusion> MainFileIncludes;
|
||||
if (Preamble) {
|
||||
MainFileIncludes = Preamble->Includes.MainFileIncludes;
|
||||
for (const auto &Inc : Preamble->Includes.MainFileIncludes)
|
||||
Inserter->addExisting(Inc);
|
||||
}
|
||||
// FIXME: Consider piping through ASTSignals to fetch this to handle the
|
||||
// case where a header file contains ObjC decls but no #imports.
|
||||
Symbol::IncludeDirective Directive =
|
||||
Inputs.Opts.ImportInsertions
|
||||
? preferredIncludeDirective(Filename, Clang->getLangOpts(),
|
||||
MainFileIncludes, {})
|
||||
: Symbol::Include;
|
||||
FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
|
||||
/*IndexRequestLimit=*/5);
|
||||
/*IndexRequestLimit=*/5, Directive);
|
||||
ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
|
||||
const clang::Diagnostic &Info) {
|
||||
return FixIncludes->fix(DiagLevl, Info);
|
||||
|
|
@ -720,6 +731,10 @@ llvm::ArrayRef<Decl *> ParsedAST::getLocalTopLevelDecls() {
|
|||
return LocalTopLevelDecls;
|
||||
}
|
||||
|
||||
llvm::ArrayRef<const Decl *> ParsedAST::getLocalTopLevelDecls() const {
|
||||
return LocalTopLevelDecls;
|
||||
}
|
||||
|
||||
const MainFileMacros &ParsedAST::getMacros() const { return Macros; }
|
||||
const std::vector<PragmaMark> &ParsedAST::getMarks() const { return Marks; }
|
||||
|
||||
|
|
|
|||
|
|
@ -86,6 +86,7 @@ public:
|
|||
/// The result does not include the decls that come from the preamble.
|
||||
/// (These should be const, but RecursiveASTVisitor requires Decl*).
|
||||
ArrayRef<Decl *> getLocalTopLevelDecls();
|
||||
ArrayRef<const Decl *> getLocalTopLevelDecls() const;
|
||||
|
||||
const std::optional<std::vector<Diag>> &getDiagnostics() const {
|
||||
return Diags;
|
||||
|
|
|
|||
|
|
@ -969,6 +969,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
|
|||
}
|
||||
Opts.UseDirtyHeaders = UseDirtyHeaders;
|
||||
Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
|
||||
Opts.ImportInsertions = ImportInsertions;
|
||||
Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
|
||||
Opts.TweakFilter = [&](const Tweak &T) {
|
||||
if (T.hidden() && !HiddenFeatures)
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ TEST(ASTSignals, Derive) {
|
|||
Pair(sym("Y", index::SymbolKind::Variable, "@N@tar@S@X@FI@\\0").ID,
|
||||
2),
|
||||
Pair(_ /*a*/, 3)));
|
||||
EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Include);
|
||||
}
|
||||
} // namespace
|
||||
} // namespace clangd
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@
|
|||
#include "Annotations.h"
|
||||
#include "ParsedAST.h"
|
||||
#include "TestTU.h"
|
||||
#include "index/Symbol.h"
|
||||
#include "clang/AST/ASTTypeTraits.h"
|
||||
#include "clang/AST/Attr.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
|
|
@ -618,6 +619,45 @@ TEST(ClangdAST, HasReservedName) {
|
|||
hasReservedScope(*findUnqualifiedDecl(AST, "secret").getDeclContext()));
|
||||
}
|
||||
|
||||
TEST(ClangdAST, PreferredIncludeDirective) {
|
||||
auto ComputePreferredDirective = [](TestTU &TU) {
|
||||
auto AST = TU.build();
|
||||
return preferredIncludeDirective(AST.tuPath(), AST.getLangOpts(),
|
||||
AST.getIncludeStructure().MainFileIncludes,
|
||||
AST.getLocalTopLevelDecls());
|
||||
};
|
||||
TestTU ObjCTU = TestTU::withCode(R"cpp(
|
||||
int main() {}
|
||||
)cpp");
|
||||
ObjCTU.Filename = "TestTU.m";
|
||||
EXPECT_EQ(ComputePreferredDirective(ObjCTU),
|
||||
Symbol::IncludeDirective::Import);
|
||||
|
||||
TestTU HeaderTU = TestTU::withCode(R"cpp(
|
||||
#import "TestTU.h"
|
||||
)cpp");
|
||||
HeaderTU.Filename = "TestTUHeader.h";
|
||||
HeaderTU.ExtraArgs = {"-xobjective-c++-header"};
|
||||
EXPECT_EQ(ComputePreferredDirective(HeaderTU),
|
||||
Symbol::IncludeDirective::Import);
|
||||
|
||||
// ObjC language option is not enough for headers.
|
||||
HeaderTU.Code = R"cpp(
|
||||
#include "TestTU.h"
|
||||
)cpp";
|
||||
EXPECT_EQ(ComputePreferredDirective(HeaderTU),
|
||||
Symbol::IncludeDirective::Include);
|
||||
|
||||
HeaderTU.Code = R"cpp(
|
||||
@interface Foo
|
||||
@end
|
||||
|
||||
Foo * getFoo();
|
||||
)cpp";
|
||||
EXPECT_EQ(ComputePreferredDirective(HeaderTU),
|
||||
Symbol::IncludeDirective::Import);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace clangd
|
||||
} // namespace clang
|
||||
|
|
|
|||
|
|
@ -2736,17 +2736,21 @@ TEST(CompletionTest, InsertIncludeOrImport) {
|
|||
Sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
|
||||
Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000,
|
||||
Symbol::Include | Symbol::Import);
|
||||
|
||||
auto Results = completions("Fun^", {Sym}).Completions;
|
||||
CodeCompleteOptions Opts;
|
||||
// Should only take effect in import contexts.
|
||||
Opts.ImportInsertions = true;
|
||||
auto Results = completions("Fun^", {Sym}, Opts).Completions;
|
||||
assert(!Results.empty());
|
||||
EXPECT_THAT(Results[0],
|
||||
AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n")));
|
||||
|
||||
Results = completions("Fun^", {Sym}, {}, "Foo.m").Completions;
|
||||
ASTSignals Signals;
|
||||
Signals.InsertionDirective = Symbol::IncludeDirective::Import;
|
||||
Opts.MainFileSignals = &Signals;
|
||||
Results = completions("Fun^", {Sym}, Opts, "Foo.m").Completions;
|
||||
assert(!Results.empty());
|
||||
// TODO: Once #import integration support is done this should be #import.
|
||||
EXPECT_THAT(Results[0],
|
||||
AllOf(named("Func"), insertIncludeText("#include \"bar.h\"\n")));
|
||||
AllOf(named("Func"), insertIncludeText("#import \"bar.h\"\n")));
|
||||
|
||||
Sym.IncludeHeaders[0].SupportedDirectives = Symbol::Import;
|
||||
Results = completions("Fun^", {Sym}).Completions;
|
||||
|
|
|
|||
Loading…
Reference in a new issue