[CodeCompletion] Add a block property setter completion result

This commit changes code completion results for Objective-C block properties:
clang now suggests an additional completion result that displays the block
property together with '=' and the block literal placeholder for the appropriate
readwrite block properties.

This commit uses a simple heuristic to determine when it's appropriate to
suggest a setter completion for block properties: the additional block setter
completion is provided iff the member access that's being completed is a
standalone statement.

rdar://28481726

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

llvm-svn: 284472

7 files changed, 153 insertions, 28 deletions

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 3ecbd620061..3b2341c917a 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -247,6 +247,11 @@ class Parser : public CodeCompletionHandler {
 
   bool SkipFunctionBodies;
 
+  /// The location of the expression statement that is being parsed right now.
+  /// Used to determine if an expression that is being parsed is a statement or
+  /// just a regular sub-expression.
+  SourceLocation ExprStatementTokLoc;
+
 public:
   Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies);
   ~Parser() override;
diff --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h
index 2ef6f107c7d..b80924ea11f 100644
--- a/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -90,7 +90,11 @@ enum {
   CCD_ProbablyNotObjCCollection = 15,
 
   /// \brief An Objective-C method being used as a property.
-  CCD_MethodAsProperty = 2
+  CCD_MethodAsProperty = 2,
+
+  /// \brief An Objective-C block property completed as a setter with a
+  /// block placeholder.
+  CCD_BlockPropertySetter = 3
 };
 
 /// \brief Priority value factors by which we will divide or multiply the
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6e0f3797643..e91b7756278 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9538,8 +9538,8 @@ public:
   void CodeCompleteExpression(Scope *S,
                               const CodeCompleteExpressionData &Data);
   void CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
-                                       SourceLocation OpLoc,
-                                       bool IsArrow);
+                                       SourceLocation OpLoc, bool IsArrow,
+                                       bool IsBaseExprStatement);
   void CodeCompletePostfixExpression(Scope *S, ExprResult LHS);
   void CodeCompleteTag(Scope *S, unsigned TagSpec);
   void CodeCompleteTypeQualifiers(DeclSpec &DS);
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 3788b18f073..b9988bb046e 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1646,9 +1646,10 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
 
       if (Tok.is(tok::code_completion)) {
         // Code completion for a member access expression.
-        Actions.CodeCompleteMemberReferenceExpr(getCurScope(), LHS.get(),
-                                                OpLoc, OpKind == tok::arrow);
-        
+        Actions.CodeCompleteMemberReferenceExpr(
+            getCurScope(), LHS.get(), OpLoc, OpKind == tok::arrow,
+            ExprStatementTokLoc == LHS.get()->getLocStart());
+
         cutOffParsing();
         return ExprError();
       }
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index d0557b8a62b..30e392fa3c9 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -396,6 +396,8 @@ StmtResult Parser::ParseExprStatement() {
   // If a case keyword is missing, this is where it should be inserted.
   Token OldToken = Tok;
 
+  ExprStatementTokLoc = Tok.getLocation();
+
   // expression[opt] ';'
   ExprResult Expr(ParseExpression());
   if (Expr.isInvalid()) {
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 6000fc66663..a0e79f6c376 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -2212,6 +2212,7 @@ static void findTypeLocationForBlockDecl(const TypeSourceInfo *TSInfo,
 static std::string
 formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
                        FunctionTypeLoc &Block, FunctionProtoTypeLoc &BlockProto,
+                       bool SuppressBlockName = false,
                        bool SuppressBlock = false,
                        Optional<ArrayRef<QualType>> ObjCSubsts = None);
 
@@ -2277,7 +2278,8 @@ static std::string FormatFunctionParameter(const PrintingPolicy &Policy,
     
   // We have the function prototype behind the block pointer type, as it was
   // written in the source.
-  return formatBlockPlaceholder(Policy, Param, Block, BlockProto, SuppressBlock,
+  return formatBlockPlaceholder(Policy, Param, Block, BlockProto,
+                                /*SuppressBlockName=*/false, SuppressBlock,
                                 ObjCSubsts);
 }
 
@@ -2293,7 +2295,7 @@ static std::string FormatFunctionParameter(const PrintingPolicy &Policy,
 static std::string
 formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
                        FunctionTypeLoc &Block, FunctionProtoTypeLoc &BlockProto,
-                       bool SuppressBlock,
+                       bool SuppressBlockName, bool SuppressBlock,
                        Optional<ArrayRef<QualType>> ObjCSubsts) {
   std::string Result;
   QualType ResultType = Block.getTypePtr()->getReturnType();
@@ -2329,7 +2331,7 @@ formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
   if (SuppressBlock) {
     // Format as a parameter.
     Result = Result + " (^";
-    if (BlockDecl->getIdentifier())
+    if (!SuppressBlockName && BlockDecl->getIdentifier())
       Result += BlockDecl->getIdentifier()->getName();
     Result += ")";
     Result += Params;
@@ -2338,7 +2340,7 @@ formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
     Result = '^' + Result;
     Result += Params;
 
-    if (BlockDecl->getIdentifier())
+    if (!SuppressBlockName && BlockDecl->getIdentifier())
       Result += BlockDecl->getIdentifier()->getName();
   }
 
@@ -3611,21 +3613,59 @@ static ObjCContainerDecl *getContainerDef(ObjCContainerDecl *Container) {
 
 static void AddObjCProperties(const CodeCompletionContext &CCContext,
                               ObjCContainerDecl *Container,
-                              bool AllowCategories,
-                              bool AllowNullaryMethods,
+                              bool AllowCategories, bool AllowNullaryMethods,
                               DeclContext *CurContext,
                               AddedPropertiesSet &AddedProperties,
-                              ResultBuilder &Results) {
+                              ResultBuilder &Results,
+                              bool IsBaseExprStatement = false) {
   typedef CodeCompletionResult Result;
 
   // Retrieve the definition.
   Container = getContainerDef(Container);
   
   // Add properties in this container.
-  for (const auto *P : Container->instance_properties())
-    if (AddedProperties.insert(P->getIdentifier()).second)
-      Results.MaybeAddResult(Result(P, Results.getBasePriority(P), nullptr),
-                             CurContext);
+  for (const auto *P : Container->instance_properties()) {
+    if (!AddedProperties.insert(P->getIdentifier()).second)
+      continue;
+
+    Results.MaybeAddResult(Result(P, Results.getBasePriority(P), nullptr),
+                           CurContext);
+
+    // Provide additional block setter completion iff the base expression is a
+    // statement.
+    if (!P->isReadOnly() && IsBaseExprStatement &&
+        P->getType().getTypePtr()->isBlockPointerType()) {
+      FunctionTypeLoc BlockLoc;
+      FunctionProtoTypeLoc BlockProtoLoc;
+      findTypeLocationForBlockDecl(P->getTypeSourceInfo(), BlockLoc,
+                                   BlockProtoLoc);
+
+      // Provide block setter completion only when we are able to find
+      // the FunctionProtoTypeLoc with parameter names for the block.
+      if (BlockLoc) {
+        CodeCompletionBuilder Builder(Results.getAllocator(),
+                                      Results.getCodeCompletionTUInfo());
+        AddResultTypeChunk(Container->getASTContext(),
+                           getCompletionPrintingPolicy(Results.getSema()), P,
+                           CCContext.getBaseType(), Builder);
+        Builder.AddTypedTextChunk(
+            Results.getAllocator().CopyString(P->getName()));
+        Builder.AddChunk(CodeCompletionString::CK_Equal);
+
+        std::string PlaceholderStr = formatBlockPlaceholder(
+            getCompletionPrintingPolicy(Results.getSema()), P, BlockLoc,
+            BlockProtoLoc, /*SuppressBlockName=*/true);
+        // Add the placeholder string.
+        Builder.AddPlaceholderChunk(
+            Builder.getAllocator().CopyString(PlaceholderStr));
+
+        Results.MaybeAddResult(
+            Result(Builder.TakeString(), P,
+                   Results.getBasePriority(P) + CCD_BlockPropertySetter),
+            CurContext);
+      }
+    }
+  }
 
   // Add nullary methods
   if (AllowNullaryMethods) {
@@ -3654,37 +3694,41 @@ static void AddObjCProperties(const CodeCompletionContext &CCContext,
   if (ObjCProtocolDecl *Protocol = dyn_cast<ObjCProtocolDecl>(Container)) {
     for (auto *P : Protocol->protocols())
       AddObjCProperties(CCContext, P, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   } else if (ObjCInterfaceDecl *IFace = dyn_cast<ObjCInterfaceDecl>(Container)){
     if (AllowCategories) {
       // Look through categories.
       for (auto *Cat : IFace->known_categories())
         AddObjCProperties(CCContext, Cat, AllowCategories, AllowNullaryMethods,
-                          CurContext, AddedProperties, Results);
+                          CurContext, AddedProperties, Results,
+                          IsBaseExprStatement);
     }
 
     // Look through protocols.
     for (auto *I : IFace->all_referenced_protocols())
       AddObjCProperties(CCContext, I, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
-    
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
+
     // Look in the superclass.
     if (IFace->getSuperClass())
       AddObjCProperties(CCContext, IFace->getSuperClass(), AllowCategories,
-                        AllowNullaryMethods, CurContext, 
-                        AddedProperties, Results);
+                        AllowNullaryMethods, CurContext, AddedProperties,
+                        Results, IsBaseExprStatement);
   } else if (const ObjCCategoryDecl *Category
                                     = dyn_cast<ObjCCategoryDecl>(Container)) {
     // Look through protocols.
     for (auto *P : Category->protocols())
       AddObjCProperties(CCContext, P, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   }
 }
 
 void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
-                                           SourceLocation OpLoc,
-                                           bool IsArrow) {
+                                           SourceLocation OpLoc, bool IsArrow,
+                                           bool IsBaseExprStatement) {
   if (!Base || !CodeCompleter)
     return;
   
@@ -3766,13 +3810,14 @@ void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
       assert(ObjCPtr && "Non-NULL pointer guaranteed above!");
       AddObjCProperties(CCContext, ObjCPtr->getInterfaceDecl(), true,
                         /*AllowNullaryMethods=*/true, CurContext,
-                        AddedProperties, Results);
+                        AddedProperties, Results, IsBaseExprStatement);
     }
 
     // Add properties from the protocols in a qualified interface.
     for (auto *I : BaseType->getAs<ObjCObjectPointerType>()->quals())
       AddObjCProperties(CCContext, I, true, /*AllowNullaryMethods=*/true,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
              (!IsArrow && BaseType->isObjCObjectType())) {
     // Objective-C instance variable access.
diff --git a/clang/test/Index/complete-block-property-assignment.m b/clang/test/Index/complete-block-property-assignment.m
new file mode 100644
index 00000000000..38156a9d35d
--- /dev/null
+++ b/clang/test/Index/complete-block-property-assignment.m
@@ -0,0 +1,68 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+// rdar://28481726
+
+void func(int x);
+typedef int Foo;
+typedef void (^FooBlock)(Foo *someParameter);
+
+@interface Obj
+@property (readwrite, nonatomic, copy) void (^onAction)(Obj *object);
+@property (readwrite, nonatomic) int foo;
+@end
+
+@interface Test : Obj
+@property (readwrite, nonatomic, copy) FooBlock onEventHandler;
+@property (readonly, nonatomic, copy) void (^onReadonly)(int *someParameter);
+@property (readonly, nonatomic, strong) Obj *obj;
+@end
+
+@implementation Test
+
+#define SELFY self
+
+- (void)test {
+  self.foo = 2;
+  [self takeInt: 2]; self.foo = 2;
+  /* Comment */ self.foo = 2;
+  SELFY.foo = 2
+}
+
+// RUN: c-index-test -code-completion-at=%s:26:8 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:27:27 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:28:22 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:29:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction}{Equal  = }{Placeholder ^(Obj *object)} (38)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler}{Equal  = }{Placeholder ^(Foo *someParameter)} (38)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+
+- (void) takeInt:(int)x { }
+
+- (int) testFailures {
+  (self.foo);
+  int x = self.foo;
+  [self takeInt: self.foo];
+  if (self.foo) {
+    func(self.foo);
+  }
+  return self.foo;
+}
+
+// RUN: c-index-test -code-completion-at=%s:47:9 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:48:16 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:49:23 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:50:12 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:51:15 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:53:15 %s | FileCheck -check-prefix=CHECK-NO %s
+// CHECK-NO: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+
+@end