[Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[dgoldman]

Summary

Exempt ObjC from arrow/dot fixits since this has limited value for
Objective-C, where properties (referenced by dot syntax) are normally
backed by ivars (referenced by arrow syntax).

In addition, the current implementation doesn't properly mark
the fix it condition for Objective-C.

This was initially added in https://reviews.llvm.org/D41537
for C++ and then later C, don't believe the Objective-C changes
were intentional.

[dgoldman]

• Fix test run line

[dgoldman]

• Fix broken diff base (due to lint fixes maybe?)

[yvvan]

I'm not a big objC expert here. The idea looks fine to me and won't affect my workflow. So let's take this patch if nobody comments against it here.

[sammccall]

clang/lib/Sema/SemaCodeComplete.cpp

5153

IncludeObjC is always !AccessOpFixIt.hasValue() - use that directly with a suitable comment?

5255

This first sentence isn't accurate - they're included for C, and even in obj-c, just not for obj-c objects.
I'd suggest dropping it and also the note about alternative solutions, unless there's a reason we're likely to want to do that instead in the future.

[dgoldman]

Check AccessOpFixIt.hasValue()

[dgoldman]

Remove stale comment

clang/lib/Sema/SemaCodeComplete.cpp

5153

Ah good call, done

5255

Removed this in favor of a comment in the checks above. Think they're useful (albeit duplicated) since they give some explanation as to why.

[sammccall]

LG, thanks!

clang/lib/Sema/SemaCodeComplete.cpp

5199

I don't really follow what the comment means by "forwarding the fixit". As I understood it, the issue is -> and . are likely both valid, so changing one to the other is confusing. Might be able to reword to clarify.

[dgoldman]

• Minor comment fixes

clang/lib/Sema/SemaCodeComplete.cpp

5199

I meant the code previously would try give fix it code completions without marking it as a fix it. But I've gone ahead and removed it in favor of the other comment.

[dgoldman]

• Fix typo

clang/lib/Sema/SemaCodeComplete.cpp

void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
   }
 
   CodeCompletionContext CCContext(contextKind, ConvertedBaseType);
   CCContext.setPreferredType(PreferredType);
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(), CCContext,
                         &ResultBuilder::IsMember);
 
   auto DoCompletion = [&](Expr *Base, bool IsArrow,
                           Optional<FixItHint> AccessOpFixIt) -> bool {
     if (!Base)
       return false;
 
     ExprResult ConvertedBase = PerformMemberExprBaseConversion(Base, IsArrow);
     if (ConvertedBase.isInvalid())
       return false;
     Base = ConvertedBase.get();
if (RecordDecl *RD = getAsRecordDecl(BaseType)) {
         CodeCompletionResult Result(
             R.render(*this, CodeCompleter->getAllocator(),
                      CodeCompleter->getCodeCompletionTUInfo()));
         if (AccessOpFixIt)
           Result.FixIts.push_back(*AccessOpFixIt);
         Results.AddResult(std::move(Result));
       }
     } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
-      // Objective-C property reference.
+      // Objective-C property reference. Bail if we're performing fix-it code
+      // completion since Objective-C properties are normally backed by ivars,
+      // most Objective-C fix-its here would have little value.
+      if (AccessOpFixIt.hasValue()) {
+        return false;
+      }
       AddedPropertiesSet AddedProperties;
 
       if (const ObjCObjectPointerType *ObjCPtr =
               BaseType->getAsObjCInterfacePointerType()) {
         // Add property results based on our interface.
         assert(ObjCPtr && "Non-NULL pointer guaranteed above!");
         AddObjCProperties(CCContext, ObjCPtr->getInterfaceDecl(), true,
                           /*AllowNullaryMethods=*/true, CurContext,
                           AddedProperties, Results, IsBaseExprStatement);
       }
 
       // Add properties from the protocols in a qualified interface.
       for (auto *I : BaseType->castAs<ObjCObjectPointerType>()->quals())
         AddObjCProperties(CCContext, I, true, /*AllowNullaryMethods=*/true,
                           CurContext, AddedProperties, Results,
                           IsBaseExprStatement, /*IsClassProperty*/ false,
                           /*InOriginalClass*/ false);
     } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
                (!IsArrow && BaseType->isObjCObjectType())) {
-      // Objective-C instance variable access.
+      // Objective-C instance variable access. Bail if we're performing fix-it
+      // code completion since Objective-C properties are normally backed by
+      // ivars, most Objective-C fix-its here would have little value.
+      if (AccessOpFixIt.hasValue()) {
+        return false;
+      }
       ObjCInterfaceDecl *Class = nullptr;
       if (const ObjCObjectPointerType *ObjCPtr =
               BaseType->getAs<ObjCObjectPointerType>())
         Class = ObjCPtr->getInterfaceDecl();
       else
         Class = BaseType->castAs<ObjCObjectType>()->getInterface();
 
       // Add all ivars from this class and its superclasses.
auto DoCompletion = [&](Expr *Base, bool IsArrow,
     return true;
   };
 
   Results.EnterNewScope();
 
   bool CompletionSucceded = DoCompletion(Base, IsArrow, None);
   if (CodeCompleter->includeFixIts()) {
     const CharSourceRange OpRange =
         CharSourceRange::getTokenRange(OpLoc, OpLoc);
     CompletionSucceded |= DoCompletion(
         OtherOpBase, !IsArrow,
         FixItHint::CreateReplacement(OpRange, IsArrow ? "." : "->"));
   }
 
   Results.ExitScope();
 
   if (!CompletionSucceded)

clang/test/CodeCompletion/objc-member-access.m

+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+@interface TypeWithPropertiesBackedByIvars {
+  int _bar;
+  int _foo;
+}
+@property(nonatomic) int foo;
+@property(nonatomic) int bar;
+@end
+
+int getFoo(id object) {
+  TypeWithPropertiesBackedByIvars *model = (TypeWithPropertiesBackedByIvars *)object;
+  int foo = model.;
+  return foo;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:14:19 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1-NOT: [#int#]_bar
+// CHECK-CC1-NOT: [#int#]_foo
+// CHECK-CC1: [#int#]bar
+// CHECK-CC1: [#int#]foo