[Import Decl] Don’t import as init using omit needless words

Previously, for an Objective-C class method declaration that could be
imported as init, we were making 4 decls:

1) The Swift 2 init
2) The Swift 2 class method decl (suppressing init formation)
3) The Swift 3 init (omitting needless words)
4) The Swift 3 class method decl (suppressing init formation and
   omitting needless words)

Decls 1), 2), and 4) exist for diagnostics and redirect the user at
3). But, 4) does not correspond to any actual Swift version name and
producing it correctly would require the user to understand how
omit-needless-words and other importer magic operates. It provides
very limited value and more importantly gets in the way of future
Clang importer refactoring. We’d like to turn Decl importing into
something that is simpler and language-version parameterized, but
there is no real Swift version to correspond to decl 4).

Therefore we will be making the following decls:

1) The "raw" decl, the name as it would appear to the user if they
   copy-pasted Objective-C code
2) The name as it appeared in Swift 2 (which could be an init)
3) The name as it appeared in Swift 3 (which could be an init and omit
   needless words)

This aligns with the language versions we want to import as in the
future: raw, swift2, swift3, …, and current.

Note that swift-ide-test prunes decls that are unavailable in the
current Swift version, so the Swift 2 non-init decls are not printed
out, though they are still present. Tests were updated and expanded to
ensure this was still the case.

Author: milseman

lib/ClangImporter/ImportDecl.cpp

@@ -1667,19 +1667,17 @@ namespace {
     ///
     /// Note: Use this rather than calling Impl.importFullName directly!
     ImportedName importFullName(const clang::NamedDecl *D,
-                                Optional<ImportedName> &swift3Name,
-                                ImportNameOptions options = None) {
+                                Optional<ImportedName> &swift3Name) {
       // Special handling when we import using the Swift 2 name.
       if (useSwift2Name) {
         // First, import based on the Swift 3 name. If that fails, we won't
         // do anything.
-        swift3Name = Impl.importFullName(D, options);
+        swift3Name = Impl.importFullName(D, None);
         if (!*swift3Name) return *swift3Name;
 
         // Import using the Swift 2 name. If that fails, or if it's identical
         // to the Swift name, we won't introduce a Swift 2 stub declaration.
-        auto swift2Name =
-          Impl.importFullName(D, options | ImportNameFlags::Swift2Name);
+        auto swift2Name = Impl.importFullName(D, ImportNameFlags::Swift2Name);
         if (!swift2Name || swift2Name.Imported == swift3Name->Imported)
           return ImportedName();
 
@@ -1689,7 +1687,7 @@ namespace {
 
       // Just import the Swift 2 name.
       swift3Name = None;
-      return Impl.importFullName(D, options);
+      return Impl.importFullName(D, None);
     }
 
     /// \brief Create a declaration name for anonymous enums, unions and
@@ -1756,6 +1754,12 @@ namespace {
       return ImportedName();
     }
 
+    bool isFactoryInit(ImportedName &name) {
+      return name && name.Imported.getBaseName() == Impl.SwiftContext.Id_init &&
+             (name.InitKind == CtorInitializerKind::Factory ||
+              name.InitKind == CtorInitializerKind::ConvenienceFactory);
+    }
+
   public:
     explicit SwiftDeclConverter(ClangImporter::Implementation &impl,
                                 bool useSwift2Name)
@@ -3162,13 +3166,6 @@ namespace {
       return result;
     }
 
-    /// If the given method is a factory method, import it as a constructor
-    Optional<ConstructorDecl *>
-    importFactoryMethodAsConstructor(Decl *member,
-                                     const clang::ObjCMethodDecl *decl,
-                                     ObjCSelector selector,
-                                     DeclContext *dc);
-
     Decl *VisitObjCMethodDecl(const clang::ObjCMethodDecl *decl,
                               DeclContext *dc) {
       return VisitObjCMethodDecl(decl, dc, false);
@@ -3206,13 +3203,71 @@ namespace {
       if (!useSwift2Name && methodAlreadyImported(selector, isInstance, dc))
         return nullptr;
 
+
+      ImportedName importedName;
       Optional<ImportedName> swift3Name;
-      auto importedName
-        = importFullName(decl, swift3Name,
-                         ImportNameFlags::SuppressFactoryMethodAsInit);
+      importedName = importFullName(decl, swift3Name);
       if (!importedName)
         return nullptr;
 
+      // Normal case applies when we're importing an older name, or when we're
+      // not an init
+      if (useSwift2Name || !isFactoryInit(importedName)) {
+        auto result = importNonInitObjCMethodDecl(decl, dc, importedName,
+                                                  selector, forceClassMethod);
+        if (useSwift2Name && result)
+          markAsSwift2Variant(result, *swift3Name);
+        return result;
+      }
+
+      // We don't want to suppress init formation in Swift 3 names. Instead, we
+      // want the normal Swift 3 name, and a "raw" name for diagnostics. The
+      // "raw" name will be imported as unavailable with a more helpful and
+      // specific message.
+      ++NumFactoryMethodsAsInitializers;
+      bool redundant = false;
+      auto result =
+          importConstructor(decl, dc, false, importedName.InitKind,
+                            /*required=*/false, selector, importedName,
+                            {decl->param_begin(), decl->param_size()},
+                            decl->isVariadic(), redundant);
+
+      // Directly ask the NameImporter for the non-init variant of the Swift 2
+      // name.
+      auto rawOptions =
+          ImportNameOptions(ImportNameFlags::SuppressFactoryMethodAsInit) |
+          ImportNameFlags::Swift2Name;
+      auto rawName = Impl.importFullName(decl, rawOptions);
+      if (!rawName)
+        return result;
+
+      auto rawDecl = importNonInitObjCMethodDecl(decl, dc, rawName, selector,
+                                                 forceClassMethod);
+      if (!rawDecl)
+        return result;
+
+      // Mark the raw imported class method "unavailable", with a useful error
+      // message.
+      llvm::SmallString<64> message;
+      llvm::raw_svector_ostream os(message);
+      os << "use object construction '" << decl->getClassInterface()->getName()
+         << "(";
+      for (auto arg : importedName.Imported.getArgumentNames()) {
+        os << arg << ":";
+      }
+      os << ")'";
+      rawDecl->getAttrs().add(AvailableAttr::createPlatformAgnostic(
+          Impl.SwiftContext, Impl.SwiftContext.AllocateCopy(os.str())));
+      markAsSwift2Variant(rawDecl, importedName);
+      Impl.addAlternateDecl(result, cast<ValueDecl>(rawDecl));
+      return result;
+    }
+
+    Decl *importNonInitObjCMethodDecl(const clang::ObjCMethodDecl *decl,
+                                      DeclContext *dc,
+                                      ImportedName importedName,
+                                      ObjCSelector selector,
+                                      bool forceClassMethod) {
       assert(dc->getDeclaredTypeOfContext() && "Method in non-type context?");
       assert(isa<ClangModuleUnit>(dc->getModuleScopeContext()) &&
              "Clang method in Swift context?");
@@ -3224,6 +3279,7 @@ namespace {
         return nullptr;
 
       // Add the implicit 'self' parameter patterns.
+      bool isInstance = decl->isInstanceMethod() && !forceClassMethod;
       SmallVector<ParameterList *, 4> bodyParams;
       auto selfVar =
         ParamDecl::createSelf(SourceLoc(), dc, /*isStatic*/!isInstance);
@@ -3361,20 +3417,9 @@ namespace {
           if (auto imported = VisitObjCMethodDecl(decl, dc,
                                                   /*forceClassMethod=*/true))
             Impl.addAlternateDecl(result, cast<ValueDecl>(imported));
-        } else if (auto factory = importFactoryMethodAsConstructor(
-                                    result, decl, selector, dc)) {
-          // We imported the factory method as an initializer, so
-          // record it as an alternate declaration.
-          if (*factory)
-            Impl.addAlternateDecl(result, *factory);
         }
-
       }
 
-      // If this is a Swift 2 stub, mark it as such.
-      if (swift3Name)
-        markAsSwift2Variant(result, *swift3Name);
-
       return result;
     }
 
@@ -4976,60 +5021,6 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
   return property;
 }
 
-Optional<ConstructorDecl *>
-SwiftDeclConverter::importFactoryMethodAsConstructor(
-    Decl *member, const clang::ObjCMethodDecl *decl, ObjCSelector selector,
-    DeclContext *dc) {
-  // Import the full name of the method.
-  Optional<ImportedName> swift3Name;
-  auto importedName = importFullName(decl, swift3Name);
-
-  // Check that we imported an initializer name.
-  DeclName initName = importedName;
-  if (initName.getBaseName() != Impl.SwiftContext.Id_init)
-    return None;
-
-  // ... that came from a factory method.
-  if (importedName.InitKind != CtorInitializerKind::Factory &&
-      importedName.InitKind != CtorInitializerKind::ConvenienceFactory)
-    return None;
-
-  bool redundant = false;
-  auto result = importConstructor(decl, dc, false, importedName.InitKind,
-                                  /*required=*/false, selector, importedName,
-                                  {decl->param_begin(), decl->param_size()},
-                                  decl->isVariadic(), redundant);
-
-  if ((result || redundant) && member) {
-    ++NumFactoryMethodsAsInitializers;
-
-    // Mark the imported class method "unavailable", with a useful error
-    // message.
-    // TODO: Could add a replacement string?
-    llvm::SmallString<64> message;
-    llvm::raw_svector_ostream os(message);
-    os << "use object construction '" << decl->getClassInterface()->getName()
-       << "(";
-    for (auto arg : initName.getArgumentNames()) {
-      os << arg << ":";
-    }
-    os << ")'";
-    member->getAttrs().add(AvailableAttr::createPlatformAgnostic(
-        Impl.SwiftContext, Impl.SwiftContext.AllocateCopy(os.str())));
-  }
-
-  /// Record the initializer as an alternative declaration for the
-  /// member.
-  if (result) {
-    Impl.addAlternateDecl(member, result);
-
-    if (swift3Name)
-      markAsSwift2Variant(result, *swift3Name);
-  }
-
-  return result;
-}
-
 ConstructorDecl *SwiftDeclConverter::importConstructor(
     const clang::ObjCMethodDecl *objcMethod, DeclContext *dc, bool implicit,
     Optional<CtorInitializerKind> kind, bool required) {

lib/ClangImporter/ImportName.cpp

@@ -1084,6 +1084,18 @@ bool NameImporter::hasErrorMethodNameCollision(
                                enableObjCInterop());
 }
 
+/// Whether we should suppress this factory method being imported as an
+/// initializer. We want to do this when explicitly directed to, or when
+/// importing a property accessor.
+static bool suppressFactoryMethodAsInit(const clang::ObjCMethodDecl *method,
+                                        ImportNameOptions options,
+                                        CtorInitializerKind initKind) {
+  return (method->isPropertyAccessor() ||
+          options.contains(ImportNameFlags::SuppressFactoryMethodAsInit)) &&
+         (initKind == CtorInitializerKind::Factory ||
+          initKind == CtorInitializerKind::ConvenienceFactory);
+}
+
 ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
                                           ImportNameOptions options) {
   ImportedName result;
@@ -1198,9 +1210,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
         // If this swift_name attribute maps a factory method to an
         // initializer and we were asked not to do so, ignore the
         // custom name.
-        if (options.contains(ImportNameFlags::SuppressFactoryMethodAsInit) &&
-            (result.InitKind == CtorInitializerKind::Factory ||
-             result.InitKind == CtorInitializerKind::ConvenienceFactory)) {
+        if (suppressFactoryMethodAsInit(method, options, result.InitKind)) {
           skipCustomName = true;
         } else {
           // Note that this is an initializer.
@@ -1331,9 +1341,7 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
     // If we would import a factory method as an initializer but were
     // asked not to, don't consider this as an initializer.
     if (isInitializer &&
-        options.contains(ImportNameFlags::SuppressFactoryMethodAsInit) &&
-        (result.InitKind == CtorInitializerKind::Factory ||
-         result.InitKind == CtorInitializerKind::ConvenienceFactory)) {
+        suppressFactoryMethodAsInit(objcMethod, options, result.InitKind)) {
       isInitializer = false;
     }
 

lib/ClangImporter/ImporterImpl.h

@@ -447,7 +447,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
 
   /// Retrieve the alternative declaration for the given imported
   /// Swift declaration.
-  TinyPtrVector<ValueDecl *> getAlternateDecls(Decl *decl) {
+  ArrayRef<ValueDecl *> getAlternateDecls(Decl *decl) {
     auto known = AlternateDecls.find(decl);
     if (known == AlternateDecls.end()) return {};
     return known->second;

test/ClangImporter/Inputs/SwiftPrivateAttr.txt

@@ -8,17 +8,10 @@ class Foo : NSObject, __PrivProto {
   func __noArgs()
   func __oneArg(_ arg: Int32)
   func __twoArgs(_ arg: Int32, other arg2: Int32)
-  class func __foo() -> Self!
   class func __withNoArgs() -> Self!
   convenience init!(__oneArg arg: Int32)
-  @available(*, unavailable, message: "use object construction 'Foo(__oneArg:)'")
-  class func __withOneArg(_ arg: Int32) -> Self!
   convenience init!(__twoArgs arg: Int32, other arg2: Int32)
-  @available(*, unavailable, message: "use object construction 'Foo(__twoArgs:other:)'")
-  class func __withTwoArgs(_ arg: Int32, other arg2: Int32) -> Self!
   convenience init!(__ arg: Int32)
-  @available(*, unavailable, message: "use object construction 'Foo(__:)'")
-  class func __foo(_ arg: Int32) -> Self!
   func objectForKeyedSubscript(_ index: Any!) -> Any!
   func __setObject(_ object: Any!, forKeyedSubscript index: Any!)
   func __objectAtIndexedSubscript(_ index: Int32) -> Any!

test/ClangImporter/attr-swift_private.swift

@@ -132,3 +132,10 @@ func testCF(_ a: __PrivCFType, b: __PrivCFSub, c: __PrivInt) {
 extension __PrivCFType {}
 extension __PrivCFSub {}
 _ = 1 as __PrivInt
+
+#if !IRGEN
+func testRawNames() {
+  let _ = Foo.__fooWithOneArg(0) // expected-error {{'__fooWithOneArg' is unavailable: use object construction 'Foo(__oneArg:)'}}
+  let _ = Foo.__foo // expected-error{{'__foo' is unavailable: use object construction 'Foo(__:)'}}
+}
+#endif

test/ClangImporter/objc_factory_method.swift

@@ -84,7 +84,7 @@ func testNonInstanceTypeFactoryMethod(_ s: String) {
 }
 
 func testUseOfFactoryMethod(_ queen: Bee) {
-  _ = Hive.withQueen(queen) // expected-error{{'withQueen' is unavailable: use object construction 'Hive(queen:)'}}
+  _ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' is unavailable: use object construction 'Hive(queen:)'}}
 }
 
 func testNonsplittableFactoryMethod() {
@@ -97,16 +97,16 @@ func testFactoryMethodBlacklist() {
 
 func test17261609() {
   _ = NSDecimalNumber(mantissa:1, exponent:1, isNegative:true)
-  _ = NSDecimalNumber.withMantissa(1, exponent:1, isNegative:true) // expected-error{{'withMantissa(_:exponent:isNegative:)' is unavailable: use object construction 'NSDecimalNumber(mantissa:exponent:isNegative:)'}}
+  _ = NSDecimalNumber.decimalNumberWithMantissa(1, exponent:1, isNegative:true) // expected-error{{'decimalNumberWithMantissa(_:exponent:isNegative:)' is unavailable: use object construction 'NSDecimalNumber(mantissa:exponent:isNegative:)'}}
 }
 
 func testURL() {
   let url = NSURL(string: "http://www.llvm.org")!
-  _ = NSURL.withString("http://www.llvm.org") // expected-error{{'withString' is unavailable: use object construction 'NSURL(string:)'}}
+  _ = NSURL.URLWithString("http://www.llvm.org") // expected-error{{'URLWithString' is unavailable: use object construction 'NSURL(string:)'}}
 
   NSURLRequest(string: "http://www.llvm.org") // expected-warning{{unused}}
   NSURLRequest(url: url as URL) // expected-warning{{unused}}
 
-  _ = NSURLRequest.withString("http://www.llvm.org") // expected-error{{'withString' is unavailable: use object construction 'NSURLRequest(string:)'}}
-  _ = NSURLRequest.withURL(url as URL) // expected-error{{'withURL' is unavailable: use object construction 'NSURLRequest(url:)'}}
+  _ = NSURLRequest.requestWithString("http://www.llvm.org") // expected-error{{'requestWithString' is unavailable: use object construction 'NSURLRequest(string:)'}}
+  _ = NSURLRequest.URLRequestWithURL(url as URL) // expected-error{{'URLRequestWithURL' is unavailable: use object construction 'NSURLRequest(url:)'}}
 }

test/ClangImporter/objc_implicit_with.swift

@@ -50,7 +50,7 @@ func testNonInstanceTypeFactoryMethod(_ s: String) {
 }
 
 func testUseOfFactoryMethod(_ queen: Bee) {
-  _ = Hive.withQueen(queen) // expected-error{{'withQueen' is unavailable: use object construction 'Hive(queen:)'}}
+  _ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' is unavailable: use object construction 'Hive(queen:)'}}
 }
 
 func testNonsplittableFactoryMethod() {

test/ClangImporter/objc_init.swift

@@ -164,3 +164,10 @@ class DesignedInitSubSub : DesignatedInitSub {
   init(double: Double) { super.init(int: 0) } // okay
   init(string: String) { super.init() } // expected-error {{must call a designated initializer of the superclass 'DesignatedInitSub'}}
 }
+
+// Make sure that our magic doesn't think the class property with the type name is an init
+func classPropertiesAreNotInit() -> ProcessInfo {
+  var procInfo = NSProcessInfo.processInfo // expected-error{{'NSProcessInfo' has been renamed to 'ProcessInfo'}}
+  procInfo = ProcessInfo.processInfo // okay
+  return procInfo
+}