[clang-tools-extra] af071f0 - [clangd] Improve ObjC property handling in SelectionTree.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-01-28T11:07:19+01:00
New Revision: af071f03f379f7f1071e9da23ee4ca26d2c5c509

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

LOG: [clangd] Improve ObjC property handling in SelectionTree.

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

Reviewers: dgoldman

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/Selection.cpp
    clang-tools-extra/clangd/unittests/FindTargetTests.cpp
    clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 14c05d1c0b49..574b7477f23f 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -462,6 +462,14 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
              TraverseStmt(S->getRangeInit()) && TraverseStmt(S->getBody());
     });
   }
+  // OpaqueValueExpr blocks traversal, we must explicitly traverse it.
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+    return traverseNode(E, [&] { return TraverseStmt(E->getSourceExpr()); });
+  }
+  // We only want to traverse the *syntactic form* to understand the selection.
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+    return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); });
+  }
 
 private:
   using Base = RecursiveASTVisitor<SelectionVisitor>;

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 39ab8890bb68..f2659a96172b 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -536,7 +536,8 @@ TEST_F(TargetDeclTest, ObjC) {
       [[f.x]].y = 0;
     }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr", "@property(atomic, retain, readwrite) I *x");
+  EXPECT_DECLS("ObjCPropertyRefExpr",
+               "@property(atomic, retain, readwrite) I *x");
 
   Code = R"cpp(
     @protocol Foo

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 4860127c9c46..e8c2bd6dbca3 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,12 +323,52 @@ TEST(SelectionTest, CommonAncestor) {
             Foo x = [[^12_ud]];
           )cpp",
           "UserDefinedLiteral"},
+
       {
           R"cpp(
         int a;
         decltype([[^a]] + a) b;
         )cpp",
           "DeclRefExpr"},
+
+      // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+      // Need to traverse the contents of the OpaqueValueExpr to the POE,
+      // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
+      {
+          R"cpp(
+            @interface I{}
+            @property(retain) I*x;
+            @property(retain) I*y;
+            @end
+            void test(I *f) { [[^f]].x.y = 0; }
+          )cpp",
+          "DeclRefExpr"},
+      {
+          R"cpp(
+            @interface I{}
+            @property(retain) I*x;
+            @property(retain) I*y;
+            @end
+            void test(I *f) { [[f.^x]].y = 0; }
+          )cpp",
+          "ObjCPropertyRefExpr"},
+      // Examples with implicit properties.
+      {
+          R"cpp(
+            @interface I{}
+            -(int)foo;
+            @end
+            int test(I *f) { return 42 + [[^f]].foo; }
+          )cpp",
+          "DeclRefExpr"},
+      {
+          R"cpp(
+            @interface I{}
+            -(int)foo;
+            @end
+            int test(I *f) { return 42 + [[f.^foo]]; }
+          )cpp",
+          "ObjCPropertyRefExpr"},
   };
   for (const Case &C : Cases) {
     Annotations Test(C.Code);
@@ -339,6 +379,7 @@ TEST(SelectionTest, CommonAncestor) {
     // FIXME: Auto-completion in a template requires disabling delayed template
     // parsing.
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+    TU.ExtraArgs.push_back("-xobjective-c++");
 
     auto AST = TU.build();
     auto T = makeSelectionTree(C.Code, AST);