[clang-tools-extra] b857a50 - [clangd] Support include-fixer inside macro arguments.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-03-23T17:12:33+01:00
New Revision: b857a5092c5e07121426196e79d51400382ff27a

URL: https://github.com/llvm/llvm-project/commit/b857a5092c5e07121426196e79d51400382ff27a
DIFF: https://github.com/llvm/llvm-project/commit/b857a5092c5e07121426196e79d51400382ff27a.diff

LOG: [clangd] Support include-fixer inside macro arguments.

Motivating case: EXPECT_EQ(42, missingFunction(bar));

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/IncludeFixer.cpp
    clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp
index 66e9552709141..6f527a48a9c0e 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -343,8 +343,8 @@ llvm::Optional<std::string> qualifiedByUnresolved(const SourceManager &SM,
                                                   SourceLocation Loc,
                                                   const LangOptions &LangOpts) {
   std::string Result;
-
-  SourceLocation NextLoc = Loc;
+  // Accept qualifier written within macro arguments, but not macro bodies.
+  SourceLocation NextLoc = SM.getTopMacroCallerLoc(Loc);
   while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) {
     if (!CCTok->is(tok::coloncolon))
       break;
@@ -373,40 +373,46 @@ struct CheapUnresolvedName {
   llvm::Optional<std::string> UnresolvedScope;
 };
 
+llvm::Optional<std::string> getSpelledSpecifier(const CXXScopeSpec &SS,
+    const SourceManager &SM) {
+  // Support specifiers written within a single macro argument.
+  if (!SM.isWrittenInSameFile(SS.getBeginLoc(), SS.getEndLoc()))
+    return llvm::None;
+  SourceRange Range(SM.getTopMacroCallerLoc(SS.getBeginLoc()), SM.getTopMacroCallerLoc(SS.getEndLoc()));
+  if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID())
+    return llvm::None;
+
+  return (toSourceCode(SM, Range) + "::").str();
+}
+
 // 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)
+      if (Nested->getKind() == NestedNameSpecifier::Global) {
         Result.ResolvedScope = "";
-      else if (const auto *NS = Nested->getAsNamespace()) {
-        auto SpecifiedNS = printNamespaceScope(*NS);
+      } else if (const auto *NS = Nested->getAsNamespace()) {
+        std::string SpecifiedNS = printNamespaceScope(*NS);
+        llvm::Optional<std::string> Spelling = getSpelledSpecifier(*SS, SM);
 
         // 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,
+        // If the resolved scope doesn't end with the spelled scope, the
+        // resolved scope may 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;
+        if (!Spelling || llvm::StringRef(SpecifiedNS).endswith(*Spelling)) {
+          Result.ResolvedScope = std::move(SpecifiedNS);
+        } else {
+          Result.UnresolvedScope = std::move(*Spelling);
+        }
       } else if (const auto *ANS = Nested->getAsNamespaceAlias()) {
         Result.ResolvedScope = printNamespaceScope(*ANS->getNamespace());
       } else {

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index a1b66b43a87b8..f62b452353dcb 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -36,6 +36,7 @@ namespace {
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Contains;
+using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
@@ -1094,6 +1095,24 @@ using Type = ns::$template[[Foo]]<int>;
                             "Include \"x.h\" for symbol ns::X")))));
 }
 
+TEST(IncludeFixerTest, TypoInMacro) {
+  auto TU = TestTU::withCode(R"cpp(// error-ok
+#define ID(T) T
+X a1;
+ID(X a2);
+ns::X a3;
+ID(ns::X a4);
+namespace ns{};
+ns::X a5;
+ID(ns::X a6);
+)cpp");
+  auto Index = buildIndexWithSymbol(
+      {SymbolWithHeader{"X", "unittest:///x.h", "\"x.h\""},
+       SymbolWithHeader{"ns::X", "unittest:///ns.h", "\"x.h\""}});
+  TU.ExternalIndex = Index.get();
+  EXPECT_THAT(*TU.build().getDiagnostics(), Each(withFix(_)));
+}
+
 TEST(IncludeFixerTest, MultipleMatchedSymbols) {
   Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace na {