Index: clangd/IncludeFixer.cpp =================================================================== --- clangd/IncludeFixer.cpp +++ clangd/IncludeFixer.cpp @@ -19,6 +19,10 @@ #include "clang/AST/Type.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Basic/TokenKinds.h" #include "clang/Sema/DeclSpec.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/Scope.h" @@ -28,6 +32,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" @@ -172,6 +177,121 @@ } return Fixes; } + +// Returns the identifiers qualified by an unresolved name. \p Loc is the +// start location of the unresolved name. For the example below, this returns +// "::X::Y" that is qualified by unresolved name "clangd": +// clang::clangd::X::Y +// ~ +llvm::Optional qualifiedByUnresolved(const SourceManager &SM, + SourceLocation Loc, + const LangOptions &LangOpts) { + std::string Result; + + SourceLocation NextLoc = Loc; + while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) { + if (!CCTok->is(tok::coloncolon)) + break; + auto IDTok = Lexer::findNextToken(CCTok->getLocation(), SM, LangOpts); + if (!IDTok || !IDTok->is(tok::raw_identifier)) + break; + Result.append(("::" + IDTok->getRawIdentifier()).str()); + NextLoc = IDTok->getLocation(); + } + if (Result.empty()) + return llvm::None; + return Result; +} + +// An unresolved name and its scope information that can be extracted cheaply. +struct CheapUnresolvedName { + std::string Name; + // This is the part of what was typed that was resolved, and it's in its + // resolved form not its typed form (think `namespace clang { clangd::x }` --> + // `clang::clangd::`). + llvm::Optional ResolvedScope; + + // Unresolved part of the scope. When the unresolved name is a specifier, we + // use the name that comes after it as the alternative name to resolve and use + // the specifier as the extra scope in the accessible scopes. + llvm::Optional UnresolvedScope; +}; + +// Extracts unresolved name and scope information around \p Unresolved. +// FIXME: try to merge this with the scope-wrangling code in CodeComplete. +llvm::Optional extractUnresolvedNameCheaply( + const SourceManager &SM, const DeclarationNameInfo &Unresolved, + CXXScopeSpec *SS, const LangOptions &LangOpts, bool UnresolvedIsSpecifier) { + bool Invalid = false; + llvm::StringRef Code = SM.getBufferData( + SM.getDecomposedLoc(Unresolved.getBeginLoc()).first, &Invalid); + if (Invalid) + return llvm::None; + CheapUnresolvedName Result; + Result.Name = Unresolved.getAsString(); + if (SS && SS->isNotEmpty()) { // "::" or "ns::" + if (auto *Nested = SS->getScopeRep()) { + if (Nested->getKind() == NestedNameSpecifier::Global) + Result.ResolvedScope = ""; + else if (const auto *NS = Nested->getAsNamespace()) { + auto SpecifiedNS = printNamespaceScope(*NS); + + // Check the specifier spelled in the source. + // If the resolved scope doesn't end with the spelled scope. The + // resolved scope can come from a sema typo correction. For example, + // sema assumes that "clangd::" is a typo of "clang::" and uses + // "clang::" as the specified scope in: + // namespace clang { clangd::X; } + // In this case, we use the "typo" specifier as extra scope instead + // of using the scope assumed by sema. + auto B = SM.getFileOffset(SS->getBeginLoc()); + auto E = SM.getFileOffset(SS->getEndLoc()); + std::string Spelling = (Code.substr(B, E - B) + "::").str(); + if (llvm::StringRef(SpecifiedNS).endswith(Spelling)) + Result.ResolvedScope = SpecifiedNS; + else + Result.UnresolvedScope = Spelling; + } else if (const auto *ANS = Nested->getAsNamespaceAlias()) { + Result.ResolvedScope = printNamespaceScope(*ANS->getNamespace()); + } 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 llvm::None; + } + } + } + + if (UnresolvedIsSpecifier) { + // If the unresolved name is a specifier e.g. + // clang::clangd::X + // ~~~~~~ + // We try to resolve clang::clangd::X instead of clang::clangd. + // FIXME: We won't be able to fix include if the specifier is what we + // should resolve (e.g. it's a class scope specifier). Collecting include + // headers for nested types could make this work. + + // Not using the end location as it doesn't always point to the end of + // identifier. + if (auto QualifiedByUnresolved = + qualifiedByUnresolved(SM, Unresolved.getBeginLoc(), LangOpts)) { + auto Split = splitQualifiedName(*QualifiedByUnresolved); + if (!Result.UnresolvedScope) + Result.UnresolvedScope.emplace(); + // If UnresolvedSpecifiedScope is already set, we simply append the + // extra scope. Suppose the unresolved name is "index" in the following + // example: + // namespace clang { clangd::index::X; } + // ~~~~~~ ~~~~~ + // "clangd::" is assumed to be clang:: by Sema, and we would have used + // it as extra scope. With "index" being a specifier, we append "index::" + // to the extra scope. + Result.UnresolvedScope->append((Result.Name + Split.first).str()); + Result.Name = Split.second; + } + } + return Result; +} + class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource { public: UnresolvedNameRecorder(llvm::Optional &LastUnresolvedName) @@ -192,51 +312,30 @@ if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc())) return clang::TypoCorrection(); - // 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 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(); - } - } - if (!SpecifiedScope && !S) // Give up if no scope available. + // This is not done lazily because `SS` can get out of scope and it's + // relatively cheap. + auto Extracted = extractUnresolvedNameCheaply( + SemaPtr->SourceMgr, Typo, SS, SemaPtr->LangOpts, + static_cast(LookupKind) == + Sema::LookupNameKind::LookupNestedNameSpecifierName); + if (!Extracted) return TypoCorrection(); - + auto CheapUnresolved = std::move(*Extracted); UnresolvedName Unresolved; - Unresolved.Name = Typo.getAsString(); + Unresolved.Name = CheapUnresolved.Name; Unresolved.Loc = Typo.getBeginLoc(); + if (!CheapUnresolved.ResolvedScope && !S) // Give up if no scope available. + return TypoCorrection(); + auto *Sem = SemaPtr; // Avoid capturing `this`. - Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() { + Unresolved.GetScopes = [Sem, CheapUnresolved, S, LookupKind]() { std::vector Scopes; - if (SpecifiedScope) { - Scopes.push_back(*SpecifiedScope); + if (CheapUnresolved.ResolvedScope) { + Scopes.push_back(*CheapUnresolved.ResolvedScope); } else { assert(S); - // No scope qualifier is specified. Collect all accessible scopes in the + // No scope specifier is specified. Collect all accessible scopes in the // context. VisitedContextCollector Collector; Sem->LookupVisibleDecls( @@ -249,6 +348,10 @@ if (isa(Ctx)) Scopes.push_back(printNamespaceScope(*Ctx)); } + + if (CheapUnresolved.UnresolvedScope) + for (auto &Scope : Scopes) + Scope.append(*CheapUnresolved.UnresolvedScope); return Scopes; }; LastUnresolvedName = std::move(Unresolved); Index: unittests/clangd/DiagnosticsTests.cpp =================================================================== --- unittests/clangd/DiagnosticsTests.cpp +++ unittests/clangd/DiagnosticsTests.cpp @@ -20,6 +20,7 @@ namespace clangd { namespace { +using testing::_; using testing::ElementsAre; using testing::Field; using testing::IsEmpty; @@ -369,6 +370,8 @@ $insert[[]]namespace ns { void foo() { $unqualified1[[X]] x; + // No fix if the unresolved type is used as specifier. (ns::)X::Nested will be + // considered the unresolved type. $unqualified2[[X]]::Nested n; } } @@ -391,10 +394,7 @@ AllOf(Diag(Test.range("unqualified1"), "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("unqualified2"), - "use of undeclared identifier 'X'"), - WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", - "Add include \"x.h\" for symbol ns::X"))), + Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"), AllOf(Diag(Test.range("qualified1"), "no type named 'X' in namespace 'ns'"), WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", @@ -487,6 +487,88 @@ } } +TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) { + Annotations Test(R"cpp( +$insert[[]]namespace ns { +} +void g() { ns::$[[scope]]::X_Y(); } + )cpp"); + auto TU = TestTU::withCode(Test.code()); + auto Index = buildIndexWithSymbol( + SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""}); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT( + TU.build().getDiagnostics(), + UnorderedElementsAre(AllOf( + Diag(Test.range(), "no member named 'scope' in namespace 'ns'"), + WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol ns::scope::X_Y"))))); +} + +TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) { + Annotations Test(R"cpp( +$insert[[]]namespace clang { +void f() { + // "clangd::" will be corrected to "clang::" by Sema. + $q1[[clangd]]::$x[[X]] x; + $q2[[clangd]]::$ns[[ns]]::Y y; +} +} + )cpp"); + auto TU = TestTU::withCode(Test.code()); + auto Index = buildIndexWithSymbol( + {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""}, + SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", "\"y.h\""}}); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT( + TU.build().getDiagnostics(), + UnorderedElementsAre( + AllOf( + Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; " + "did you mean 'clang'?"), + WithFix(_, // change clangd to clang + Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol clang::clangd::X"))), + AllOf( + Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"), + WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol clang::clangd::X"))), + AllOf( + Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; " + "did you mean 'clang'?"), + WithFix( + _, // change clangd to clangd + Fix(Test.range("insert"), "#include \"y.h\"\n", + "Add include \"y.h\" for symbol clang::clangd::ns::Y"))), + AllOf(Diag(Test.range("ns"), + "no member named 'ns' in namespace 'clang'"), + WithFix(Fix( + Test.range("insert"), "#include \"y.h\"\n", + "Add include \"y.h\" for symbol clang::clangd::ns::Y"))))); +} + +TEST(IncludeFixerTest, SpecifiedScopeIsNamespaceAlias) { + Annotations Test(R"cpp( +$insert[[]]namespace a {} +namespace b = a; +namespace c { + b::$[[X]] x; +} + )cpp"); + auto TU = TestTU::withCode(Test.code()); + auto Index = buildIndexWithSymbol( + SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""}); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT(TU.build().getDiagnostics(), + UnorderedElementsAre(AllOf( + Diag(Test.range(), "no type named 'X' in namespace 'a'"), + WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol a::X"))))); +} + } // namespace } // namespace clangd } // namespace clang