[clangd] Fix highlighting for implicit ObjC property refs

[dgoldman]

Summary

Objective-C lets you use the self.prop syntax as sugar for both
[self prop] and [self setProp:], but clangd previously did not
provide a semantic token for prop.

Now, we provide a semantic token, treating it like a normal property
except it's backed by a ObjCMethodDecl instead of a
ObjCPropertyDecl.

[kadircet]

clang-tools-extra/clangd/SemanticHighlighting.cpp

579

is there any difference to using one or the other ? (i.e. can setter be static while getter isn't? I suppose not). maybe mention that in the comment and say that we are choosing whichever exists (and change the logic below to if followed by an else if?

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

699

we have an explicit @property here, but comments in the implementation says otherwise. did you mean not having any explicit getter/setter or @synthesize statement ?

[sammccall]

Drive by from vacation, sorry - can this be tackled "upstream" in findExplicitReferences rather than special cased in syntax highlighting?

That would help with xrefs, rename etc. Features should really be only handled here if they're somehow not a simple token->decl reference, but this seems to be.

[dgoldman]

In D104117#2835599, @sammccall wrote:

Drive by from vacation, sorry - can this be tackled "upstream" in findExplicitReferences rather than special cased in syntax highlighting?

That would help with xrefs, rename etc. Features should really be only handled here if they're somehow not a simple token->decl reference, but this seems to be.

There's a couple of problems with that at the moment:

• Currently all ObjC method handling is done here since ObjC has selectors which need to be special cased
• FindExplicitRefs right now resolves these down to their ObjCMethodDecls, losing syntactic information that they were referenced by property expressions and not method expressions. We'd have to store that somehow to note these as Field and not Method.

clang-tools-extra/clangd/SemanticHighlighting.cpp

579

Not like that - but it's technically possible you could have a getter in an SDK and a user-provided setter for it (but that would be really weird); both of them can exist.

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

699

After some testing it seems like property refs to class properties always use the implicit form like ObjCPropertyRefExpr 0x7f87cb8116f0 <col:11, col:15> '<pseudo-object type>' lvalue objcproperty Kind=MethodRef Getter="sharedInstance" Setter="(null)" Messaging=Getter

[dgoldman]

Minor comment fixes

[dgoldman]

Fix clang-tidy warning

clang-tools-extra/clangd/SemanticHighlighting.cpp

bool VisitObjCMessageExpr(ObjCMessageExpr *OME) {
     bool DefaultLibrary = false;
     if (ObjCMethodDecl *OMD = OME->getMethodDecl())
       DefaultLibrary = isDefaultLibrary(OMD);
     highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage(),
                           DefaultLibrary);
     return true;
   }
 
+  // Objective-C allows you to use property syntax `self.prop` as sugar for
+  // `[self prop]` and `[self setProp:]` when there's no explicit `@property`
+  // for `prop` as well as for class properties. We treat this like a property
+  // even though semantically it's equivalent to a method expression.
+  void highlightObjCImplicitPropertyRef(const ObjCMethodDecl *OMD,
+                                        SourceLocation Loc) {
+    auto &Tok = H.addToken(Loc, HighlightingKind::Field)
+                    .addModifier(HighlightingModifier::ClassScope);
+    if (OMD->isClassMethod())
+      Tok.addModifier(HighlightingModifier::Static);
+    if (isDefaultLibrary(OMD))
+      Tok.addModifier(HighlightingModifier::DefaultLibrary);
+  }
+
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *OPRE) {
+    // We need to handle implicit properties here since they will appear to
+    // reference `ObjCMethodDecl` via an implicit `ObjCMessageExpr`, so normal
+    // highlighting will not work.
+    if (!OPRE->isImplicitProperty())
+      return true;
+    // A single property expr can reference both a getter and setter, but we can
+    // only provide a single semantic token, so prefer the getter. In most cases
+    // the end result should be the same, although it's technically possible
+    // that the user defines a setter for a system SDK.
+    if (OPRE->isMessagingGetter()) {
+      highlightObjCImplicitPropertyRef(OPRE->getImplicitPropertyGetter(),
+                                       OPRE->getLocation());
+      return true;
+    } else if (OPRE->isMessagingSetter()) {
+      highlightObjCImplicitPropertyRef(OPRE->getImplicitPropertySetter(),
+                                       OPRE->getLocation());
+    }
+    return true;
+  }
+
   bool VisitOverloadExpr(OverloadExpr *E) {
     if (!E->decls().empty())
       return true; // handled by findExplicitReferences.
     auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::Unknown)
                     .addModifier(HighlightingModifier::DependentName);
     if (llvm::isa<UnresolvedMemberExpr>(E))
       Tok.addModifier(HighlightingModifier::ClassScope);
     // other case is UnresolvedLookupExpr, scope is unknown.

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

)cpp",
         @end
       )cpp",
       R"cpp(
         // ObjC: Properties and Ivars.
         @interface $Class_decl[[Foo]] {
           int $Field_decl[[_someProperty]];
         }
         @property(nonatomic, assign) int $Field_decl[[someProperty]];
+        @property(readonly, class) $Class[[Foo]] *$Field_decl_readonly_static[[sharedInstance]];
         @end
         @implementation $Class_decl[[Foo]]
         @synthesize someProperty = _someProperty;
+        - (int)$Method_decl[[otherMethod]] {
+          return 0;
+        }
         - (int)$Method_decl[[doSomething]] {
-          self.$Field[[someProperty]] = self.$Field[[someProperty]] + 1;
+          $Class[[Foo]].$Field_static[[sharedInstance]].$Field[[someProperty]] = 1;
+          self.$Field[[someProperty]] = self.$Field[[someProperty]] + self.$Field[[otherMethod]] + 1;
           self->$Field[[_someProperty]] = $Field[[_someProperty]] + 1;
         }
         @end
       )cpp",
       // Member imported from dependent base
       R"cpp(
         template <typename> struct $Class_decl[[Base]] {
           int $Field_decl[[member]];