[clangd] Improve XRefs support for ObjCMethodDecl

[dgoldman]

Summary

• Correct nameLocation to point to the first selector fragment instead of the - or +

• getDefinition now searches through the proper impl decls to find the definition of the ObjCMethodDecl if one exists

[dgoldman]

Run clang-format

[dgoldman]

Minor grammar update to comment

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/XRefs.cpp

89

can you have a comment here saying "Look for method definition inside the implementation decl"

clang-tools-extra/clangd/AST.h

 /// locations (e.g. file name, offset in file).
 /// FIXME: the USR semantics might not be stable enough as the ID for index
 /// macro (e.g. a change in definition offset can result in a different USR). We
 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
 SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
                      const SourceManager &SM);
 
+/// Return the corresponding implementation/definition for the given ObjC
+/// container if it has one, otherwise, return nullptr.
+///
+/// Objective-C classes can have three types of declarations:
+///
+/// - forward declaration: @class MyClass;
+/// - true declaration (interface definition): @interface MyClass ... @end
+/// - true definition (implementation): @implementation MyClass ... @end
+///
+/// Objective-C categories are extensions on classes:
+///
+/// - declaration: @interface MyClass (Ext) ... @end
+/// - definition: @implementation MyClass (Ext) ... @end
+///
+/// With one special case, a class extension, which is normally used to keep
+/// some declarations internal to a file without exposing them in a header.
+///
+/// - class extension declaration: @interface MyClass () ... @end
+/// - which really links to class definition: @implementation MyClass ... @end
+///
+/// For Objective-C protocols, e.g. @protocol MyProtocol ... @end this will
+/// return nullptr as protocols don't have an implementation.
+const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D);
+
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like anonymous/inline namespace.
 std::string printType(const QualType QT, const DeclContext &CurContext,
                       llvm::StringRef Placeholder = "");
 
 /// Indicates if \p D is a template instantiation implicitly generated by the
 /// compiler, e.g.
 ///     template <class T> struct vector {};

clang-tools-extra/clangd/AST.cpp

 
 bool isImplementationDetail(const Decl *D) {
   return !isSpelledInSource(D->getLocation(),
                             D->getASTContext().getSourceManager());
 }
 
 SourceLocation nameLocation(const clang::Decl &D, const SourceManager &SM) {
   auto L = D.getLocation();
+  // For `- (void)foo` we want `foo` not the `-`.
+  if (const auto *MD = dyn_cast<ObjCMethodDecl>(&D))
+    L = MD->getSelectorStartLoc();
   if (isSpelledInSource(L, SM))
     return SM.getSpellingLoc(L);
   return SM.getExpansionLoc(L);
 }
 
 std::string printQualifiedName(const NamedDecl &ND) {
   std::string QName;
   llvm::raw_string_ostream OS(QName);
SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
   if (MI == nullptr)
     return {};
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
     return {};
   return SymbolID(USR);
 }
 
+const ObjCImplDecl *getCorrespondingObjCImpl(const ObjCContainerDecl *D) {
+  if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(D))
+    return ID->getImplementation();
+  if (const auto *CD = dyn_cast<ObjCCategoryDecl>(D)) {
+    if (CD->IsClassExtension()) {
+      if (const auto *ID = CD->getClassInterface())
+        return ID->getImplementation();
+      return nullptr;
+    }
+    return CD->getImplementation();
+  }
+  return nullptr;
+}
+
 std::string printType(const QualType QT, const DeclContext &CurContext,
                       const llvm::StringRef Placeholder) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
   PP.SuppressTagKeyword = true;
   PP.SuppressUnwrittenScope = true;
 

clang-tools-extra/clangd/XRefs.cpp

if (const auto *TD = dyn_cast<TagDecl>(D))
     return TD->getDefinition();
   if (const auto *VD = dyn_cast<VarDecl>(D))
     return VD->getDefinition();
   if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return FD->getDefinition();
   if (const auto *CTD = dyn_cast<ClassTemplateDecl>(D))
     if (const auto *RD = CTD->getTemplatedDecl())
       return RD->getDefinition();
-  // Objective-C classes can have three types of declarations:
-  //
-  // - forward declaration: @class MyClass;
-  // - true declaration (interface definition): @interface MyClass ... @end
-  // - true definition (implementation): @implementation MyClass ... @end
-  //
-  // Objective-C categories are extensions are on classes:
-  //
-  // - declaration: @interface MyClass (Ext) ... @end
-  // - definition: @implementation MyClass (Ext) ... @end
-  //
-  // With one special case, a class extension, which is normally used to keep
-  // some declarations internal to a file without exposing them in a header.
-  //
-  // - class extension declaration: @interface MyClass () ... @end
-  // - which really links to class definition: @implementation MyClass ... @end
-  if (const auto *ID = dyn_cast<ObjCInterfaceDecl>(D))
-    return ID->getImplementation();
-  if (const auto *CD = dyn_cast<ObjCCategoryDecl>(D)) {
-    if (CD->IsClassExtension()) {
-      if (const auto *ID = CD->getClassInterface())
-        return ID->getImplementation();
+  if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
+    if (MD->isThisDeclarationADefinition())
+      return MD;
+    // Look for the method definition inside the implementation decl.
+    auto *DeclCtx = cast<Decl>(MD->getDeclContext());
+    if (DeclCtx->isInvalidDecl())
       return nullptr;
-    }
-    return CD->getImplementation();
-  }
+
+    if (const auto *CD = dyn_cast<ObjCContainerDecl>(DeclCtx))
+      if (const auto *Impl = getCorrespondingObjCImpl(CD))
+        return Impl->getMethod(MD->getSelector(), MD->isInstanceMethod());
+  if (const auto *CD = dyn_cast<ObjCContainerDecl>(D))
+    return getCorrespondingObjCImpl(CD);
   // Only a single declaration is allowed.
   if (isa<ValueDecl>(D) || isa<TemplateTypeParmDecl>(D) ||
       isa<TemplateTemplateParmDecl>(D)) // except cases above
     return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
 }
 

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

const char *Tests[] = {
         @class Foo;
         @interface $decl[[Foo]]
         @end
         @implementation $def[[Foo]]
         @end
         Fo^o * getFoo() {
           return 0;
         }
+      )objc",
+
+      R"objc(// Method decl and definition for ObjC class.
+        @interface Cat
+        - (void)$decl[[meow]];
+        @end
+        @implementation Cat
+        - (void)$def[[meow]] {}
+        @end
+        void makeNoise(Cat *kitty) {
+          [kitty me^ow];
+        }
+      )objc",
+
+      R"objc(// Method decl and definition for ObjC category.
+        @interface Dog
+        @end
+        @interface Dog (Play)
+        - (void)$decl[[runAround]];
+        @end
+        @implementation Dog (Play)
+        - (void)$def[[runAround]] {}
+        @end
+        void play(Dog *dog) {
+          [dog run^Around];
+        }
+      )objc",
+
+      R"objc(// Method decl and definition for ObjC class extension.
+        @interface Dog
+        @end
+        @interface Dog ()
+        - (void)$decl[[howl]];
+        @end
+        @implementation Dog
+        - (void)$def[[howl]] {}
+        @end
+        void play(Dog *dog) {
+          [dog ho^wl];
+        }
       )objc"};
   for (const char *Test : Tests) {
     Annotations T(Test);
     llvm::Optional<Range> WantDecl;
     llvm::Optional<Range> WantDef;
     if (!T.ranges().empty())
       WantDecl = WantDef = T.range();
     if (!T.ranges("decl").empty())