Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -60,32 +60,6 @@ static URISchemeRegistry::Add X("test", "Test scheme for clangd lit tests."); -TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) { - Range ReplacementRange = { - offsetToPosition(Code, R.getOffset()), - offsetToPosition(Code, R.getOffset() + R.getLength())}; - return {ReplacementRange, R.getReplacementText()}; -} - -std::vector -replacementsToEdits(StringRef Code, - const std::vector &Replacements) { - // Turn the replacements into the format specified by the Language Server - // Protocol. Fuse them into one big JSON array. - std::vector Edits; - for (const auto &R : Replacements) - Edits.push_back(replacementToEdit(Code, R)); - return Edits; -} - -std::vector replacementsToEdits(StringRef Code, - const tooling::Replacements &Repls) { - std::vector Edits; - for (const auto &R : Repls) - Edits.push_back(replacementToEdit(Code, R)); - return Edits; -} - SymbolKindBitset defaultSymbolKinds() { SymbolKindBitset Defaults; for (size_t I = SymbolKindMin; I <= static_cast(SymbolKind::Array); @@ -141,9 +115,7 @@ {"workspaceSymbolProvider", true}, {"executeCommandProvider", json::obj{ - {"commands", - {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE}}, + {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, }}, }}}}); } @@ -216,42 +188,6 @@ reply("Fix applied."); ApplyEdit(*Params.workspaceEdit); - } else if (Params.command == - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) { - auto &FileURI = Params.includeInsertion->textDocument.uri; - auto Code = DraftMgr.getDraft(FileURI.file()); - if (!Code) - return replyError(ErrorCode::InvalidParams, - ("command " + - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE + - " called on non-added file " + FileURI.file()) - .str()); - llvm::StringRef DeclaringHeader = Params.includeInsertion->declaringHeader; - if (DeclaringHeader.empty()) - return replyError( - ErrorCode::InvalidParams, - "declaringHeader must be provided for include insertion."); - llvm::StringRef PreferredHeader = Params.includeInsertion->preferredHeader; - auto Replaces = Server.insertInclude( - FileURI.file(), *Code, DeclaringHeader, - PreferredHeader.empty() ? DeclaringHeader : PreferredHeader); - if (!Replaces) { - std::string ErrMsg = - ("Failed to generate include insertion edits for adding " + - DeclaringHeader + " (" + PreferredHeader + ") into " + - FileURI.file()) - .str(); - log(ErrMsg + ":" + llvm::toString(Replaces.takeError())); - replyError(ErrorCode::InternalError, ErrMsg); - return; - } - auto Edits = replacementsToEdits(*Code, *Replaces); - WorkspaceEdit WE; - WE.changes = {{FileURI.uri(), Edits}}; - - reply(("Inserted header " + DeclaringHeader + " (" + PreferredHeader + ")") - .str()); - ApplyEdit(std::move(WE)); } else { // We should not get here because ExecuteCommandParams would not have // parsed in the first place and this handler should not be called. But if Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -179,22 +179,6 @@ void rename(PathRef File, Position Pos, llvm::StringRef NewName, Callback> CB); - /// Inserts a new #include into \p File, if it's not present in \p Code. - /// - /// \p DeclaringHeader The original header corresponding to this insertion - /// e.g. the header that declared a symbol. This can be either a URI or a - /// literal string quoted with <> or "" that can be #included directly. - /// \p InsertedHeader The preferred header to be inserted. This may be - /// different from \p DeclaringHeader as a header file can have a different - /// canonical include. This can be either a URI or a literal string quoted - /// with <> or "" that can be #included directly. - /// - /// Both OriginalHeader and InsertedHeader will be considered to determine - /// whether an include needs to be added. - Expected insertInclude(PathRef File, StringRef Code, - StringRef DeclaringHeader, - StringRef InsertedHeader); - /// Only for testing purposes. /// Waits until all requests to worker thread are finished and dumps AST for /// \p File. \p File must be in the list of added documents. Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -161,6 +161,7 @@ // both the old and the new version in case only one of them matches. CompletionList Result = clangd::codeComplete( File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, + PreambleData ? PreambleData->IncLocations : std::vector(), IP->Contents, Pos, FS, PCHs, CodeCompleteOpts); CB(std::move(Result)); }; @@ -272,66 +273,6 @@ "Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB))); } -/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal -/// include. -static llvm::Expected toHeaderFile(StringRef Header, - llvm::StringRef HintPath) { - if (isLiteralInclude(Header)) - return HeaderFile{Header.str(), /*Verbatim=*/true}; - auto U = URI::parse(Header); - if (!U) - return U.takeError(); - - auto IncludePath = URI::includeSpelling(*U); - if (!IncludePath) - return IncludePath.takeError(); - if (!IncludePath->empty()) - return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; - - auto Resolved = URI::resolve(*U, HintPath); - if (!Resolved) - return Resolved.takeError(); - return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; -} - -Expected -ClangdServer::insertInclude(PathRef File, StringRef Code, - StringRef DeclaringHeader, - StringRef InsertedHeader) { - assert(!DeclaringHeader.empty() && !InsertedHeader.empty()); - std::string ToInclude; - auto ResolvedOrginal = toHeaderFile(DeclaringHeader, File); - if (!ResolvedOrginal) - return ResolvedOrginal.takeError(); - auto ResolvedPreferred = toHeaderFile(InsertedHeader, File); - if (!ResolvedPreferred) - return ResolvedPreferred.takeError(); - tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File); - auto Include = - calculateIncludePath(File, Code, *ResolvedOrginal, *ResolvedPreferred, - CompileCommand, FSProvider.getFileSystem()); - if (!Include) - return Include.takeError(); - if (Include->empty()) - return tooling::Replacements(); - ToInclude = std::move(*Include); - - auto Style = format::getStyle("file", File, "llvm"); - if (!Style) { - llvm::consumeError(Style.takeError()); - // FIXME(ioeric): needs more consistent style support in clangd server. - Style = format::getLLVMStyle(); - } - // Replacement with offset UINT_MAX and length 0 will be treated as include - // insertion. - tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude); - auto Replaces = - format::cleanupAroundReplacements(Code, tooling::Replacements(R), *Style); - if (!Replaces) - return Replaces; - return formatReplacements(Code, *Replaces, *Style); -} - void ClangdServer::dumpAST(PathRef File, UniqueFunction Callback) { auto Action = [](decltype(Callback) Callback, Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -12,6 +12,7 @@ #include "Diagnostics.h" #include "Function.h" +#include "Headers.h" #include "Path.h" #include "Protocol.h" #include "clang/Frontend/FrontendAction.h" @@ -40,18 +41,16 @@ namespace clangd { -using InclusionLocations = std::vector>; - // Stores Preamble and associated data. struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector TopLevelDeclIDs, - std::vector Diags, InclusionLocations IncLocations); + std::vector Diags, std::vector IncLocations); PrecompiledPreamble Preamble; std::vector TopLevelDeclIDs; std::vector Diags; - InclusionLocations IncLocations; + std::vector IncLocations; }; /// Information required to run clang, e.g. to parse AST or do code completion. @@ -95,14 +94,14 @@ /// Returns the esitmated size of the AST and the accessory structures, in /// bytes. Does not include the size of the preamble. std::size_t getUsedBytes() const; - const InclusionLocations &getInclusionLocations() const; + const std::vector &getInclusionLocations() const; private: ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, std::vector TopLevelDecls, std::vector Diags, - InclusionLocations IncLocations); + std::vector IncLocations); private: void ensurePreambleDeclsDeserialized(); @@ -122,7 +121,7 @@ std::vector Diags; std::vector TopLevelDecls; bool PreambleDeclsDeserialized; - InclusionLocations IncLocations; + std::vector IncLocations; }; using ASTParsedCallback = std::function; Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -83,41 +83,13 @@ std::vector TopLevelDecls; }; -class InclusionLocationsCollector : public PPCallbacks { -public: - InclusionLocationsCollector(SourceManager &SourceMgr, - InclusionLocations &IncLocations) - : SourceMgr(SourceMgr), IncLocations(IncLocations) {} - - void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, - StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, const FileEntry *File, - StringRef SearchPath, StringRef RelativePath, - const Module *Imported) override { - auto SR = FilenameRange.getAsRange(); - if (SR.isInvalid() || !File || File->tryGetRealPathName().empty()) - return; - - if (SourceMgr.isInMainFile(SR.getBegin())) { - // Only inclusion directives in the main file make sense. The user cannot - // select directives not in the main file. - IncLocations.emplace_back(halfOpenToRange(SourceMgr, FilenameRange), - File->tryGetRealPathName()); - } - } - -private: - SourceManager &SourceMgr; - InclusionLocations &IncLocations; -}; - class CppFilePreambleCallbacks : public PreambleCallbacks { public: std::vector takeTopLevelDeclIDs() { return std::move(TopLevelDeclIDs); } - InclusionLocations takeInclusionLocations() { + std::vector takeInclusionLocations() { return std::move(IncLocations); } @@ -145,14 +117,13 @@ std::unique_ptr createPPCallbacks() override { assert(SourceMgr && "SourceMgr must be set at this point"); - return llvm::make_unique(*SourceMgr, - IncLocations); + return collectInclusionsInMainFileCallback(*SourceMgr, IncLocations); } private: std::vector TopLevelDecls; std::vector TopLevelDeclIDs; - InclusionLocations IncLocations; + std::vector IncLocations; SourceManager *SourceMgr = nullptr; }; @@ -190,15 +161,14 @@ return llvm::None; } - InclusionLocations IncLocations; + std::vector IncLocations; // Copy over the includes from the preamble, then combine with the // non-preamble includes below. if (Preamble) IncLocations = Preamble->IncLocations; - Clang->getPreprocessor().addPPCallbacks( - llvm::make_unique(Clang->getSourceManager(), - IncLocations)); + Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback( + Clang->getSourceManager(), IncLocations)); if (!Action->Execute()) log("Execute() failed when building AST for " + MainInput.getFile()); @@ -278,14 +248,14 @@ ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags); } -const InclusionLocations &ParsedAST::getInclusionLocations() const { +const std::vector &ParsedAST::getInclusionLocations() const { return IncLocations; } PreambleData::PreambleData(PrecompiledPreamble Preamble, std::vector TopLevelDeclIDs, std::vector Diags, - InclusionLocations IncLocations) + std::vector IncLocations) : Preamble(std::move(Preamble)), TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)), IncLocations(std::move(IncLocations)) {} @@ -294,7 +264,7 @@ std::unique_ptr Clang, std::unique_ptr Action, std::vector TopLevelDecls, - std::vector Diags, InclusionLocations IncLocations) + std::vector Diags, std::vector IncLocations) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false), Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -15,6 +15,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H +#include "Headers.h" #include "Logger.h" #include "Path.h" #include "Protocol.h" @@ -69,6 +70,7 @@ CompletionList codeComplete(PathRef FileName, const tooling::CompileCommand &Command, PrecompiledPreamble const *Preamble, + const std::vector &Inclusions, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -18,9 +18,11 @@ #include "CodeCompletionStrings.h" #include "Compiler.h" #include "FuzzyMatch.h" +#include "Headers.h" #include "Logger.h" #include "SourceCode.h" #include "Trace.h" +#include "URI.h" #include "index/Index.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" @@ -219,6 +221,76 @@ return S; } +/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal +/// include. +static llvm::Expected toHeaderFile(StringRef Header, + llvm::StringRef HintPath) { + if (isLiteralInclude(Header)) + return HeaderFile{Header.str(), /*Verbatim=*/true}; + auto U = URI::parse(Header); + if (!U) + return U.takeError(); + + auto IncludePath = URI::includeSpelling(*U); + if (!IncludePath) + return IncludePath.takeError(); + if (!IncludePath->empty()) + return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; + + auto Resolved = URI::resolve(*U, HintPath); + if (!Resolved) + return Resolved.takeError(); + return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; +} + +// Calculates include path and insertion edit for a new header. +class IncludeInserter { +public: + IncludeInserter(StringRef FileName, StringRef Code, + const format::FormatStyle &Style, StringRef BuildDir, + HeaderSearch &HeaderSearchInfo) + : FileName(FileName), Code(Code), BuildDir(BuildDir), + HeaderSearchInfo(HeaderSearchInfo), + Inserter(FileName, Code, Style.IncludeStyle) {} + + void setExistingInclusions(std::vector &&Inclusions) { + this->Inclusions = std::move(Inclusions); + } + + // The header name is either a URI or a literal include. + Expected> insert(StringRef FileName, + StringRef DeclaringHeader, + StringRef InsertedHeader) const { + auto ResolvedDeclaring = toHeaderFile(DeclaringHeader, FileName); + if (!ResolvedDeclaring) + return ResolvedDeclaring.takeError(); + auto ResolvedInserted = toHeaderFile(InsertedHeader, FileName); + if (!ResolvedInserted) + return ResolvedInserted.takeError(); + auto Include = + calculateIncludePath(FileName, BuildDir, HeaderSearchInfo, Inclusions, + *ResolvedDeclaring, *ResolvedInserted); + if (!Include) + return Include.takeError(); + if (Include->empty()) + return llvm::None; + StringRef IncludeRef = *Include; + auto Insertion = + Inserter.insert(IncludeRef.trim("\"<>"), IncludeRef.startswith("<")); + if (!Insertion) + return llvm::None; + return replacementToEdit(Code, *Insertion); + } + +private: + StringRef FileName; + StringRef Code; + StringRef BuildDir; + HeaderSearch &HeaderSearchInfo; + std::vector Inclusions; + tooling::HeaderIncludes Inserter; // Computers insertion replacement. +}; + /// A code completion result, in clang-native form. /// It may be promoted to a CompletionItem if it's among the top-ranked results. struct CompletionCandidate { @@ -255,10 +327,10 @@ } // Builds an LSP completion item. - CompletionItem build(llvm::StringRef FileName, - const CompletionItemScores &Scores, + CompletionItem build(StringRef FileName, const CompletionItemScores &Scores, const CodeCompleteOptions &Opts, - CodeCompletionString *SemaCCS) const { + CodeCompletionString *SemaCCS, + const IncludeInserter *Includes) const { assert(bool(SemaResult) == bool(SemaCCS)); CompletionItem I; if (SemaResult) { @@ -289,26 +361,25 @@ I.documentation = D->Documentation; if (I.detail.empty()) I.detail = D->CompletionDetail; - // FIXME: delay creating include insertion command to - // "completionItem/resolve", when it is supported - if (!D->IncludeHeader.empty()) { - // LSP favors additionalTextEdits over command. But we are still using - // command here because it would be expensive to calculate #include - // insertion edits for all candidates, and the include insertion edit - // is unlikely to conflict with the code completion edits. - Command Cmd; - // Command title is not added since this is not a user-facing command. - Cmd.command = ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE; - IncludeInsertion Insertion; + if (Includes && !D->IncludeHeader.empty()) { // Fallback to canonical header if declaration location is invalid. - Insertion.declaringHeader = + llvm::StringRef DeclaringHeader = IndexResult->CanonicalDeclaration.FileURI.empty() ? D->IncludeHeader : IndexResult->CanonicalDeclaration.FileURI; - Insertion.preferredHeader = D->IncludeHeader; - Insertion.textDocument.uri = URIForFile(FileName); - Cmd.includeInsertion = std::move(Insertion); - I.command = std::move(Cmd); + llvm::StringRef InsertedHeader = D->IncludeHeader; + assert(!DeclaringHeader.empty() && !InsertedHeader.empty()); + Expected> Edit = + Includes->insert(FileName, DeclaringHeader, InsertedHeader); + if (!Edit) { + std::string ErrMsg = + ("Failed to generate include insertion edits for adding " + + DeclaringHeader + " (" + InsertedHeader + ") into " + FileName) + .str(); + log(ErrMsg + ":" + llvm::toString(Edit.takeError())); + } else if (*Edit) { + I.additionalTextEdits = {std::move(**Edit)}; + } } } } @@ -670,6 +741,7 @@ PathRef FileName; const tooling::CompileCommand &Command; PrecompiledPreamble const *Preamble; + const std::vector &Inclusions; StringRef Contents; Position Pos; IntrusiveRefCntPtr VFS; @@ -677,9 +749,14 @@ }; // Invokes Sema code completion on a file. +// If \p CallbackBeforeAction is set, it will be run after a compiler instance +// has been set up and before the frontend action is executed. For example, this +// can be used to register preprocessor callbacks. bool semaCodeComplete(std::unique_ptr Consumer, const clang::CodeCompleteOptions &Options, - const SemaCompleteInput &Input) { + const SemaCompleteInput &Input, + llvm::function_ref + CallbackBeforeAction = nullptr) { trace::Span Tracer("Sema completion"); std::vector ArgStrs; for (const auto &S : Input.Command.CommandLine) @@ -750,6 +827,8 @@ Input.FileName); return false; } + if (CallbackBeforeAction) + CallbackBeforeAction(*Clang); if (!Action.Execute()) { log("Execute() failed when running codeComplete for " + Input.FileName); return false; @@ -856,7 +935,8 @@ CompletionRecorder *Recorder = nullptr; int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. bool Incomplete = false; // Would more be available with a higher limit? - llvm::Optional Filter; // Initialized once Sema runs. + llvm::Optional Filter; // Initialized once Sema runs. + std::unique_ptr Includes; // Initialized once compiler runs. public: // A CodeCompleteFlow object is only useful for calling run() exactly once. @@ -865,20 +945,44 @@ CompletionList run(const SemaCompleteInput &SemaCCInput) && { trace::Span Tracer("CodeCompleteFlow"); + auto Style = format::getStyle("file", FileName, "LLVM", + SemaCCInput.Contents, SemaCCInput.VFS.get()); + if (!Style) { + log("Failed to get FormaStyle for file" + FileName + + ". Fall back to use LLVM style. Error: " + + llvm::toString(Style.takeError())); + Style = format::getLLVMStyle(); + } + // Inclusions from preamble or the current preprocesor run in sema. + std::vector Inclusions = SemaCCInput.Inclusions; // We run Sema code completion first. It builds an AST and calculates: // - completion results based on the AST. // - partial identifier and context. We need these for the index query. CompletionList Output; auto RecorderOwner = llvm::make_unique(Opts, [&]() { assert(Recorder && "Recorder is not set"); + assert(Includes && "Includes is not set"); + // If preprocessor was run, inclusions from preprocessor callback should + // already be added to Inclusions. + Includes->setExistingInclusions(std::move(Inclusions)); Output = runWithSema(); + Includes.release(); // Make sure this doesn't out-live Clang. SPAN_ATTACH(Tracer, "sema_completion_kind", getCompletionKindString(Recorder->CCContext.getKind())); }); Recorder = RecorderOwner.get(); semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(), - SemaCCInput); + SemaCCInput, + /*CallbackBeforeAction*/ [&](CompilerInstance &Clang) { + Clang.getPreprocessor().addPPCallbacks( + collectInclusionsInMainFileCallback( + Clang.getSourceManager(), Inclusions)); + Includes = llvm::make_unique( + FileName, SemaCCInput.Contents, *Style, + SemaCCInput.Command.Directory, + Clang.getPreprocessor().getHeaderSearchInfo()); + }); SPAN_ATTACH(Tracer, "sema_results", NSema); SPAN_ATTACH(Tracer, "index_results", NIndex); @@ -1004,19 +1108,20 @@ CodeCompletionString *SemaCCS = nullptr; if (auto *SR = Candidate.SemaResult) SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments); - return Candidate.build(FileName, Scores, Opts, SemaCCS); + return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get()); } }; CompletionList codeComplete(PathRef FileName, const tooling::CompileCommand &Command, PrecompiledPreamble const *Preamble, + const std::vector &Inclusions, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, CodeCompleteOptions Opts) { return CodeCompleteFlow(FileName, Opts) - .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs}); + .run({FileName, Command, Preamble, Inclusions, Contents, Pos, VFS, PCHs}); } SignatureHelp signatureHelp(PathRef FileName, @@ -1031,10 +1136,10 @@ Options.IncludeMacros = false; Options.IncludeCodePatterns = false; Options.IncludeBriefComments = true; - semaCodeComplete(llvm::make_unique(Options, Result), - Options, - {FileName, Command, Preamble, Contents, Pos, std::move(VFS), - std::move(PCHs)}); + semaCodeComplete( + llvm::make_unique(Options, Result), Options, + {FileName, Command, Preamble, + /*Inclusions*/ {}, Contents, Pos, std::move(VFS), std::move(PCHs)}); return Result; } Index: clangd/Headers.h =================================================================== --- clangd/Headers.h +++ clangd/Headers.h @@ -11,9 +11,16 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H #include "Path.h" +#include "Protocol.h" #include "clang/Basic/VirtualFileSystem.h" +#include "clang/Format/Format.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/HeaderSearch.h" +#include "clang/Lex/PPCallbacks.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Core/HeaderIncludes.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" namespace clang { @@ -32,25 +39,37 @@ bool valid() const; }; +// A header inclusion in the main file. +struct Inclusion { + Range R; // Inclusion range. + std::string Written; // Inclusion name as written e.g. . + Path Resolved; // Resolved path of included file. Empty if not resolved. +}; + +/// Returns a PPCallback that collects all inclusions in the main file. +/// Inclusions are added to \p Includes. +std::unique_ptr +collectInclusionsInMainFileCallback(const SourceManager &SM, + std::vector &Inclusions); + /// Determines the preferred way to #include a file, taking into account the /// search path. Usually this will prefer a shorter representation like /// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'. /// /// \param File is an absolute file path. +/// \param Inclusions Existing inclusions in the main file. /// \param DeclaringHeader is the original header corresponding to \p /// InsertedHeader e.g. the header that declares a symbol. /// \param InsertedHeader The preferred header to be inserted. This could be the /// same as DeclaringHeader but must be provided. // \return A quoted "path" or . This returns an empty string if: /// - Either \p DeclaringHeader or \p InsertedHeader is already (directly) -/// included in the file (including those included via different paths). +/// in \p Inclusions (including those included via different paths). /// - \p DeclaringHeader or \p InsertedHeader is the same as \p File. -llvm::Expected -calculateIncludePath(PathRef File, llvm::StringRef Code, - const HeaderFile &DeclaringHeader, - const HeaderFile &InsertedHeader, - const tooling::CompileCommand &CompileCommand, - IntrusiveRefCntPtr FS); +llvm::Expected calculateIncludePath( + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, + const std::vector &Inclusions, const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader); } // namespace clangd } // namespace clang Index: clangd/Headers.cpp =================================================================== --- clangd/Headers.cpp +++ clangd/Headers.cpp @@ -10,12 +10,9 @@ #include "Headers.h" #include "Compiler.h" #include "Logger.h" -#include "clang/Frontend/CompilerInstance.h" -#include "clang/Frontend/CompilerInvocation.h" -#include "clang/Frontend/FrontendActions.h" +#include "SourceCode.h" +#include "Trace.h" #include "clang/Lex/HeaderSearch.h" -#include "clang/Lex/PreprocessorOptions.h" -#include "clang/Tooling/CompilationDatabase.h" #include "llvm/Support/Path.h" namespace clang { @@ -24,26 +21,32 @@ class RecordHeaders : public PPCallbacks { public: - RecordHeaders(llvm::StringSet<> &WrittenHeaders, - llvm::StringSet<> &ResolvedHeaders) - : WrittenHeaders(WrittenHeaders), ResolvedHeaders(ResolvedHeaders) {} + RecordHeaders(const SourceManager &SM, std::vector &Includes) + : SM(SM), Includes(Includes) {} - void InclusionDirective(SourceLocation /*HashLoc*/, - const Token & /*IncludeTok*/, + // Record existing #includes - both written and resolved paths. Only #includes + // in the main file are collected. + void InclusionDirective(SourceLocation HashLoc, const Token & /*IncludeTok*/, llvm::StringRef FileName, bool IsAngled, - CharSourceRange /*FilenameRange*/, - const FileEntry *File, llvm::StringRef /*SearchPath*/, + CharSourceRange FilenameRange, const FileEntry *File, + llvm::StringRef /*SearchPath*/, llvm::StringRef /*RelativePath*/, const Module * /*Imported*/) override { - WrittenHeaders.insert( - (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str()); - if (File != nullptr && !File->tryGetRealPathName().empty()) - ResolvedHeaders.insert(File->tryGetRealPathName()); + // Only inclusion directives in the main file make sense. The user cannot + // select directives not in the main file. + if (HashLoc.isInvalid() || !SM.isInMainFile(HashLoc)) + return; + std::string Written = + (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str(); + std::string Resolved = (!File || File->tryGetRealPathName().empty()) + ? "" + : File->tryGetRealPathName(); + Includes.push_back({halfOpenToRange(SM, FilenameRange), Written, Resolved}); } private: - llvm::StringSet<> &WrittenHeaders; - llvm::StringSet<> &ResolvedHeaders; + const SourceManager &SM; + std::vector &Includes; }; } // namespace @@ -57,81 +60,42 @@ (!Verbatim && llvm::sys::path::is_absolute(File)); } + +std::unique_ptr +collectInclusionsInMainFileCallback(const SourceManager &SM, + std::vector &Includes) { + return llvm::make_unique(SM, Includes); +} + /// FIXME(ioeric): we might not want to insert an absolute include path if the /// path is not shortened. -llvm::Expected -calculateIncludePath(llvm::StringRef File, llvm::StringRef Code, - const HeaderFile &DeclaringHeader, - const HeaderFile &InsertedHeader, - const tooling::CompileCommand &CompileCommand, - IntrusiveRefCntPtr FS) { - assert(llvm::sys::path::is_absolute(File)); +llvm::Expected calculateIncludePath( + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, + const std::vector &Inclusions, const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader) { assert(DeclaringHeader.valid() && InsertedHeader.valid()); if (File == DeclaringHeader.File || File == InsertedHeader.File) return ""; - FS->setCurrentWorkingDirectory(CompileCommand.Directory); - - // Set up a CompilerInstance and create a preprocessor to collect existing - // #include headers in \p Code. Preprocesor also provides HeaderSearch with - // which we can calculate the shortest include path for \p Header. - std::vector Argv; - for (const auto &S : CompileCommand.CommandLine) - Argv.push_back(S.c_str()); - IgnoringDiagConsumer IgnoreDiags; - auto CI = clang::createInvocationFromCommandLine( - Argv, - CompilerInstance::createDiagnostics(new DiagnosticOptions(), &IgnoreDiags, - false), - FS); - if (!CI) - return llvm::make_error( - "Failed to create a compiler instance for " + File, - llvm::inconvertibleErrorCode()); - CI->getFrontendOpts().DisableFree = false; - // Parse the main file to get all existing #includes in the file, and then we - // can make sure the same header (even with different include path) is not - // added more than once. - CI->getPreprocessorOpts().SingleFileParseMode = true; - - // The diagnostic options must be set before creating a CompilerInstance. - CI->getDiagnosticOpts().IgnoreWarnings = true; - auto Clang = prepareCompilerInstance( - std::move(CI), /*Preamble=*/nullptr, - llvm::MemoryBuffer::getMemBuffer(Code, File), - std::make_shared(), FS, IgnoreDiags); - - if (Clang->getFrontendOpts().Inputs.empty()) - return llvm::make_error( - "Empty frontend action inputs empty for file " + File, - llvm::inconvertibleErrorCode()); - PreprocessOnlyAction Action; - if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) - return llvm::make_error( - "Failed to begin preprocessor only action for file " + File, - llvm::inconvertibleErrorCode()); - llvm::StringSet<> WrittenHeaders; - llvm::StringSet<> ResolvedHeaders; - Clang->getPreprocessor().addPPCallbacks( - llvm::make_unique(WrittenHeaders, ResolvedHeaders)); - if (!Action.Execute()) - return llvm::make_error( - "Failed to execute preprocessor only action for file " + File, - llvm::inconvertibleErrorCode()); + + llvm::StringSet<> IncludedHeaders; + for (const auto &Inc : Inclusions) { + IncludedHeaders.insert(Inc.Written); + if (!Inc.Resolved.empty()) + IncludedHeaders.insert(Inc.Resolved); + } auto Included = [&](llvm::StringRef Header) { - return WrittenHeaders.find(Header) != WrittenHeaders.end() || - ResolvedHeaders.find(Header) != ResolvedHeaders.end(); + return IncludedHeaders.find(Header) != IncludedHeaders.end(); }; if (Included(DeclaringHeader.File) || Included(InsertedHeader.File)) return ""; - auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo(); bool IsSystem = false; if (InsertedHeader.Verbatim) return InsertedHeader.File; std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics( - InsertedHeader.File, CompileCommand.Directory, &IsSystem); + InsertedHeader.File, BuildDir, &IsSystem); if (IsSystem) Suggested = "<" + Suggested + ">"; else Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -99,6 +99,9 @@ return std::tie(LHS.line, LHS.character) == std::tie(RHS.line, RHS.character); } + friend bool operator!=(const Position &LHS, const Position &RHS) { + return !(LHS == RHS); + } friend bool operator<(const Position &LHS, const Position &RHS) { return std::tie(LHS.line, LHS.character) < std::tie(RHS.line, RHS.character); @@ -538,26 +541,6 @@ bool fromJSON(const json::Expr &, WorkspaceEdit &); json::Expr toJSON(const WorkspaceEdit &WE); -struct IncludeInsertion { - /// The document in which the command was invoked. - /// If either originalHeader or preferredHeader has been (directly) included - /// in the current file, no new include will be inserted. - TextDocumentIdentifier textDocument; - - /// The declaring header corresponding to this insertion e.g. the header that - /// declares a symbol. This could be either a URI or a literal string quoted - /// with <> or "" that can be #included directly. - std::string declaringHeader; - /// The preferred header to be inserted. This may be different from - /// originalHeader as a header file can have a different canonical include. - /// This could be either a URI or a literal string quoted with <> or "" that - /// can be #included directly. If empty, declaringHeader is used to calculate - /// the #include path. - std::string preferredHeader; -}; -bool fromJSON(const json::Expr &, IncludeInsertion &); -json::Expr toJSON(const IncludeInsertion &II); - /// Exact commands are not specified in the protocol so we define the /// ones supported by Clangd here. The protocol specifies the command arguments /// to be "any[]" but to make this safer and more manageable, each command we @@ -569,16 +552,12 @@ struct ExecuteCommandParams { // Command to apply fix-its. Uses WorkspaceEdit as argument. const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND; - // Command to insert an #include into code. - const static llvm::StringLiteral CLANGD_INSERT_HEADER_INCLUDE; /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND std::string command; // Arguments llvm::Optional workspaceEdit; - - llvm::Optional includeInsertion; }; bool fromJSON(const json::Expr &, ExecuteCommandParams &); @@ -750,7 +729,6 @@ /// themselves. std::vector additionalTextEdits; - llvm::Optional command; // TODO(krasimir): The following optional fields defined by the language // server protocol are unsupported: // Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -390,9 +390,6 @@ const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND = "clangd.applyFix"; -const llvm::StringLiteral ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE = - "clangd.insertInclude"; - bool fromJSON(const json::Expr &Params, ExecuteCommandParams &R) { json::ObjectMapper O(Params); if (!O || !O.map("command", R.command)) @@ -402,9 +399,6 @@ if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) { return Args && Args->size() == 1 && fromJSON(Args->front(), R.workspaceEdit); - } else if (R.command == ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) { - return Args && Args->size() == 1 && - fromJSON(Args->front(), R.includeInsertion); } return false; // Unrecognized command. } @@ -433,8 +427,6 @@ auto Cmd = json::obj{{"title", C.title}, {"command", C.command}}; if (C.workspaceEdit) Cmd["arguments"] = {*C.workspaceEdit}; - else if (C.includeInsertion) - Cmd["arguments"] = {*C.includeInsertion}; return std::move(Cmd); } @@ -447,18 +439,6 @@ return json::obj{{"changes", std::move(FileChanges)}}; } -bool fromJSON(const json::Expr &II, IncludeInsertion &R) { - json::ObjectMapper O(II); - return O && O.map("textDocument", R.textDocument) && - O.map("declaringHeader", R.declaringHeader) && - O.map("preferredHeader", R.preferredHeader); -} -json::Expr toJSON(const IncludeInsertion &II) { - return json::obj{{"textDocument", II.textDocument}, - {"declaringHeader", II.declaringHeader}, - {"preferredHeader", II.preferredHeader}}; -} - json::Expr toJSON(const ApplyWorkspaceEditParams &Params) { return json::obj{{"edit", Params.edit}}; } @@ -519,8 +499,6 @@ Result["textEdit"] = *CI.textEdit; if (!CI.additionalTextEdits.empty()) Result["additionalTextEdits"] = json::ary(CI.additionalTextEdits); - if (CI.command) - Result["command"] = *CI.command; return std::move(Result); } Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H #include "Protocol.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Tooling/Core/Replacement.h" namespace clang { class SourceManager; @@ -55,6 +56,15 @@ std::pair splitQualifiedName(llvm::StringRef QName); +TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R); + +std::vector +replacementsToEdits(StringRef Code, + const std::vector &Replacements); + +std::vector replacementsToEdits(StringRef Code, + const tooling::Replacements &Repls); + } // namespace clangd } // namespace clang #endif Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ clangd/SourceCode.cpp @@ -166,5 +166,31 @@ return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)}; } +TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) { + Range ReplacementRange = { + offsetToPosition(Code, R.getOffset()), + offsetToPosition(Code, R.getOffset() + R.getLength())}; + return {ReplacementRange, R.getReplacementText()}; +} + +std::vector +replacementsToEdits(StringRef Code, + const std::vector &Replacements) { + // Turn the replacements into the format specified by the Language Server + // Protocol. Fuse them into one big JSON array. + std::vector Edits; + for (const auto &R : Replacements) + Edits.push_back(replacementToEdit(Code, R)); + return Edits; +} + +std::vector replacementsToEdits(StringRef Code, + const tooling::Replacements &Repls) { + std::vector Edits; + for (const auto &R : Repls) + Edits.push_back(replacementToEdit(Code, R)); + return Edits; +} + } // namespace clangd } // namespace clang Index: clangd/XRefs.cpp =================================================================== --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -235,11 +235,12 @@ std::vector Result; // Handle goto definition for #include. for (auto &IncludeLoc : AST.getInclusionLocations()) { - Range R = IncludeLoc.first; + if (IncludeLoc.Resolved.empty()) + continue; Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg); - if (R.contains(Pos)) - Result.push_back(Location{URIForFile{IncludeLoc.second}, {}}); + if (IncludeLoc.R.contains(Pos)) + Result.push_back(Location{URIForFile{IncludeLoc.Resolved}, {}}); } if (!Result.empty()) return Result; Index: test/clangd/initialize-params-invalid.test =================================================================== --- test/clangd/initialize-params-invalid.test +++ test/clangd/initialize-params-invalid.test @@ -24,8 +24,7 @@ # CHECK-NEXT: "documentRangeFormattingProvider": true, # CHECK-NEXT: "executeCommandProvider": { # CHECK-NEXT: "commands": [ -# CHECK-NEXT: "clangd.applyFix", -# CHECK-NEXT: "clangd.insertInclude" +# CHECK-NEXT: "clangd.applyFix" # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, Index: test/clangd/initialize-params.test =================================================================== --- test/clangd/initialize-params.test +++ test/clangd/initialize-params.test @@ -24,8 +24,7 @@ # CHECK-NEXT: "documentRangeFormattingProvider": true, # CHECK-NEXT: "executeCommandProvider": { # CHECK-NEXT: "commands": [ -# CHECK-NEXT: "clangd.applyFix", -# CHECK-NEXT: "clangd.insertInclude" +# CHECK-NEXT: "clangd.applyFix" # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, Index: test/clangd/insert-include.test =================================================================== --- test/clangd/insert-include.test +++ /dev/null @@ -1,36 +0,0 @@ -# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} ---- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void f() {}"}}} ---- -{"jsonrpc":"2.0","id":3,"method":"workspace/executeCommand","params":{"arguments":[{"declaringHeader":"\"/usr/include/bits/vector\"", "preferredHeader":"","textDocument":{"uri":"test:///main.cpp"}}],"command":"clangd.insertInclude"}} -# CHECK: "id": 3, -# CHECK-NEXT: "jsonrpc": "2.0", -# CHECK-NEXT: "result": "Inserted header \"/usr/include/bits/vector\" ()" -# CHECK-NEXT: } -# CHECK: "method": "workspace/applyEdit", -# CHECK-NEXT: "params": { -# CHECK-NEXT: "edit": { -# CHECK-NEXT: "changes": { -# CHECK-NEXT: "file://{{.*}}/main.cpp": [ -# CHECK-NEXT: { -# CHECK-NEXT: "newText": "#include \n", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: ] -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } ---- -{"jsonrpc":"2.0","id":4,"method":"shutdown"} Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -938,67 +938,6 @@ ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both? } -TEST_F(ClangdVFSTest, InsertIncludes) { - MockFSProvider FS; - ErrorCheckingDiagConsumer DiagConsumer; - MockCompilationDatabase CDB; - std::string SearchDirArg = (llvm::Twine("-I") + testRoot()).str(); - CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), {SearchDirArg.c_str()}); - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); - - auto FooCpp = testPath("foo.cpp"); - const auto Code = R"cpp( -#include "x.h" -#include "z.h" - -void f() {} -)cpp"; - FS.Files[FooCpp] = Code; - runAddDocument(Server, FooCpp, Code); - - auto ChangedCode = [&](llvm::StringRef Original, llvm::StringRef Preferred) { - auto Replaces = Server.insertInclude( - FooCpp, Code, Original, Preferred.empty() ? Original : Preferred); - EXPECT_TRUE(static_cast(Replaces)); - auto Changed = tooling::applyAllReplacements(Code, *Replaces); - EXPECT_TRUE(static_cast(Changed)); - return *Changed; - }; - auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred, - llvm::StringRef Expected) { - return llvm::StringRef(ChangedCode(Original, Preferred)) - .contains((llvm::Twine("#include ") + Expected + "").str()); - }; - - EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"", "\"y.h\"")); - EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"\"Y.h\"", "\"Y.h\"")); - EXPECT_TRUE(Inserted("", /*Preferred=*/"", "")); - EXPECT_TRUE(Inserted("", /*Preferred=*/"", "")); - - std::string OriginalHeader = URI::createFile(testPath("y.h")).toString(); - std::string PreferredHeader = URI::createFile(testPath("Y.h")).toString(); - EXPECT_TRUE(Inserted(OriginalHeader, - /*Preferred=*/"", "\"y.h\"")); - EXPECT_TRUE(Inserted(OriginalHeader, - /*Preferred=*/"", "")); - EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\"")); - EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\"")); - auto TestURIHeader = - URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str()); - EXPECT_TRUE(static_cast(TestURIHeader)); - EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\"")); - - // Check that includes are sorted. - const auto Expected = R"cpp( -#include "x.h" -#include "y.h" -#include "z.h" - -void f() {} -)cpp"; - EXPECT_EQ(Expected, ChangedCode("\"y.h\"", /*Preferred=*/"")); -} - TEST_F(ClangdVFSTest, FormatCode) { MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -45,6 +45,16 @@ MATCHER_P(Filter, F, "") { return arg.filterText == F; } MATCHER_P(Doc, D, "") { return arg.documentation == D; } MATCHER_P(Detail, D, "") { return arg.detail == D; } +MATCHER_P(InsertInclude, IncludeHeader, "") { + if (arg.additionalTextEdits.size() != 1) + return false; + const auto &Edit = arg.additionalTextEdits[0]; + if (Edit.range.start != Edit.range.end) + return false; + SmallVector Matches; + llvm::Regex RE(R"(#include[ ]*(["<][^">]*[">]))"); + return RE.match(Edit.newText, &Matches) && Matches[1] == IncludeHeader; +} MATCHER_P(PlainText, Text, "") { return arg.insertTextFormat == clangd::InsertTextFormat::PlainText && arg.insertText == Text; @@ -58,6 +68,8 @@ return true; return llvm::StringRef(arg.insertText).contains(arg.filterText); } +MATCHER(HasAdditionalEdits, "") { return !arg.additionalTextEdits.empty(); } + // Shorthand for Contains(Named(Name)). Matcher &> Has(std::string Name) { return Contains(Named(std::move(Name))); @@ -75,9 +87,7 @@ return MemIndex::build(std::move(Slab).build()); } -// Builds a server and runs code completion. -// If IndexSymbols is non-empty, an index will be built and passed to opts. -CompletionList completions(StringRef Text, +CompletionList completions(ClangdServer &Server, StringRef Text, std::vector IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { std::unique_ptr OverrideIndex; @@ -87,10 +97,6 @@ Opts.Index = OverrideIndex.get(); } - MockFSProvider FS; - MockCompilationDatabase CDB; - IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); @@ -101,6 +107,18 @@ return CompletionList; } +// Builds a server and runs code completion. +// If IndexSymbols is non-empty, an index will be built and passed to opts. +CompletionList completions(StringRef Text, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + return completions(Server, Text, std::move(IndexSymbols), std::move(Opts)); +} + std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) { std::string Result; raw_string_ostream OS(Result); @@ -144,6 +162,20 @@ Symbol var(StringRef Name) { return sym(Name, index::SymbolKind::Variable, "@\\0"); } + +Symbol::Details detailWithInclude(StringRef IncludeHeader) { + Symbol::Details D; + D.IncludeHeader = IncludeHeader; + return D; +} +Symbol &&addDeclaringHeaders(Symbol &&Sym, StringRef DeclaringHeader = "") { + Sym.CanonicalDeclaration.FileURI = DeclaringHeader; + return std::move(Sym); +} +Symbol &&addDetails(Symbol &&Sym, const Symbol::Details &D) { + Sym.Detail = &D; + return std::move(Sym); +} Symbol ns(StringRef Name) { return sym(Name, index::SymbolKind::Namespace, "@N@\\0"); } @@ -505,6 +537,73 @@ EXPECT_TRUE(Results.isIncomplete); } +TEST(CompletionTest, InsertNewInclude) { + auto Results = completions( + R"cpp( + int main() { ns::^ } + )cpp", + {addDetails(cls("ns::X"), detailWithInclude(""))}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), InsertInclude("")))); +} + +TEST(CompletionTest, PreferIncludeHeaderToDeclaringHeader) { + auto Results = completions( + R"cpp( + int main() { ns::^ } + )cpp", + {addDetails(addDeclaringHeaders(cls("ns::X"), ""), + detailWithInclude(""))}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), InsertInclude("")))); +} + +TEST(CompletionTest, HeaderAlreadyIncludedRegex) { + auto Results = completions( + R"cpp( + #include + int main() { ns::^ } + )cpp", + {addDetails(cls("ns::X"), detailWithInclude(""))}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits())))); +} + +TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) { + MockFSProvider FS; + MockCompilationDatabase CDB; + std::string Subdir = testPath("sub"); + std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str(); + CDB.ExtraClangFlags = {SearchDirArg.c_str()}; + std::string BarHeader = testPath("sub/bar.h"); + FS.Files[BarHeader] = ""; + + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + // Shoten include path based on search dirctory. + auto Results = completions( + Server, + R"cpp( + int main() { ns::^ } + )cpp", + {addDetails(cls("ns::X"), + detailWithInclude(URI::createFile(BarHeader).toString()))}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\"")))); + // Duplicate based on inclusions in preamble. + Results = completions( + Server, + R"cpp( + #include "sub/bar.h" // not shortest + int main() { ns::^ } + )cpp", + {addDetails(cls("ns::X"), + detailWithInclude(URI::createFile(BarHeader).toString()))}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits())))); +} + TEST(CompletionTest, IndexSuppressesPreambleCompletions) { MockFSProvider FS; MockCompilationDatabase CDB; Index: unittests/clangd/HeadersTests.cpp =================================================================== --- unittests/clangd/HeadersTests.cpp +++ unittests/clangd/HeadersTests.cpp @@ -8,7 +8,13 @@ //===----------------------------------------------------------------------===// #include "Headers.h" + +#include "Compiler.h" #include "TestFS.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/CompilerInvocation.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Lex/PreprocessorOptions.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -16,30 +22,82 @@ namespace clangd { namespace { +using ::testing::AllOf; +using ::testing::UnorderedElementsAre; + class HeadersTest : public ::testing::Test { public: HeadersTest() { CDB.ExtraClangFlags = {SearchDirArg.c_str()}; FS.Files[MainFile] = ""; + // Make sure directory sub/ exists. + FS.Files[testPath("sub/EMPTY")] = ""; + } + +private: + std::unique_ptr setupClang() { + auto Cmd = CDB.getCompileCommand(MainFile); + assert(static_cast(Cmd)); + auto VFS = FS.getFileSystem(); + VFS->setCurrentWorkingDirectory(Cmd->Directory); + + std::vector Argv; + for (const auto &S : Cmd->CommandLine) + Argv.push_back(S.c_str()); + auto CI = clang::createInvocationFromCommandLine( + Argv, + CompilerInstance::createDiagnostics(new DiagnosticOptions(), + &IgnoreDiags, false), + VFS); + EXPECT_TRUE(static_cast(CI)); + CI->getFrontendOpts().DisableFree = false; + + // The diagnostic options must be set before creating a CompilerInstance. + CI->getDiagnosticOpts().IgnoreWarnings = true; + auto Clang = prepareCompilerInstance( + std::move(CI), /*Preamble=*/nullptr, + llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile), + std::make_shared(), VFS, IgnoreDiags); + + EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty()); + return Clang; } protected: + std::vector collectIncludes() { + auto Clang = setupClang(); + PreprocessOnlyAction Action; + EXPECT_TRUE( + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])); + std::vector Inclusions; + Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback( + Clang->getSourceManager(), Inclusions)); + EXPECT_TRUE(Action.Execute()); + Action.EndSourceFile(); + return Inclusions; + } + // Calculates the include path, or returns "" on error. std::string calculate(PathRef Original, PathRef Preferred = "", + const std::vector &Inclusions = {}, bool ExpectError = false) { + auto Clang = setupClang(); + PreprocessOnlyAction Action; + EXPECT_TRUE( + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])); + if (Preferred.empty()) Preferred = Original; - auto VFS = FS.getFileSystem(); - auto Cmd = CDB.getCompileCommand(MainFile); - assert(static_cast(Cmd)); - VFS->setCurrentWorkingDirectory(Cmd->Directory); auto ToHeaderFile = [](llvm::StringRef Header) { return HeaderFile{Header, /*Verbatim=*/!llvm::sys::path::is_absolute(Header)}; }; - auto Path = calculateIncludePath(MainFile, FS.Files[MainFile], - ToHeaderFile(Original), - ToHeaderFile(Preferred), *Cmd, VFS); + + auto Path = calculateIncludePath( + MainFile, CDB.getCompileCommand(MainFile)->Directory, + Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions, + ToHeaderFile(Original), ToHeaderFile(Preferred)); + Action.EndSourceFile(); if (!Path) { llvm::consumeError(Path.takeError()); EXPECT_TRUE(ExpectError); @@ -49,52 +107,31 @@ } return std::move(*Path); } + MockFSProvider FS; MockCompilationDatabase CDB; std::string MainFile = testPath("main.cpp"); std::string Subdir = testPath("sub"); std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str(); + IgnoringDiagConsumer IgnoreDiags; }; -TEST_F(HeadersTest, InsertInclude) { - std::string Path = testPath("sub/bar.h"); - FS.Files[Path] = ""; - EXPECT_EQ(calculate(Path), "\"bar.h\""); -} +MATCHER_P(Written, Name, "") { return arg.Written == Name; } +MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; } -TEST_F(HeadersTest, DontInsertDuplicateSameName) { +TEST_F(HeadersTest, CollectRewrittenAndResolved) { FS.Files[MainFile] = R"cpp( -#include "bar.h" +#include "sub/bar.h" // not shortest )cpp"; std::string BarHeader = testPath("sub/bar.h"); FS.Files[BarHeader] = ""; - EXPECT_EQ(calculate(BarHeader), ""); -} -TEST_F(HeadersTest, DontInsertDuplicateDifferentName) { - std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; - FS.Files[MainFile] = R"cpp( -#include "sub/bar.h" // not shortest -)cpp"; - EXPECT_EQ(calculate("\"sub/bar.h\""), ""); // Duplicate rewritten. - EXPECT_EQ(calculate(BarHeader), ""); // Duplicate resolved. - EXPECT_EQ(calculate(BarHeader, "\"BAR.h\""), ""); // Do not insert preferred. + EXPECT_THAT(collectIncludes(), + UnorderedElementsAre( + AllOf(Written("\"sub/bar.h\""), Resolved(BarHeader)))); } -TEST_F(HeadersTest, DontInsertDuplicatePreferred) { - std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; - FS.Files[MainFile] = R"cpp( -#include "sub/bar.h" // not shortest -)cpp"; - // Duplicate written. - EXPECT_EQ(calculate("\"original.h\"", "\"sub/bar.h\""), ""); - // Duplicate resolved. - EXPECT_EQ(calculate("\"original.h\"", BarHeader), ""); -} - -TEST_F(HeadersTest, StillInsertIfTrasitivelyIncluded) { +TEST_F(HeadersTest, OnlyCollectInclusionsInMain) { std::string BazHeader = testPath("sub/baz.h"); FS.Files[BazHeader] = ""; std::string BarHeader = testPath("sub/bar.h"); @@ -104,27 +141,65 @@ FS.Files[MainFile] = R"cpp( #include "bar.h" )cpp"; - EXPECT_EQ(calculate(BazHeader), "\"baz.h\""); + EXPECT_THAT( + collectIncludes(), + UnorderedElementsAre(AllOf(Written("\"bar.h\""), Resolved(BarHeader)))); +} + +TEST_F(HeadersTest, UnResolvedInclusion) { + FS.Files[MainFile] = R"cpp( +#include "foo.h" +)cpp"; + + EXPECT_THAT(collectIncludes(), + UnorderedElementsAre(AllOf(Written("\"foo.h\""), Resolved("")))); +} + +TEST_F(HeadersTest, InsertInclude) { + std::string Path = testPath("sub/bar.h"); + FS.Files[Path] = ""; + EXPECT_EQ(calculate(Path), "\"bar.h\""); } TEST_F(HeadersTest, DoNotInsertIfInSameFile) { MainFile = testPath("main.h"); - FS.Files[MainFile] = ""; EXPECT_EQ(calculate(MainFile), ""); } +TEST_F(HeadersTest, ShortenedInclude) { + std::string BarHeader = testPath("sub/bar.h"); + EXPECT_EQ(calculate(BarHeader), "\"bar.h\""); +} + +TEST_F(HeadersTest, NotShortenedInclude) { + std::string BarHeader = testPath("sub-2/bar.h"); + EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\""); +} + TEST_F(HeadersTest, PreferredHeader) { - FS.Files[MainFile] = ""; std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; EXPECT_EQ(calculate(BarHeader, ""), ""); std::string BazHeader = testPath("sub/baz.h"); - FS.Files[BazHeader] = ""; EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\""); } +TEST_F(HeadersTest, DontInsertDuplicatePreferred) { + std::vector Inclusions = { + {Range(), /*Written*/ "\"bar.h\"", /*Resolved*/ ""}}; + EXPECT_EQ(calculate(testPath("sub/bar.h"), "\"bar.h\"", Inclusions), ""); + EXPECT_EQ(calculate("\"x.h\"", "\"bar.h\"", Inclusions), ""); +} + +TEST_F(HeadersTest, DontInsertDuplicateResolved) { + std::string BarHeader = testPath("sub/bar.h"); + std::vector Inclusions = { + {Range(), /*Written*/ "fake-bar.h", /*Resolved*/ BarHeader}}; + EXPECT_EQ(calculate(BarHeader, "", Inclusions), ""); + // Do not insert preferred. + EXPECT_EQ(calculate(BarHeader, "\"BAR.h\"", Inclusions), ""); +} + } // namespace } // namespace clangd } // namespace clang -