[clangd] Handle unresolved scope specifier when fixing includes.
Summary:
In the following examples, "clangd" is unresolved, and the fixer will try to fix
include for `clang::clangd`; however, clang::clangd::X is usually intended. So
when handling a qualifier that is unresolved, we change the unresolved name and
scopes so that the fixer will fix "clang::clangd::X" in the following example.
```
namespace clang {
clangd::X
~~~~~~
}
// or
clang::clangd::X
~~~~~~
```
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58185
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@354330 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/clangd/IncludeFixer.cpp b/clangd/IncludeFixer.cpp
index 9b9b6e1..b04cf21 100644
--- a/clangd/IncludeFixer.cpp
+++ b/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<std::string> 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<std::string> 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<std::string> UnresolvedScope;
+};
+
+// Extracts unresolved name and scope information around \p Unresolved.
+// FIXME: try to merge this with the scope-wrangling code in CodeComplete.
+llvm::Optional<CheapUnresolvedName> 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<UnresolvedName> &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<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();
- }
- }
- 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<Sema::LookupNameKind>(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<std::string> 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<NamespaceDecl>(Ctx))
Scopes.push_back(printNamespaceScope(*Ctx));
}
+
+ if (CheapUnresolved.UnresolvedScope)
+ for (auto &Scope : Scopes)
+ Scope.append(*CheapUnresolved.UnresolvedScope);
return Scopes;
};
LastUnresolvedName = std::move(Unresolved);
diff --git a/unittests/clangd/DiagnosticsTests.cpp b/unittests/clangd/DiagnosticsTests.cpp
index d110848..f6f6de3 100644
--- a/unittests/clangd/DiagnosticsTests.cpp
+++ b/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
