[clangd] IncludeFixer: resolve Class in ns::Class::method();

[sammccall]

Summary

We distinguish by case, assuming a::b:: is a namespace and a::B:: is a class.
Crude, but be a significant improvement over assuming it's always a namespace.

(Motivating case from clangd: clangd::CommandMangler::detect()).

[sammccall]

Friendly ping :-)

clang-tools-extra/clangd/IncludeFixer.cpp

if (auto *Nested = SS->getScopeRep()) {
       } 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.
-
+  // If the unresolved name is a namespace qualifier e.g.
+  // we want to resolve clang::clangd::X instead of clang::clangd.
+  //
+  // On the other hand if it's a class name qualifier e.g.
+  //      clang::PrecompiledPreamble::Build()
+  //             ~~~~~~~~~~~~~~~~~~~
+  // then we should resolve the class clang::PrecompiledPreamble.
+  // 
+  // We simply guess based on the case of the identifier. Namespaces are almost
+  // universally lowercase, while class names are often uppercase.
+  assert(!Result.Name.empty());
+  if (UnresolvedIsSpecifier && !isUppercase(Result.Name.front())) {
     // 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

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

if (D.Fixes.size() != 1) {
       continue;
     }
     EXPECT_EQ(D.Fixes[0].Message, std::string("Include \"a.h\" for symbol X"));
   }
 }
 
 TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
   Annotations Test(R"cpp(// error-ok
-$insert[[]]namespace ns {
+$insert[[]]namespace ns {}
+void g() {
+  ns::$lower[[scope]]::X_Y();
+  ns::$upper[[Scope]]::X_Y();
 }
-void g() {  ns::$[[scope]]::X_Y();  }
   )cpp");
   TestTU TU;
   TU.Code = std::string(Test.code());
   // FIXME: Figure out why this is needed and remove it, PR43662.
   TU.ExtraArgs.push_back("-fno-ms-compatibility");
   auto Index = buildIndexWithSymbol(
-      SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+      {SymbolWithHeader{"ns::scope::X_Y", "unittest:///ns.h", "\"ns.h\""},
+       SymbolWithHeader{"ns::Scope", "unittest:///class.h", "\"class.h\""}});
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
       UnorderedElementsAre(
-          AllOf(Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+          AllOf(Diag(Test.range("lower"),
+                     "no member named 'scope' in namespace 'ns'"),
                 diagName("no_member"),
-                withFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-                            "Include \"x.h\" for symbol ns::scope::X_Y")))));
+                withFix(Fix(Test.range("insert"), "#include \"ns.h\"\n",
+                            "Include \"ns.h\" for symbol ns::scope::X_Y"))),
+          AllOf(Diag(Test.range("upper"),
+                     "no member named 'Scope' in namespace 'ns'"),
+                diagName("no_member"),
+                withFix(Fix(Test.range("insert"), "#include \"class.h\"\n",
+                            "Include \"class.h\" for symbol ns::Scope")))));
 }
 
 TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
   Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace clang {
 void f() {
   // "clangd::" will be corrected to "clang::" by Sema.
   $q1[[clangd]]::$x[[X]] x;