[clangd] SelectionTree should prefer lexical declcontext

[kadircet]

Summary

This is important especially for code that tries to traverse scopes as
written in code, which is the contract SelectionTree tries to satisfy.

Related https://github.com/clangd/clangd/issues/900.

[kadircet]

• Related https://github.com/clangd/clangd/issues/900.

[sammccall]

clang-tools-extra/clangd/Selection.cpp

893

Maybe insert the word "lexical" into the doc comment for this function

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

742

Nit: call the case DeclContextIsLexical to make the point clearer?

751

I'd probably use makeSelectionTree(Test.code(), AST) for brevity and only test the #2 case, but up to you

[kadircet]

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

751

i'd rather keep both, just to make sure we are not accidentally regressing the other case.

clang-tools-extra/clangd/Selection.h

struct Node {
     // The parent within the selection tree. nullptr for TranslationUnitDecl.
     Node *Parent;
     // Direct children within the selection tree.
     llvm::SmallVector<const Node *> Children;
     // The corresponding node from the full AST.
     DynTypedNode ASTNode;
     // The extent to which this node is covered by the selection.
     Selection Selected;
-    // Walk up the AST to get the DeclContext of this Node,
-    // which is not the node itself.
+    // Walk up the AST to get the lexical DeclContext of this Node, which is not
+    // the node itself.
     const DeclContext &getDeclContext() const;
     // Printable node kind, like "CXXRecordDecl" or "AutoTypeLoc".
     std::string kind() const;
     // If this node is a wrapper with no syntax (e.g. implicit cast), return
     // its contents. (If multiple wrappers are present, unwraps all of them).
     const Node &ignoreImplicit() const;
     // If this node is inside a wrapper with no syntax (e.g. implicit cast),
     // return that wrapper. (If multiple are present, unwraps all of them).

clang-tools-extra/clangd/Selection.cpp

const Node *SelectionTree::commonAncestor() const {
   while (Ancestor->Children.size() == 1 && !Ancestor->Selected)
     Ancestor = Ancestor->Children.front();
   // Returning nullptr here is a bit unprincipled, but it makes the API safer:
   // the TranslationUnitDecl contains all of the preamble, so traversing it is a
   // performance cliff. Callers can check for null and use root() if they want.
   return Ancestor != Root ? Ancestor : nullptr;
 }
 
 const DeclContext &SelectionTree::Node::getDeclContext() const {
   for (const Node *CurrentNode = this; CurrentNode != nullptr;
        CurrentNode = CurrentNode->Parent) {
     if (const Decl *Current = CurrentNode->ASTNode.get<Decl>()) {
       if (CurrentNode != this)
         if (auto *DC = dyn_cast<DeclContext>(Current))
           return *DC;
-      return *Current->getDeclContext();
+      return *Current->getLexicalDeclContext();
     }
   }
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
 
 const SelectionTree::Node &SelectionTree::Node::ignoreImplicit() const {
   if (Children.size() == 1 &&
       getSourceRange(Children.front()->ASTNode) == getSourceRange(ASTNode))

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

SelectionTree::createEach(AST.getASTContext(), AST.getTokens(),
                             Test.point("unique"), Test.point("ambiguous"),
                             [&](SelectionTree T) {
                               ++Seen;
                               return false;
                             });
   EXPECT_EQ(1u, Seen) << "one tree for nontrivial selection";
 }
 
+TEST(SelectionTest, DeclContextIsLexical) {
+  llvm::Annotations Test("namespace a { void $1^foo(); } void a::$2^foo();");
+  auto AST = TestTU::withCode(Test.code()).build();
+  {
+    auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+                                         Test.point("1"), Test.point("1"));
+    EXPECT_FALSE(ST.commonAncestor()->getDeclContext().isTranslationUnit());
+  }
+  {
+    auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+                                         Test.point("2"), Test.point("2"));
+    EXPECT_TRUE(ST.commonAncestor()->getDeclContext().isTranslationUnit());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang