[clang-tools-extra] d5c022d - [clangd][ObjC] Support nullability annotations

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2020-10-20T17:36:32-04:00
New Revision: d5c022d846999127352aa3c08c0f81418a942aab

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

LOG: [clangd][ObjC] Support nullability annotations

Nullability annotations are implmented using attributes; previusly
clangd would skip over AttributedTypeLoc since their location
points to the attribute instead of the modified type.

Also add some test cases for this.

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/Selection.cpp
    clang-tools-extra/clangd/unittests/FindTargetTests.cpp
    clang-tools-extra/clangd/unittests/HoverTests.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 5c08485d8b92..fbd72be320a7 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -605,6 +605,10 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
   bool canSafelySkipNode(const DynTypedNode &N) {
     SourceRange S = N.getSourceRange();
     if (auto *TL = N.get<TypeLoc>()) {
+      // FIXME: TypeLoc::getBeginLoc()/getEndLoc() are pretty fragile
+      // heuristics. We should consider only pruning critical TypeLoc nodes, to
+      // be more robust.
+
       // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
       // failing
       // to descend into the child expression.
@@ -616,6 +620,10 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
       // rid of this patch.
       if (auto DT = TL->getAs<DecltypeTypeLoc>())
         S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+      // AttributedTypeLoc may point to the attribute's range, NOT the modified
+      // type's range.
+      if (auto AT = TL->getAs<AttributedTypeLoc>())
+        S = AT.getModifiedLoc().getSourceRange();
     }
     if (!SelChecker.mayHit(S)) {
       dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index a68ea1d6a3dc..dd7e9878a6d5 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -831,6 +831,24 @@ TEST_F(TargetDeclTest, ObjC) {
   EXPECT_DECLS("ObjCPropertyRefExpr",
                "@property(atomic, retain, readwrite) I *x");
 
+  Code = R"cpp(
+    @interface MYObject
+    @end
+    @interface Interface
+    @property(retain) [[MYObject]] *x;
+    @end
+  )cpp";
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject");
+
+  Code = R"cpp(
+    @interface MYObject2
+    @end
+    @interface Interface
+    @property(retain, nonnull) [[MYObject2]] *x;
+    @end
+  )cpp";
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject2");
+
   Code = R"cpp(
     @protocol Foo
     @end

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 48c0fef45ab8..87a1b42315b3 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1991,6 +1991,34 @@ TEST(Hover, All) {
             HI.NamespaceScope = "ObjC::"; // FIXME: fix it
             HI.Definition = "char data";
           }},
+      {
+          R"cpp(
+          @interface MYObject
+          @end
+          @interface Interface
+          @property(retain) [[MYOb^ject]] *x;
+          @end
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "MYObject";
+            HI.Kind = index::SymbolKind::Class;
+            HI.NamespaceScope = "";
+            HI.Definition = "@interface MYObject\n at end";
+          }},
+      {
+          R"cpp(
+          @interface MYObject
+          @end
+          @interface Interface
+          - (void)doWith:([[MYOb^ject]] *)object;
+          @end
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "MYObject";
+            HI.Kind = index::SymbolKind::Class;
+            HI.NamespaceScope = "";
+            HI.Definition = "@interface MYObject\n at end";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 2026292bc6f3..55b05ebe11ab 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -356,6 +356,22 @@ TEST(SelectionTest, CommonAncestor) {
         )cpp",
           "DeclRefExpr"},
 
+      // Objective-C nullability attributes.
+      {
+          R"cpp(
+            @interface I{}
+            @property(nullable) [[^I]] *x;
+            @end
+          )cpp",
+          "ObjCInterfaceTypeLoc"},
+      {
+          R"cpp(
+            @interface I{}
+            - (void)doSomething:(nonnull [[i^d]])argument;
+            @end
+          )cpp",
+          "TypedefTypeLoc"},
+
       // 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.