[clangd] Support include-fixer inside macro arguments.

[sammccall]

Summary

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

[thakis]

This breaks tests on windows: http://45.33.8.238/win/54910/step_9.txt

Please take a look and revert for now if it takes a while to fix.

[sammccall]

In D120619#3403021, @thakis wrote:

This breaks tests on windows: http://45.33.8.238/win/54910/step_9.txt

Please take a look and revert for now if it takes a while to fix.

Thanks, forced the test to pass in 31dc248ffc3cbf9ea6d11819b6b7b731b414b352.

The actual feature doesn't work on windows, and I'm not sure why (other than it's something under -fms-compatibility)

clang-tools-extra/clangd/IncludeFixer.cpp

 // 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;
+  // 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;
     auto IDTok = Lexer::findNextToken(CCTok->getLocation(), SM, LangOpts);
     if (!IDTok || !IDTok->is(tok::raw_identifier))
       break;
     Result.append(("::" + IDTok->getRawIdentifier()).str());
     NextLoc = IDTok->getLocation();
struct CheapUnresolvedName {
   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;
 };
 
+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 {
         // 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;
       }
     }

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

 
 namespace clang {
 namespace clangd {
 namespace {
 
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Contains;
+using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 ::testing::Matcher<const Diag &> withFix(::testing::Matcher<Fix> FixMatcher) {
EXPECT_THAT(
                 withFix(Fix(Test.range("insert"), "#include \"foo.h\"\n",
                             "Include \"foo.h\" for symbol ns::Foo"))),
           AllOf(Diag(Test.range("base"), "expected class name"),
                 diagName("expected_class_name"),
                 withFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
                             "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 {
 namespace nb {
 void foo() {
   $unqualified[[X]] x;
 }
 }