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

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Oct 31 17:20:55 2017
New Revision: 317056

URL: http://llvm.org/viewvc/llvm-project?rev=317056&view=rev
Log:
[refactor][extract] prohibit extraction of ObjC property setters

Added:
    cfe/trunk/test/Refactor/Extract/ObjCProperty.m
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td
    cfe/trunk/lib/Tooling/Refactoring/Extract.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td?rev=317056&r1=317055&r2=317056&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td Tue Oct 31 17:20:55 2017
@@ -26,6 +26,8 @@ def err_refactor_code_outside_of_functio
   "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">;
 
 }
 

Modified: cfe/trunk/lib/Tooling/Refactoring/Extract.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Extract.cpp?rev=317056&r1=317055&r2=317056&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/Extract.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Extract.cpp Tue Oct 31 17:20:55 2017
@@ -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(RefactoringRul
     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);
 }
 

Added: cfe/trunk/test/Refactor/Extract/ObjCProperty.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Refactor/Extract/ObjCProperty.m?rev=317056&view=auto
==============================================================================
--- cfe/trunk/test/Refactor/Extract/ObjCProperty.m (added)
+++ cfe/trunk/test/Refactor/Extract/ObjCProperty.m Tue Oct 31 17:20:55 2017
@@ -0,0 +1,41 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
+
+ at interface HasProperty
+
+ at property (strong) HasProperty *item;
+
+- (HasProperty *)implicitProp;
+
+- (void)setImplicitSetter:(HasProperty *)value;
+
+ at end
+
+ at 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
+
+ at end