Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -302,11 +302,12 @@ 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.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl, const clang::Diagnostic &Info) { return FixIncludes->fix(DiagLevl, Info); }); + Clang->setExternalSemaSource(FixIncludes->typoRecorder()); } // Copy over the includes from the preamble, then combine with the Index: clangd/IncludeFixer.h =================================================================== --- clangd/IncludeFixer.h +++ clangd/IncludeFixer.h @@ -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/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 @@ -25,24 +32,68 @@ /// include headers with the definition. class IncludeFixer { public: - IncludeFixer(llvm::StringRef File, std::unique_ptr Inserter, + IncludeFixer(CompilerInstance &Compiler, llvm::StringRef File, + std::unique_ptr 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(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 typoRecorder() { + return RecordTypo; + } + private: - /// Attempts to recover diagnostic caused by an incomplete type \p T. std::vector fixInCompleteType(const Type &T) const; - /// Generates header insertion fixes for \p Sym. std::vector 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 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 lastTypo() const { return LastTypo; } + + private: + CompilerInstance &Compiler; + + llvm::Optional 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 fixTypo(const TypoRecord &Typo) const; + std::string File; std::unique_ptr Inserter; const SymbolIndex &Index; + CompilerInstance &Compiler; + // This collects the last typo so that we can associate it with the + // diagnostic. + llvm::IntrusiveRefCntPtr RecordTypo; + mutable unsigned IndexRequestCount = 0; }; Index: clangd/IncludeFixer.cpp =================================================================== --- clangd/IncludeFixer.cpp +++ clangd/IncludeFixer.cpp @@ -14,12 +14,20 @@ #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/None.h" #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" +#include 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 takeVisitedContexts() { + return std::move(Visited); + } + +private: + std::vector Visited; +}; + } // namespace std::vector IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel, @@ -49,6 +73,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 {}; } @@ -120,5 +159,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(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 IncludeFixer::fixTypo(const TypoRecord &Typo) const { + std::vector 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(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 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: unittests/clangd/DiagnosticsTests.cpp =================================================================== --- unittests/clangd/DiagnosticsTests.cpp +++ unittests/clangd/DiagnosticsTests.cpp @@ -30,6 +30,11 @@ return Field(&Diag::Fixes, ElementsAre(FixMatcher)); } +testing::Matcher WithFix(testing::Matcher FixMatcher1, + testing::Matcher FixMatcher2) { + return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2)); +} + testing::Matcher WithNote(testing::Matcher NoteMatcher) { return Field(&Diag::Notes, ElementsAre(NoteMatcher)); } @@ -280,6 +285,25 @@ Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty()))); } +struct SymbolWithHeader { + std::string QName; + std::string DeclaringFile; + std::string IncludeHeader; +}; + +std::unique_ptr +buildIndexWithSymbol(llvm::ArrayRef Syms) { + SymbolSlab::Builder Slab; + for (const auto &S : Syms) { + Symbol Sym = cls(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 { @@ -292,14 +316,8 @@ } )cpp"); auto TU = TestTU::withCode(Test.code()); - Symbol Sym = cls("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( @@ -314,6 +332,64 @@ "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