Skip to content

Commit

Permalink
[clangd] Suggest adding missing includes for typos (like include-fixer).
Browse files Browse the repository at this point in the history
Summary:
This adds include-fixer feature into clangd based on D56903. Clangd now captures
diagnostics caused by typos and attach include insertion fixes to potentially
fix the typo.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, MaskRay, javed.absar, ilya-biryukov, mgorny

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57021

llvm-svn: 353380
  • Loading branch information
Eric Liu committed Feb 7, 2019
1 parent 55f7c72 commit a9e9c50
Show file tree
Hide file tree
Showing 5 changed files with 329 additions and 34 deletions.
9 changes: 5 additions & 4 deletions clang-tools-extra/clangd/ClangdUnit.cpp
Expand Up @@ -322,6 +322,7 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
const clang::Diagnostic &Info) {
return FixIncludes->fix(DiagLevl, Info);
});
Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}

// Copy over the includes from the preamble, then combine with the
Expand Down Expand Up @@ -565,10 +566,10 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
// dirs.
}

return ParsedAST::build(
llvm::make_unique<CompilerInvocation>(*Invocation), Preamble,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs,
std::move(VFS), Inputs.Index ? Inputs.Index : nullptr, Inputs.Opts);
return ParsedAST::build(llvm::make_unique<CompilerInvocation>(*Invocation),
Preamble,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents),
PCHs, std::move(VFS), Inputs.Index, Inputs.Opts);
}

SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
Expand Down
212 changes: 198 additions & 14 deletions clang-tools-extra/clangd/IncludeFixer.cpp
Expand Up @@ -14,16 +14,47 @@
#include "Trace.h"
#include "index/Index.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/Type.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/TypoCorrection.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include <vector>

namespace clang {
namespace clangd {

namespace {

// Collects contexts visited during a Sema name lookup.
class VisitedContextCollector : public VisibleDeclConsumer {
public:
void EnteredContext(DeclContext *Ctx) override { Visited.push_back(Ctx); }

void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
bool InBaseClass) override {}

std::vector<DeclContext *> takeVisitedContexts() {
return std::move(Visited);
}

private:
std::vector<DeclContext *> Visited;
};

} // namespace

std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const {
if (IndexRequestCount >= IndexRequestLimit)
Expand All @@ -42,6 +73,28 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
return fixIncompleteType(*T);
}
}
break;
case diag::err_unknown_typename:
case diag::err_unknown_typename_suggest:
case diag::err_typename_nested_not_found:
case diag::err_no_template:
case diag::err_no_template_suggest:
if (LastUnresolvedName) {
// Try to fix unresolved name caused by missing declaraion.
// E.g.
// clang::SourceManager SM;
// ~~~~~~~~~~~~~
// UnresolvedName
// or
// namespace clang { SourceManager SM; }
// ~~~~~~~~~~~~~
// UnresolvedName
// We only attempt to recover a diagnostic if it has the same location as
// the last seen unresolved name.
if (DiagLevel >= DiagnosticsEngine::Error &&
LastUnresolvedName->Loc == Info.getLocation())
return fixUnresolvedName();
}
}
return {};
}
Expand Down Expand Up @@ -74,11 +127,12 @@ std::vector<Fix> IncludeFixer::fixIncompleteType(const Type &T) const {
if (!Matched || Matched->IncludeHeaders.empty() || !Matched->Definition ||
Matched->CanonicalDeclaration.FileURI != Matched->Definition.FileURI)
return {};
return fixesForSymbol(*Matched);
return fixesForSymbols({*Matched});
}

std::vector<Fix> IncludeFixer::fixesForSymbol(const Symbol &Sym) const {
auto Inserted = [&](llvm::StringRef Header)
std::vector<Fix>
IncludeFixer::fixesForSymbols(llvm::ArrayRef<Symbol> Syms) const {
auto Inserted = [&](const Symbol &Sym, llvm::StringRef Header)
-> llvm::Expected<std::pair<std::string, bool>> {
auto ResolvedDeclaring =
toHeaderFile(Sym.CanonicalDeclaration.FileURI, File);
Expand All @@ -93,21 +147,151 @@ std::vector<Fix> IncludeFixer::fixesForSymbol(const Symbol &Sym) const {
};

std::vector<Fix> Fixes;
for (const auto &Inc : getRankedIncludes(Sym)) {
if (auto ToInclude = Inserted(Inc)) {
if (ToInclude->second)
if (auto Edit = Inserter->insert(ToInclude->first))
Fixes.push_back(
Fix{llvm::formatv("Add include {0} for symbol {1}{2}",
ToInclude->first, Sym.Scope, Sym.Name),
{std::move(*Edit)}});
} else {
vlog("Failed to calculate include insertion for {0} into {1}: {2}", File,
Inc, ToInclude.takeError());
// Deduplicate fixes by include headers. This doesn't distiguish symbols in
// different scopes from the same header, but this case should be rare and is
// thus ignored.
llvm::StringSet<> InsertedHeaders;
for (const auto &Sym : Syms) {
for (const auto &Inc : getRankedIncludes(Sym)) {
if (auto ToInclude = Inserted(Sym, Inc)) {
if (ToInclude->second) {
auto I = InsertedHeaders.try_emplace(ToInclude->first);
if (!I.second)
continue;
if (auto Edit = Inserter->insert(ToInclude->first))
Fixes.push_back(
Fix{llvm::formatv("Add include {0} for symbol {1}{2}",
ToInclude->first, Sym.Scope, Sym.Name),
{std::move(*Edit)}});
}
} else {
vlog("Failed to calculate include insertion for {0} into {1}: {2}",
File, Inc, ToInclude.takeError());
}
}
}
return Fixes;
}
class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
public:
UnresolvedNameRecorder(llvm::Optional<UnresolvedName> &LastUnresolvedName)
: LastUnresolvedName(LastUnresolvedName) {}

void InitializeSema(Sema &S) override { this->SemaPtr = &S; }

// Captures the latest typo and treat it as an unresolved name that can
// potentially be fixed by adding #includes.
TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo, int LookupKind,
Scope *S, CXXScopeSpec *SS,
CorrectionCandidateCallback &CCC,
DeclContext *MemberContext, bool EnteringContext,
const ObjCObjectPointerType *OPT) override {
assert(SemaPtr && "Sema must have been set.");
if (SemaPtr->isSFINAEContext())
return TypoCorrection();
if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
return clang::TypoCorrection();

assert(S && "Enclosing scope must be set.");

UnresolvedName Unresolved;
Unresolved.Name = Typo.getAsString();
Unresolved.Loc = Typo.getBeginLoc();

// FIXME: support invalid scope before a type name. In the following
// example, namespace "clang::tidy::" hasn't been declared/imported.
// namespace clang {
// void f() {
// tidy::Check c;
// ~~~~
// // or
// clang::tidy::Check c;
// ~~~~
// }
// }
// For both cases, the typo and the diagnostic are both on "tidy", and no
// diagnostic is generated for "Check". However, what we want to fix is
// "clang::tidy::Check".

// Extract the typed scope. This is not done lazily because `SS` can get
// out of scope and it's relatively cheap.
llvm::Optional<std::string> SpecifiedScope;
if (SS && SS->isNotEmpty()) { // "::" or "ns::"
if (auto *Nested = SS->getScopeRep()) {
if (Nested->getKind() == NestedNameSpecifier::Global)
SpecifiedScope = "";
else if (const auto *NS = Nested->getAsNamespace())
SpecifiedScope = printNamespaceScope(*NS);
else
// We don't fix symbols in scopes that are not top-level e.g. class
// members, as we don't collect includes for them.
return TypoCorrection();
}
}

auto *Sem = SemaPtr; // Avoid capturing `this`.
Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
std::vector<std::string> Scopes;
if (SpecifiedScope) {
Scopes.push_back(*SpecifiedScope);
} else {
// No scope qualifier is specified. Collect all accessible scopes in the
// context.
VisitedContextCollector Collector;
Sem->LookupVisibleDecls(
S, static_cast<Sema::LookupNameKind>(LookupKind), Collector,
/*IncludeGlobalScope=*/false,
/*LoadExternal=*/false);

Scopes.push_back("");
for (const auto *Ctx : Collector.takeVisitedContexts())
if (isa<NamespaceDecl>(Ctx))
Scopes.push_back(printNamespaceScope(*Ctx));
}
return Scopes;
};
LastUnresolvedName = std::move(Unresolved);

// Never return a valid correction to try to recover. Our suggested fixes
// always require a rebuild.
return TypoCorrection();
}

private:
Sema *SemaPtr = nullptr;

llvm::Optional<UnresolvedName> &LastUnresolvedName;
};

llvm::IntrusiveRefCntPtr<ExternalSemaSource>
IncludeFixer::unresolvedNameRecorder() {
return new UnresolvedNameRecorder(LastUnresolvedName);
}

std::vector<Fix> IncludeFixer::fixUnresolvedName() const {
assert(LastUnresolvedName.hasValue());
auto &Unresolved = *LastUnresolvedName;
std::vector<std::string> Scopes = Unresolved.GetScopes();
vlog("Trying to fix unresolved name \"{0}\" in scopes: [{1}]",
Unresolved.Name, llvm::join(Scopes.begin(), Scopes.end(), ", "));

FuzzyFindRequest Req;
Req.AnyScope = false;
Req.Query = Unresolved.Name;
Req.Scopes = Scopes;
Req.RestrictForCodeCompletion = true;
Req.Limit = 100;

SymbolSlab::Builder Matches;
Index.fuzzyFind(Req, [&](const Symbol &Sym) {
if (Sym.Name != Req.Query)
return;
if (!Sym.IncludeHeaders.empty())
Matches.insert(Sym);
});
auto Syms = std::move(Matches).build();
return fixesForSymbols(std::vector<Symbol>(Syms.begin(), Syms.end()));
}

} // namespace clangd
} // namespace clang
37 changes: 35 additions & 2 deletions clang-tools-extra/clangd/IncludeFixer.h
Expand Up @@ -14,6 +14,13 @@
#include "index/Index.h"
#include "clang/AST/Type.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Sema/ExternalSemaSource.h"
#include "clang/Sema/Sema.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include <memory>

Expand All @@ -34,18 +41,44 @@ class IncludeFixer {
std::vector<Fix> fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const;

/// Returns an ExternalSemaSource that records failed name lookups in Sema.
/// This allows IncludeFixer to suggest inserting headers that define those
/// names.
llvm::IntrusiveRefCntPtr<ExternalSemaSource> unresolvedNameRecorder();

private:
/// Attempts to recover diagnostic caused by an incomplete type \p T.
std::vector<Fix> fixIncompleteType(const Type &T) const;

/// Generates header insertion fixes for \p Sym.
std::vector<Fix> fixesForSymbol(const Symbol &Sym) const;
/// Generates header insertion fixes for all symbols. Fixes are deduplicated.
std::vector<Fix> fixesForSymbols(llvm::ArrayRef<Symbol> Syms) const;

struct UnresolvedName {
std::string Name; // E.g. "X" in foo::X.
SourceLocation Loc; // Start location of the unresolved name.
// Lazily get the possible scopes of the unresolved name. `Sema` must be
// alive when this is called.
std::function<std::vector<std::string>()> GetScopes;
};

/// Records the last unresolved name seen by Sema.
class UnresolvedNameRecorder;

/// Attempts to fix the unresolved name associated with the current
/// diagnostic. We assume a diagnostic is caused by a unresolved name when
/// they have the same source location and the unresolved name is the last
/// one we've seen during the Sema run.
std::vector<Fix> fixUnresolvedName() const;

std::string File;
std::shared_ptr<IncludeInserter> Inserter;
const SymbolIndex &Index;
const unsigned IndexRequestLimit; // Make at most 5 index requests.
mutable unsigned IndexRequestCount = 0;

// These collect the last unresolved name so that we can associate it with the
// diagnostic.
llvm::Optional<UnresolvedName> LastUnresolvedName;
};

} // namespace clangd
Expand Down
10 changes: 5 additions & 5 deletions clang-tools-extra/clangd/SourceCode.h
Expand Up @@ -47,7 +47,7 @@ size_t lspLength(StringRef Code);
/// The returned value is in the range [0, Code.size()].
llvm::Expected<size_t>
positionToOffset(llvm::StringRef Code, Position P,
bool AllowColumnsBeyondLineLength = true);
bool AllowColumnsBeyondLineLength = true);

/// Turn an offset in Code into a [line, column] pair.
/// The offset must be in range [0, Code.size()].
Expand Down Expand Up @@ -110,7 +110,7 @@ Range halfOpenToRange(const SourceManager &SM, CharSourceRange R);
// The offset must be in range [0, Code.size()].
// Prefer to use SourceManager if one is available.
std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code,
size_t Offset);
size_t Offset);

/// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no
/// qualifier.
Expand All @@ -120,10 +120,10 @@ splitQualifiedName(llvm::StringRef QName);
TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R);

std::vector<TextEdit> replacementsToEdits(StringRef Code,
const tooling::Replacements &Repls);
const tooling::Replacements &Repls);

TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
const LangOptions &L);
const LangOptions &L);

/// Get the canonical path of \p F. This means:
///
Expand All @@ -136,7 +136,7 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
/// component that generate it, so that paths are normalized as much as
/// possible.
llvm::Optional<std::string> getCanonicalPath(const FileEntry *F,
const SourceManager &SourceMgr);
const SourceManager &SourceMgr);

bool isRangeConsecutive(const Range &Left, const Range &Right);

Expand Down

0 comments on commit a9e9c50

Please sign in to comment.