ioeric updated this revision to Diff 182800.
ioeric added a comment.
- Rebase on D56903 <https://reviews.llvm.org/D56903>.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57021/new/
https://reviews.llvm.org/D57021
Files:
clangd/ClangdUnit.cpp
clangd/IncludeFixer.cpp
clangd/IncludeFixer.h
unittests/clangd/IncludeFixerTests.cpp
Index: unittests/clangd/IncludeFixerTests.cpp
===================================================================
--- unittests/clangd/IncludeFixerTests.cpp
+++ unittests/clangd/IncludeFixerTests.cpp
@@ -13,6 +13,8 @@
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -24,7 +26,12 @@
using testing::UnorderedElementsAre;
testing::Matcher<const Diag &> WithFix(testing::Matcher<Fix> FixMatcher) {
- return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+ return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher));
+}
+
+testing::Matcher<const Diag &> WithFix(testing::Matcher<Fix> FixMatcher1,
+ testing::Matcher<Fix> FixMatcher2) {
+ return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
}
MATCHER_P2(Diag, Range, Message,
@@ -39,6 +46,24 @@
arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
}
+struct SymbolWithHeader {
+ std::string QName;
+ std::string DeclaringFile;
+ std::string IncludeHeader;
+};
+
+std::unique_ptr<SymbolIndex> buildIndexWithSymbol(llvm::ArrayRef<SymbolWithHeader> Syms) {
+ SymbolSlab::Builder Slab;
+ for (const auto &S : Syms) {
+ Symbol Sym = symbol(S.QName);
+ Sym.Flags |= Symbol::IndexedForCodeCompletion;
+ Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str();
+ Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1);
+ Slab.insert(Sym);
+ }
+ return MemIndex::build(std::move(Slab).build(), RefSlab());
+}
+
TEST(IncludeFixerTest, IncompleteType) {
Annotations Test(R"cpp(
$insert[[]]namespace ns {
@@ -51,14 +76,8 @@
}
)cpp");
auto TU = TestTU::withCode(Test.code());
- Symbol Sym = symbol("ns::X");
- Sym.Flags |= Symbol::IndexedForCodeCompletion;
- Sym.CanonicalDeclaration.FileURI = "unittest:///x.h";
- Sym.IncludeHeaders.emplace_back("\"x.h\"", 1);
-
- SymbolSlab::Builder Slab;
- Slab.insert(Sym);
- auto Index = MemIndex::build(std::move(Slab).build(), RefSlab());
+ auto Index = buildIndexWithSymbol(
+ {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""}});
TU.ExternalIndex = Index.get();
EXPECT_THAT(
@@ -73,6 +92,65 @@
"Add include \"x.h\" for symbol ns::X")))));
}
+TEST(IncludeFixerTest, Typo) {
+ Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+void foo() {
+ $unqualified[[X]] x;
+}
+}
+void bar() {
+ ns::$qualified[[X]] x; // ns:: is valid.
+ ::$global[[Global]] glob;
+}
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ auto Index = buildIndexWithSymbol(
+ {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""},
+ SymbolWithHeader{"Global", "unittest:///global.h", "\"global.h\""}});
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+ WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+ "Add include \"x.h\" for symbol ns::X"))),
+ AllOf(Diag(Test.range("qualified"),
+ "no type named 'X' in namespace 'ns'"),
+ WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+ "Add include \"x.h\" for symbol ns::X"))),
+ AllOf(Diag(Test.range("global"),
+ "no type named 'Global' in the global namespace"),
+ WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
+ "Add include \"global.h\" for symbol Global")))));
+}
+
+TEST(IncludeFixerTest, MultipleMatchedSymbols) {
+ Annotations Test(R"cpp(
+$insert[[]]namespace na {
+namespace nb {
+void foo() {
+ $unqualified[[X]] x;
+}
+}
+}
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ auto Index = buildIndexWithSymbol(
+ {SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""},
+ SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(AllOf(
+ Diag(Test.range("unqualified"), "unknown type name 'X'"),
+ WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
+ "Add include \"a.h\" for symbol na::X"),
+ Fix(Test.range("insert"), "#include \"b.h\"\n",
+ "Add include \"b.h\" for symbol na::nb::X")))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/IncludeFixer.h
===================================================================
--- clangd/IncludeFixer.h
+++ clangd/IncludeFixer.h
@@ -15,6 +15,13 @@
#include "index/Index.h"
#include "clang/AST/Type.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Sema/ExternalSemaSource.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include <memory>
@@ -26,24 +33,67 @@
/// include headers with the definition.
class IncludeFixer {
public:
- IncludeFixer(llvm::StringRef File, std::unique_ptr<IncludeInserter> Inserter,
+ IncludeFixer(CompilerInstance &Compiler, llvm::StringRef File,
+ std::unique_ptr<IncludeInserter> Inserter,
const SymbolIndex &Index)
- : File(File), Inserter(std::move(Inserter)), Index(Index) {}
+ : File(File), Inserter(std::move(Inserter)), Index(Index),
+ Compiler(Compiler), RecordTypo(new TypoRecorder(Compiler)) {}
/// Returns include insertions that can potentially recover the diagnostic.
std::vector<Fix> fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const;
+ /// Returns an ExternalSemaSource that records typos seen in Sema. It must be
+ /// used in the same Sema run as the IncludeFixer.
+ llvm::IntrusiveRefCntPtr<ExternalSemaSource> typoRecorder() {
+ return RecordTypo;
+ }
+
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;
+ struct TypoRecord {
+ std::string Typo; // The typo identifier e.g. "X" in ns::X.
+ SourceLocation Loc; // Location of the typo.
+ Scope *S; // Scope in which the typo is found.
+ llvm::Optional<std::string> SS; // The scope qualifier before the typo.
+ Sema::LookupNameKind LookupKind; // LookupKind of the typo.
+ };
+
+ /// Records the last typo seen by Sema.
+ class TypoRecorder : public ExternalSemaSource {
+ public:
+ TypoRecorder(CompilerInstance &Compiler) : Compiler(Compiler) {}
+
+ // Captures the latest typo.
+ TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo, int LookupKind,
+ Scope *S, CXXScopeSpec *SS,
+ CorrectionCandidateCallback &CCC,
+ DeclContext *MemberContext, bool EnteringContext,
+ const ObjCObjectPointerType *OPT) override;
+
+ llvm::Optional<TypoRecord> lastTypo() const { return LastTypo; }
+
+ private:
+ CompilerInstance &Compiler;
+
+ llvm::Optional<TypoRecord> LastTypo;
+ };
+
+ /// Attempts to fix the typo associated with the current diagnostic. We assume
+ /// a diagnostic is caused by a typo when they have the same source location
+ /// and the typo is the last typo we've seen during the Sema run.
+ std::vector<Fix> fixTypo(const TypoRecord &Typo) const;
+
std::string File;
std::unique_ptr<IncludeInserter> Inserter;
const SymbolIndex &Index;
+ CompilerInstance &Compiler;
+ // This collects the last typo so that we can associate it with the
+ // diagnostic.
+ llvm::IntrusiveRefCntPtr<TypoRecorder> RecordTypo;
};
} // namespace clangd
Index: clangd/IncludeFixer.cpp
===================================================================
--- clangd/IncludeFixer.cpp
+++ clangd/IncludeFixer.cpp
@@ -14,12 +14,20 @@
#include "SourceCode.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/None.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
+#include <vector>
namespace clang {
namespace clangd {
@@ -32,6 +40,22 @@
DiagID == diag::err_incomplete_base_class;
}
+// 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,
@@ -47,6 +71,21 @@
return fixInCompleteType(*T);
}
}
+ } else if (auto LastTypo = RecordTypo->lastTypo()) {
+ // Try to fix typos caused by missing declaraion.
+ // E.g.
+ // clang::SourceManager SM;
+ // ~~~~~~~~~~~~~
+ // Typo
+ // or
+ // namespace clang { SourceManager SM; }
+ // ~~~~~~~~~~~~~
+ // Typo
+ // We only attempt to recover a diagnostic if it has the same location as
+ // the last seen typo.
+ if (DiagLevel >= DiagnosticsEngine::Error &&
+ LastTypo->Loc == Info.getLocation())
+ return fixTypo(*LastTypo);
}
return {};
}
@@ -115,5 +154,97 @@
return Fixes;
}
+TypoCorrection IncludeFixer::TypoRecorder::CorrectTypo(
+ const DeclarationNameInfo &Typo, int LookupKind, Scope *S, CXXScopeSpec *SS,
+ CorrectionCandidateCallback &CCC, DeclContext *MemberContext,
+ bool EnteringContext, const ObjCObjectPointerType *OPT) {
+ if (Compiler.getSema().isSFINAEContext())
+ return TypoCorrection();
+ if (!Compiler.getSourceManager().isWrittenInMainFile(Typo.getLoc()))
+ return clang::TypoCorrection();
+
+ TypoRecord Record;
+ Record.Typo = Typo.getAsString();
+ Record.Loc = Typo.getBeginLoc();
+ assert(S);
+ Record.S = S;
+ Record.LookupKind = static_cast<Sema::LookupNameKind>(LookupKind);
+
+ // 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".
+ if (SS && SS->isNotEmpty()) { // "::" or "ns::"
+ if (auto *Nested = SS->getScopeRep()) {
+ if (Nested->getKind() == NestedNameSpecifier::Global)
+ Record.SS = "";
+ else if (const auto *NS = Nested->getAsNamespace())
+ Record.SS = 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();
+ }
+ }
+
+ LastTypo = std::move(Record);
+
+ return TypoCorrection();
+}
+
+std::vector<Fix> IncludeFixer::fixTypo(const TypoRecord &Typo) const {
+ std::vector<std::string> Scopes;
+ if (Typo.SS) {
+ Scopes.push_back(*Typo.SS);
+ } else {
+ // No scope qualifier is specified. Collect all accessible scopes in the
+ // context.
+ VisitedContextCollector Collector;
+ Compiler.getSema().LookupVisibleDecls(Typo.S, Typo.LookupKind, Collector,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+
+ Scopes.push_back("");
+ for (const auto *Ctx : Collector.takeVisitedContexts())
+ if (isa<NamespaceDecl>(Ctx))
+ Scopes.push_back(printNamespaceScope(*Ctx));
+ }
+ vlog("Trying to fix typo \"{0}\" in scopes: [{1}]", Typo.Typo,
+ llvm::join(Scopes.begin(), Scopes.end(), ", "));
+
+ FuzzyFindRequest Req;
+ Req.AnyScope = false;
+ Req.Query = Typo.Typo;
+ Req.Scopes = Scopes;
+ Req.RestrictForCodeCompletion = true;
+
+ 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();
+ if (Syms.empty())
+ return {};
+ std::vector<Fix> Results;
+ for (const auto &Sym : Syms) {
+ auto Fixes = fixesForSymbol(Sym);
+ Results.insert(Results.end(), Fixes.begin(), Fixes.end());
+ }
+ return Results;
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -304,8 +304,10 @@
for (const auto &Inc : Preamble->Includes.MainFileIncludes)
Inserter->addExisting(Inc);
}
- FixIncludes.emplace(MainInput.getFile(), std::move(Inserter), *Index);
+ FixIncludes.emplace(*Clang, MainInput.getFile(), std::move(Inserter),
+ *Index);
ASTDiags.setIncludeFixer(*FixIncludes);
+ Clang->setExternalSemaSource(FixIncludes->typoRecorder());
}
// Copy over the includes from the preamble, then combine with the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
