[clangd] Fix early selection for non-vardecl declarators

[kadircet]

Summary

Selection tree was performing an early claim only for VarDecls, but
there are other cases where we can have declarators, e.g. FieldDecls. This patch
extends the early claim logic to all types of declarators.

Fixes https://github.com/clangd/clangd/issues/292

[sammccall]

clang-tools-extra/clangd/Selection.cpp

613

I think we can combine these two cases.
The weird cases handled by FunctionDecl are actually a bit wrong at the moment (early claiming types).

• conversion operator: we should probably be claiming operator only, which hapepns to be getLocation()
• destructor: we should be claiming ~ only, which is also getLocation()
• constructor: we shouldn't be claiming anything, need to blacklist
• all the others seem fine to be handled by the DeclaratorDecl case

This changes more behaviour (and the changes are likely to be seen as regressions in some cases, though more consistent) so we could also consider just landing this as-is.

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

374

I'd suggest using digraphs instead, if it works:

int hash<:32:>

[kadircet]

• Address comments

[sammccall]

clang-tools-extra/clangd/Selection.cpp

612

we may have to blacklist deduction guides too?

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

372

can you add the other special cases we discussed offline? conversion operations should be triggered by operator, destructors by ~.

[kadircet]

• Address comments

[sammccall]

clang-tools-extra/clangd/Selection.cpp

609

can you check this at HEAD?
IIRC this was related to the reason kythe was failing to report constructors as references to the class, which is now fixed. But I don't know if they fixed the AST or worked around it.

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

342 ↗

(On Diff #246635)

want to keep the conversion operator test too? (by pointing at operator)

[kadircet]

• Add the old test for hover over operator back.

clang-tools-extra/clangd/Selection.cpp

609

nope, seems like still broken.

clang-tools-extra/clangd/Selection.cpp

 //===--- Selection.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Selection.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
void pop() {
     Stack.pop();
   }
 
   // Returns the range of tokens that this node will claim directly, and
   // is not available to the node's children.
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
     if (const Decl *D = N.get<Decl>()) {
+      // FunctionDecl is also a DeclaratorDecl, but getLocation doesn't work for
+      // operator overloads, e.g. operator [[int]](); Therefore we handle them
+      // explicitly.
       // void [[foo]]();
       if (auto *FD = llvm::dyn_cast<FunctionDecl>(D))
         return FD->getNameInfo().getSourceRange();
       // int (*[[s]])();
-      else if (auto *VD = llvm::dyn_cast<VarDecl>(D))
-        return VD->getLocation();
+      if (auto *DD = llvm::dyn_cast<DeclaratorDecl>(D))
+        return DD->getLocation();
     } else if (const auto* CCI = N.get<CXXCtorInitializer>()) {
       // : [[b_]](42)
       return CCI->getMemberLocation();
     }
     return SourceRange();
   }
 
   // Perform hit-testing of a complete Node against the selection.

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

Case Cases[] = {
       {
           R"cpp(
             @interface I{}
             -(int)foo;
             @end
             int test(I *f) { return 42 + [[f.^foo]]; }
           )cpp",
           "ObjCPropertyRefExpr"},
+      {
+          R"cpp(
+            // We've got this nice trick, as annotation library eagerly selects
+            // the range and if we've got [32] below, there's no way to select
+            // the full range.
+            #define X [32]
+            struct foo {
+              [[int has^h X]];
+            };
+          )cpp",
+          "FieldDecl"},
   };
   for (const Case &C : Cases) {
     Annotations Test(C.Code);
 
     TestTU TU;
     TU.Code = std::string(Test.code());
 
     // FIXME: Auto-completion in a template requires disabling delayed template