From: Alex Lorenz <arphaman@gmail.com>
Date: Wed, 1 Nov 2017 00:20:55 +0000 (+0000)
Subject: [refactor][extract] prohibit extraction of ObjC property setters
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=06b063e3bbf2ce05d25770a4c77cf1b748ce198c;p=clang

[refactor][extract] prohibit extraction of ObjC property setters


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@317056 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Basic/DiagnosticRefactoringKinds.td b/include/clang/Basic/DiagnosticRefactoringKinds.td
index b54b9301a7..ee396b9307 100644
--- a/include/clang/Basic/DiagnosticRefactoringKinds.td
+++ b/include/clang/Basic/DiagnosticRefactoringKinds.td
@@ -26,6 +26,8 @@ def err_refactor_code_outside_of_function : Error<"the selected code is not a "
   "part of a function's / method's body">;
 def err_refactor_extract_simple_expression : Error<"the selected expression "
   "is too simple to extract">;
+def err_refactor_extract_prohibited_expression : Error<"the selected "
+  "expression can't be extracted">;
 
 }
 
diff --git a/lib/Tooling/Refactoring/Extract.cpp b/lib/Tooling/Refactoring/Extract.cpp
index b1000b60ee..e81bb3ffe9 100644
--- a/lib/Tooling/Refactoring/Extract.cpp
+++ b/lib/Tooling/Refactoring/Extract.cpp
@@ -16,6 +16,7 @@
 #include "clang/Tooling/Refactoring/Extract/Extract.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprObjC.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 
 namespace clang {
@@ -70,12 +71,20 @@ ExtractFunction::initiate(RefactoringRuleContext &Context,
     return Context.createDiagnosticError(
         diag::err_refactor_code_outside_of_function);
 
-  // Avoid extraction of simple literals and references.
-  if (Code.size() == 1 && isSimpleExpression(dyn_cast<Expr>(Code[0])))
-    return Context.createDiagnosticError(
-        diag::err_refactor_extract_simple_expression);
+  if (Code.size() == 1) {
+    // Avoid extraction of simple literals and references.
+    if (isSimpleExpression(dyn_cast<Expr>(Code[0])))
+      return Context.createDiagnosticError(
+          diag::err_refactor_extract_simple_expression);
+
+    // Property setters can't be extracted.
+    if (const auto *PRE = dyn_cast<ObjCPropertyRefExpr>(Code[0])) {
+      if (!PRE->isMessagingGetter())
+        return Context.createDiagnosticError(
+            diag::err_refactor_extract_prohibited_expression);
+    }
+  }
 
-  // FIXME (Alex L): Prohibit extraction of Objective-C property setters.
   return ExtractFunction(std::move(Code), DeclName);
 }
 
diff --git a/test/Refactor/Extract/ObjCProperty.m b/test/Refactor/Extract/ObjCProperty.m
new file mode 100644
index 0000000000..152ccb3484
--- /dev/null
+++ b/test/Refactor/Extract/ObjCProperty.m
@@ -0,0 +1,41 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
+
+@interface HasProperty
+
+@property (strong) HasProperty *item;
+
+- (HasProperty *)implicitProp;
+
+- (void)setImplicitSetter:(HasProperty *)value;
+
+@end
+
+@implementation HasProperty
+
+- (void)allowGetterExtraction {
+  /*range allow_getter=->+0:42*/self.item;
+  /*range allow_imp_getter=->+0:54*/self.implicitProp;
+}
+// CHECK: 1 'allow_getter' results:
+// CHECK:      extracted() {
+// CHECK-NEXT: return self.item;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: - (void)allowGetterExtraction {
+// CHECK-NEXT: extracted();
+
+// CHECK: 1 'allow_imp_getter' results:
+// CHECK:      extracted() {
+// CHECK-NEXT: return self.implicitProp;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: - (void)allowGetterExtraction {
+// CHECK-NEXT: self.item;
+// CHECK-NEXT: extracted();
+
+- (void)prohibitSetterExtraction {
+  /*range prohibit_setter=->+0:45*/self.item = 0;
+  /*range prohibit_setter=->+0:55*/self.implicitSetter = 0;
+}
+// CHECK: 2 'prohibit_setter' results:
+// CHECK: the selected expression can't be extracted
+
+@end