Newtype fixit 3.0 (#2752)

* [Import as Member] Print full context in fixit

Due to swift_name and swift_newtype, we are frequently importing onto
different contexts. This was confusing the fixit logic for unavailable
swift2 names, as we were trying to use Clang names when the Swift name
might be totally different (and even a nested type). This change has a
two-fold effect:

1) Globals who are imported onto swift_newtype-ed typedefs should be
   considered ImportAsMember.
2) When printing out the name of an ImportAsMember Swift 3 decl, we
   need to print out a fully qualified context, which also uses the
   Swift names, not the Clang names.

* [swift_newtype] Also test for the right FIXIT

Author: milseman

lib/ClangImporter/ClangImporter.cpp

@@ -1893,12 +1893,48 @@ namespace {
   typedef ClangImporter::Implementation::ImportedName ImportedName;
 }
 
-void ClangImporter::Implementation::ImportedName::printSwiftName(
-       llvm::raw_ostream &os) const {
+/// Will recursively print out the fully qualified context for the given name.
+/// Ends with a trailing "."
+static void
+printFullContextPrefix(ClangImporter::Implementation::ImportedName name,
+                       llvm::raw_ostream &os,
+                       ClangImporter::Implementation &Impl) {
+  const clang::NamedDecl *newDeclContextNamed = nullptr;
+  switch (name.EffectiveContext.getKind()) {
+  case EffectiveClangContext::UnresolvedContext:
+    os << name.EffectiveContext.getUnresolvedName() << ".";
+    // And we're done!
+    return;
+
+  case EffectiveClangContext::DeclContext: {
+    auto namedDecl =
+        dyn_cast<clang::NamedDecl>(name.EffectiveContext.getAsDeclContext());
+    if (!namedDecl) {
+      // We're done
+      return;
+    }
+    newDeclContextNamed = cast<clang::NamedDecl>(namedDecl);
+    break;
+  }
+
+  case EffectiveClangContext::TypedefContext:
+    newDeclContextNamed = name.EffectiveContext.getTypedefName();
+    break;
+  }
+
+  // Now, let's print out the parent
+  assert(newDeclContextNamed && "should of been set");
+  auto parentName = Impl.importFullName(newDeclContextNamed);
+  printFullContextPrefix(parentName, os, Impl);
+  os << parentName.Imported << ".";
+}
+
+void ClangImporter::Implementation::printSwiftName(ImportedName name,
+                                                   llvm::raw_ostream &os) {
   // Property accessors.
   bool isGetter = false;
   bool isSetter = false;
-  switch (AccessorKind) {
+  switch (name.AccessorKind) {
   case ImportedAccessorKind::None:
     break;
 
@@ -1917,43 +1953,27 @@ void ClangImporter::Implementation::ImportedName::printSwiftName(
 
   // If we're importing a global as a member, we need to provide the
   // effective context.
-  if (ImportAsMember) {
-    switch (EffectiveContext.getKind()) {
-    case EffectiveClangContext::DeclContext:
-      os << SwiftLookupTable::translateDeclContext(
-              EffectiveContext.getAsDeclContext())->second;
-      break;
-
-    case EffectiveClangContext::TypedefContext:
-      os << EffectiveContext.getTypedefName()->getName();
-      break;
-
-    case EffectiveClangContext::UnresolvedContext:
-      os << EffectiveContext.getUnresolvedName();
-      break;
-    }
-
-    os << ".";
-  }
+  if (name.ImportAsMember)
+    printFullContextPrefix(name, os, *this);
 
   // Base name.
-  os << Imported.getBaseName().str();
+  os << name.Imported.getBaseName().str();
 
   // Determine the number of argument labels we'll be producing.
-  auto argumentNames = Imported.getArgumentNames();
+  auto argumentNames = name.Imported.getArgumentNames();
   unsigned numArguments = argumentNames.size();
-  if (SelfIndex) ++numArguments;
+  if (name.SelfIndex) ++numArguments;
   if (isSetter) ++numArguments;
 
   // If the result is a simple name that is not a getter, we're done.
-  if (numArguments == 0 && Imported.isSimpleName() && !isGetter) return;
+  if (numArguments == 0 && name.Imported.isSimpleName() && !isGetter) return;
 
   // We need to produce a function name.
   os << "(";
   unsigned currentArgName = 0;
   for (unsigned i = 0; i != numArguments; ++i) {
     // The "self" parameter.
-    if (SelfIndex && *SelfIndex == i) {
+    if (name.SelfIndex && *name.SelfIndex == i) {
       os << "self:";
       continue;
     }
@@ -2357,6 +2377,7 @@ auto ClangImporter::Implementation::importFullName(
   // Import onto a swift_newtype if present
   } else if (auto newtypeDecl = findSwiftNewtype(D, clangSema, swift2Name)) {
     result.EffectiveContext = newtypeDecl;
+    result.ImportAsMember = true;
   // Everything else goes into its redeclaration context.
   } else {
     result.EffectiveContext = dc->getRedeclContext();

lib/ClangImporter/ImportDecl.cpp

@@ -1268,14 +1268,15 @@ namespace {
 
     /// Mark the given declaration as the Swift 2 variant of a Swift 3
     /// declaration with the given name.
-    static void markAsSwift2Variant(Decl *decl, ImportedName swift3Name) {
+    void markAsSwift2Variant(Decl *decl, ImportedName swift3Name,
+                             DeclContext *newDC = nullptr) {
       ASTContext &ctx = decl->getASTContext();
 
       llvm::SmallString<64> renamed;
       {
         // Render a swift_name string.
         llvm::raw_svector_ostream os(renamed);
-        swift3Name.printSwiftName(os);
+        Impl.printSwiftName(swift3Name, os);
       }
 
       auto attr = AvailableAttr::createUnconditional(

lib/ClangImporter/ImporterImpl.h

@@ -912,9 +912,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
       }
     }
 
-    /// Print this imported name as a string suitable for the swift_name
-    /// attribute.
-    void printSwiftName(llvm::raw_ostream &os) const;
   };
 
   /// Flags that control the import of names in importFullName.
@@ -943,6 +940,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
                              const clang::MacroInfo *macro,
                              clang::ASTContext &clangCtx);
 
+  /// Print an imported name as a string suitable for the swift_name attribute.
+  void printSwiftName(ImportedName, llvm::raw_ostream &os);
+
   /// Retrieve the property type as determined by the given accessor.
   static clang::QualType
   getAccessorPropertyType(const clang::FunctionDecl *accessor, bool isSetter,

test/IDE/Inputs/custom-modules/Newtype.h

@@ -94,3 +94,11 @@ extern MyABIOldTypeNS getMyABIOldTypeNS(void);
 extern void takeMyABINewTypeNonNullNS(__nonnull MyABINewTypeNS);
 extern void takeMyABIOldTypeNonNullNS(__nonnull MyABIOldTypeNS);
 
+// Nested types
+typedef struct {int i;} NSSomeContext;
+
+typedef NSString *NSSomeContextName __attribute((swift_newtype(struct)))
+__attribute((swift_name("NSSomeContext.Name")));
+
+extern const NSSomeContextName NSMyContextName;
+

test/IDE/newtype.swift

@@ -91,6 +91,52 @@
 // PRINT-NEXT:  }
 // PRINT-NEXT:  func FooAudited() -> CFNewType
 // PRINT-NEXT:  func FooUnaudited() -> Unmanaged<CFString>
+//
+// PRINT-NEXT:  struct MyABINewType : RawRepresentable, _SwiftNewtypeWrapper {
+// PRINT-NEXT:    init(_ rawValue: CFString)
+// PRINT-NEXT:    init(rawValue: CFString)
+// PRINT-NEXT:    let rawValue: CFString
+// PRINT-NEXT:  }
+// PRINT-NEXT:  typealias MyABIOldType = CFString
+// PRINT-NEXT:  extension MyABINewType {
+// PRINT-NEXT:    static let global: MyABINewType!
+// PRINT-NEXT:  }
+// PRINT-NEXT:  let kMyABIOldTypeGlobal: MyABIOldType!
+// PRINT-NEXT:  func getMyABINewType() -> MyABINewType!
+// PRINT-NEXT:  func getMyABIOldType() -> MyABIOldType!
+// PRINT-NEXT:  func takeMyABINewType(_: MyABINewType!)
+// PRINT-NEXT:  func takeMyABIOldType(_: MyABIOldType!)
+// PRINT-NEXT:  func takeMyABINewTypeNonNull(_: MyABINewType)
+// PRINT-NEXT:  func takeMyABIOldTypeNonNull(_: MyABIOldType)
+// PRINT-NEXT:  struct MyABINewTypeNS : RawRepresentable, _SwiftNewtypeWrapper, Equatable, Hashable, Comparable, _ObjectiveCBridgeable {
+// PRINT-NEXT:    init(_ rawValue: String)
+// PRINT-NEXT:    init(rawValue: String)
+// PRINT-NEXT:    var _rawValue: NSString
+// PRINT-NEXT:    var rawValue: String { get }
+// PRINT-NEXT:  }
+// PRINT-NEXT:  typealias MyABIOldTypeNS = NSString
+// PRINT-NEXT:  func getMyABINewTypeNS() -> MyABINewTypeNS!
+// PRINT-NEXT:  func getMyABIOldTypeNS() -> String!
+// PRINT-NEXT:  func takeMyABINewTypeNonNullNS(_: MyABINewTypeNS)
+// PRINT-NEXT:  func takeMyABIOldTypeNonNullNS(_: String)
+//
+// PRINT-NEXT:  struct NSSomeContext {
+// PRINT-NEXT:    var i: Int32
+// PRINT-NEXT:    init()
+// PRINT-NEXT:    init(i: Int32)
+// PRINT-NEXT:  }
+// PRINT-NEXT:  extension NSSomeContext {
+// PRINT-NEXT:    struct Name : RawRepresentable, _SwiftNewtypeWrapper, Equatable, Hashable, Comparable, _ObjectiveCBridgeable {
+// PRINT-NEXT:      init(_ rawValue: String)
+// PRINT-NEXT:      init(rawValue: String)
+// PRINT-NEXT:      var _rawValue: NSString
+// PRINT-NEXT:      var rawValue: String { get }
+// PRINT-NEXT:    }
+// PRINT-NEXT:  }
+// PRINT-NEXT:  extension NSSomeContext.Name {
+// PRINT-NEXT:    static let myContextName: NSSomeContext.Name
+// PRINT-NEXT:  }
+
 import Newtype
 
 func tests() {
@@ -129,3 +175,8 @@ func testConformances(ed: ErrorDomain) {
   acceptComparable(ed)
   acceptObjectiveCBridgeable(ed)
 }
+
+func testFixit() {
+	let _ = NSMyContextName
+	  // expected-error@-1{{'NSMyContextName' has been renamed to 'NSSomeContext.Name.myContextName'}} {{10-25=NSSomeContext.Name.myContextName}}
+}