SE-0064 / SR-1239: Fix-Its for string-literal selectors naming proper…

…ty accessors. When we see a string literal used to initializer a Selector, and that string literal names the selector for the getter or setter of an Objective-C property, suggest #selector(getter: ...) or This completes SE-0064. Big thanks for Alex Hoppen (@ahoppen) for his contributions here.
waytc · May 12, 2016 · 3650ce8 · 3650ce8
1 parent a0a74ae
commit 3650ce8
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -212,6 +212,14 @@ Swift 3.0
     }
     ```
 
+* [SE-0064](https://github.com/apple/swift-evolution/blob/master/proposals/0064-property-selectors.md) The Objective-C selectors for the getter or setter of a property can now be referenced with `#selector`. For example:
+
+    ```swift
+    let sel1 = #selector(getter: UIView.backgroundColor) // sel1 has type Selector
+    let sel2 = #selector(setter: UIView.backgroundColor) // sel2 has type Selector
+    ```
+
+
 Swift 2.2
 ---------
 

diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -2809,22 +2809,10 @@ class ObjCSelectorWalker : public ASTWalker {
       return { true, expr };
     }
 
-    // Separate out the accessor methods.
-    auto splitPoint =
-      std::stable_partition(allMethods.begin(), allMethods.end(),
-                            [](AbstractFunctionDecl *abstractFunc) -> bool {
-                              if (auto func = dyn_cast<FuncDecl>(abstractFunc))
-                                return !func->isAccessor();
-
-                              return true;
-                            });
-    ArrayRef<AbstractFunctionDecl *> methods(allMethods.begin(), splitPoint);
-    ArrayRef<AbstractFunctionDecl *> accessors(splitPoint, allMethods.end());
-
     // Find the "best" method that has this selector, so we can report
     // that.
     AbstractFunctionDecl *bestMethod = nullptr;
-    for (auto method : methods) {
+    for (auto method : allMethods) {
       // If this is the first method, use it.
       if (!bestMethod) {
         bestMethod = method;
@@ -2883,9 +2871,38 @@ class ObjCSelectorWalker : public ASTWalker {
         llvm::raw_svector_ostream out(replacement);
         auto nominal = bestMethod->getDeclContext()
                          ->getAsNominalTypeOrNominalTypeExtensionContext();
-        auto name = bestMethod->getFullName();
-        out << "#selector(" << nominal->getName().str() << "."
-            << name.getBaseName().str();
+        out << "#selector(";
+
+        DeclName name;
+        auto bestFunc = dyn_cast<FuncDecl>(bestMethod);
+        bool isAccessor = bestFunc && bestFunc->isAccessor();
+        if (isAccessor) {
+          switch (bestFunc->getAccessorKind()) {
+          case AccessorKind::NotAccessor:
+            llvm_unreachable("not an accessor");
+
+          case AccessorKind::IsGetter:
+            out << "getter: ";
+            name = bestFunc->getAccessorStorageDecl()->getFullName();
+            break;
+
+          case AccessorKind::IsSetter:
+          case AccessorKind::IsWillSet:
+          case AccessorKind::IsDidSet:
+            out << "setter: ";
+            name = bestFunc->getAccessorStorageDecl()->getFullName();
+            break;
+
+          case AccessorKind::IsMaterializeForSet:
+          case AccessorKind::IsAddressor:
+          case AccessorKind::IsMutableAddressor:
+            llvm_unreachable("cannot be @objc");
+          }
+        } else {
+          name = bestMethod->getFullName();
+        }
+
+        out << nominal->getName().str() << "." << name.getBaseName().str();
         auto argNames = name.getArgumentNames();
 
         // Only print the parentheses if there are some argument
@@ -2902,7 +2919,7 @@ class ObjCSelectorWalker : public ASTWalker {
 
         // If there will be an ambiguity when referring to the method,
         // introduce a coercion to resolve it to the method we found.
-        if (isSelectorReferenceAmbiguous(bestMethod)) {
+        if (!isAccessor && isSelectorReferenceAmbiguous(bestMethod)) {
           if (auto fnType =
                 bestMethod->getInterfaceType()->getAs<FunctionType>()) {
             // For static/class members, drop the metatype argument.

diff --git a/test/ClangModules/objc_parse.swift b/test/ClangModules/objc_parse.swift
@@ -378,16 +378,16 @@ func testPreferClassMethodToCurriedInstanceMethod(_ obj: NSObject) {
 func testPropertyAndMethodCollision(_ obj: PropertyAndMethodCollision,
                                     rev: PropertyAndMethodReverseCollision) {
   obj.object = nil
-  obj.object(obj, doSomething:Selector("action"))
+  obj.object(obj, doSomething:#selector(getter: NSMenuItem.action))
 
   obj.dynamicType.classRef = nil
-  obj.dynamicType.classRef(obj, doSomething:Selector("action"))
+  obj.dynamicType.classRef(obj, doSomething:#selector(getter: NSMenuItem.action))
 
   rev.object = nil
-  rev.object(rev, doSomething:Selector("action"))
+  rev.object(rev, doSomething:#selector(getter: NSMenuItem.action))
 
   rev.dynamicType.classRef = nil
-  rev.dynamicType.classRef(rev, doSomething:Selector("action"))
+  rev.dynamicType.classRef(rev, doSomething:#selector(getter: NSMenuItem.action))
 
   var value: AnyObject
   value = obj.protoProp()

diff --git a/test/expr/unary/selector/fixits.swift b/test/expr/unary/selector/fixits.swift
@@ -64,8 +64,8 @@ func testDeprecatedStringLiteralSelector() {
   _ = sel1
 
   _ = "methodWithValue:label:" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' instead}}{{7-43=#selector(Foo.method(_:label:))}}
-  _ = "property" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' or explicitly construct a 'Selector'}}{{7-7=Selector(}}{{17-29=)}}
-  _ = "setProperty:" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' or explicitly construct a 'Selector'}}{{7-7=Selector(}}{{21-33=)}}
+  _ = "property" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' instead}}{{7-29=#selector(getter: Foo.property)}}
+  _ = "setProperty:" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' instead}}{{7-33=#selector(setter: Foo.property)}}
   _ = "unknownMethodWithValue:label:" as Selector // expected-warning{{no method declared with Objective-C selector 'unknownMethodWithValue:label:'}}{{7-7=Selector(}}{{38-50=)}}
   _ = "badSelector:label" as Selector // expected-warning{{string literal is not a valid Objective-C selector}}
   _ = "method2WithValue:" as Selector // expected-warning{{use of string literal for Objective-C selectors is deprecated; use '#selector' instead}}{{7-38=#selector(Foo.method2(_:))}}
@@ -96,6 +96,9 @@ func testDeprecatedStringLiteralSelector() {
 
 func testSelectorConstruction() {
   _ = Selector("methodWithValue:label:") // expected-warning{{use '#selector' instead of explicitly constructing a 'Selector'}}{{7-41=#selector(Foo.method(_:label:))}}
+  _ = Selector("property") // expected-warning{{use '#selector' instead of explicitly constructing a 'Selector'}}{{7-27=#selector(getter: Foo.property)}}
+  _ = Selector("setProperty:") // expected-warning{{use '#selector' instead of explicitly constructing a 'Selector'}}{{7-31=#selector(setter: Foo.property)}}
+
   _ = Selector("unknownMethodWithValue:label:") // expected-warning{{no method declared with Objective-C selector 'unknownMethodWithValue:label:'}}
   // expected-note@-1{{wrap the selector name in parentheses to suppress this warning}}{{16-16=(}}{{47-47=)}}
   _ = Selector(("unknownMethodWithValue:label:"))