Merge pull request swiftlang#58718 from ahoppen/pr/generate-memberwise-property-wrapper

[Refactoring] Several improvements to memberwise initializer generation

Author: ahoppen

lib/IDE/Refactoring.cpp

@@ -3152,26 +3152,24 @@ bool RefactoringActionLocalizeString::performChange() {
 }
 
 struct MemberwiseParameter {
-  Identifier Name;
+  CharSourceRange NameRange;
   Type MemberType;
   Expr *DefaultExpr;
 
-  MemberwiseParameter(Identifier name, Type type, Expr *initialExpr)
-      : Name(name), MemberType(type), DefaultExpr(initialExpr) {}
+  MemberwiseParameter(CharSourceRange nameRange, Type type, Expr *initialExpr)
+      : NameRange(nameRange), MemberType(type), DefaultExpr(initialExpr) {}
 };
 
 static void generateMemberwiseInit(SourceEditConsumer &EditConsumer,
                                    SourceManager &SM,
                                    ArrayRef<MemberwiseParameter> memberVector,
                                    SourceLoc targetLocation) {
 
-  assert(!memberVector.empty());
-
   EditConsumer.accept(SM, targetLocation, "\ninternal init(");
   auto insertMember = [&SM](const MemberwiseParameter &memberData,
                             raw_ostream &OS, bool wantsSeparator) {
     {
-      OS << memberData.Name << ": ";
+      OS << SM.extractText(memberData.NameRange) << ": ";
       // Unconditionally print '@escaping' if we print out a function type -
       // the assignments we generate below will escape this parameter.
       if (isa<AnyFunctionType>(memberData.MemberType->getCanonicalType())) {
@@ -3180,16 +3178,19 @@ static void generateMemberwiseInit(SourceEditConsumer &EditConsumer,
       OS << memberData.MemberType.getString();
     }
 
+    bool HasAddedDefault = false;
     if (auto *expr = memberData.DefaultExpr) {
-      if (isa<NilLiteralExpr>(expr)) {
-        OS << " = nil";
-      } else if (expr->getSourceRange().isValid()) {
+      if (expr->getSourceRange().isValid()) {
         auto range =
           Lexer::getCharSourceRangeFromSourceRange(
             SM, expr->getSourceRange());
         OS << " = " << SM.extractText(range);
+        HasAddedDefault = true;
       }
     }
+    if (!HasAddedDefault && memberData.MemberType->isOptional()) {
+      OS << " = nil";
+    }
 
     if (wantsSeparator) {
       OS << ", ";
@@ -3199,18 +3200,17 @@ static void generateMemberwiseInit(SourceEditConsumer &EditConsumer,
   // Process the initial list of members, inserting commas as appropriate.
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
-  for (const auto &memberData : memberVector.drop_back()) {
-    insertMember(memberData, OS, /*wantsSeparator*/ true);
+  for (const auto &memberData : llvm::enumerate(memberVector)) {
+    bool wantsSeparator = (memberData.index() != memberVector.size() - 1);
+    insertMember(memberData.value(), OS, wantsSeparator);
   }
 
-  // Process the last (or perhaps, only) member.
-  insertMember(memberVector.back(), OS, /*wantsSeparator*/ false);
-
   // Synthesize the body.
   OS << ") {\n";
   for (auto &member : memberVector) {
     // self.<property> = <property>
-    OS << "self." << member.Name << " = " << member.Name << "\n";
+    auto name = SM.extractText(member.NameRange);
+    OS << "self." << name << " = " << name << "\n";
   }
   OS << "}\n";
 
@@ -3239,30 +3239,41 @@ collectMembersForInit(const ResolvedCursorInfo &CursorInfo,
   if (!targetLocation.isValid())
     return SourceLoc();
 
-  for (auto varDecl : nominalDecl->getStoredProperties()) {
-    auto patternBinding = varDecl->getParentPatternBinding();
-    if (!patternBinding)
+  SourceManager &SM = nominalDecl->getASTContext().SourceMgr;
+
+  for (auto member : nominalDecl->getMembers()) {
+    auto varDecl = dyn_cast<VarDecl>(member);
+    if (!varDecl) {
+      continue;
+    }
+    if (varDecl->getAttrs().hasAttribute<LazyAttr>()) {
+      // Exclude lazy members from the memberwise initializer. This is
+      // inconsistent with the implicitly synthesized memberwise initializer but
+      // we think it makes more sense because otherwise the lazy variable's
+      // initializer gets evaluated eagerly.
       continue;
+    }
 
     if (!varDecl->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) {
       continue;
     }
 
+    auto patternBinding = varDecl->getParentPatternBinding();
+    if (!patternBinding)
+      continue;
+
     const auto i = patternBinding->getPatternEntryIndexForVarDecl(varDecl);
     Expr *defaultInit = nullptr;
     if (patternBinding->isExplicitlyInitialized(i) ||
         patternBinding->isDefaultInitializable()) {
-      defaultInit = varDecl->getParentInitializer();
+      defaultInit = patternBinding->getOriginalInit(i);
     }
 
-    memberVector.emplace_back(varDecl->getName(),
-                              varDecl->getType(), defaultInit);
+    auto NameRange =
+        Lexer::getCharSourceRangeFromSourceRange(SM, varDecl->getNameLoc());
+    memberVector.emplace_back(NameRange, varDecl->getType(), defaultInit);
   }
-  
-  if (memberVector.empty()) {
-    return SourceLoc();
-  }
-  
+
   return targetLocation;
 }
 

test/refactoring/MemberwiseInit/Outputs/generate_memberwise/class_members.swift.expected

@@ -1,5 +1,5 @@
 class Person {
-internal init(firstName: String? = nil, lastName: String? = nil, age: Int? = nil, planet: String = "Earth", solarSystem: String = "Milky Way", avgHeight: Int = 175, location: @escaping () -> Place = { fatalError() }, secondLocation: (() -> Place)? = nil) {
+internal init(firstName: String? = nil, lastName: String? = nil, age: Int? = nil, planet: String = "Earth", solarSystem: String = "Milky Way", avgHeight: Int = 175, location: @escaping () -> Place = { fatalError() }, secondLocation: (() -> Place)? = nil, wrapped: String = "", didSet: String = "ds") {
 self.firstName = firstName
 self.lastName = lastName
 self.age = age
@@ -8,6 +8,8 @@ self.solarSystem = solarSystem
 self.avgHeight = avgHeight
 self.location = location
 self.secondLocation = secondLocation
+self.wrapped = wrapped
+self.didSet = didSet
 }
 
   var firstName: String!
@@ -19,6 +21,15 @@ self.secondLocation = secondLocation
   lazy var idea: Idea = { fatalError() }()
   var location: () -> Place = { fatalError() }
   var secondLocation: (() -> Place)!
+  @MyWrapper var wrapped: String = ""
+  var computed: String { "hi" }
+  var getSet: String {
+    get { "hi" }
+    set {}
+  }
+  var didSet: String = "ds" {
+    didSet { print("didSet") }
+  }
 }
 
 struct Place {
@@ -31,6 +42,8 @@ struct Place {
   let postalCode: Int
   let plusFour: Int?
   let callback: Callback
+  @MyWrapper var wrapped: String
+  var `protocol`: String
 }
 
 protocol Thing {
@@ -41,6 +54,17 @@ enum Idea {
   var subject: String { fatalError() }
 }
 
+struct OnlyComputed {
+  lazy var idea: Idea = { fatalError() }()
+  var computed: String { "hi" }
+}
+
+@propertyWrapper
+struct MyWrapper {
+  let wrappedValue: String
+}
+
+
 
 
 

test/refactoring/MemberwiseInit/Outputs/generate_memberwise/only_computed_members.swift.expected

@@ -0,0 +1,60 @@
+class Person {
+  var firstName: String!
+  var lastName: String!
+  var age: Int!
+  var planet = "Earth", solarSystem = "Milky Way"
+  var avgHeight = 175
+  let line = #line, file = #file, handle = #dsohandle
+  lazy var idea: Idea = { fatalError() }()
+  var location: () -> Place = { fatalError() }
+  var secondLocation: (() -> Place)!
+  @MyWrapper var wrapped: String = ""
+  var computed: String { "hi" }
+  var getSet: String {
+    get { "hi" }
+    set {}
+  }
+  var didSet: String = "ds" {
+    didSet { print("didSet") }
+  }
+}
+
+struct Place {
+  typealias Callback = () -> ()
+  let person: Person
+  let street: String
+  let apartment: Optional<String>
+  let city: String
+  let state: String
+  let postalCode: Int
+  let plusFour: Int?
+  let callback: Callback
+  @MyWrapper var wrapped: String
+  var `protocol`: String
+}
+
+protocol Thing {
+  var idea: Idea { get }
+}
+
+enum Idea {
+  var subject: String { fatalError() }
+}
+
+struct OnlyComputed {
+internal init() {
+}
+
+  lazy var idea: Idea = { fatalError() }()
+  var computed: String { "hi" }
+}
+
+@propertyWrapper
+struct MyWrapper {
+  let wrappedValue: String
+}
+
+
+
+
+

test/refactoring/MemberwiseInit/Outputs/generate_memberwise/struct_members.swift.expected

@@ -8,10 +8,19 @@ class Person {
   lazy var idea: Idea = { fatalError() }()
   var location: () -> Place = { fatalError() }
   var secondLocation: (() -> Place)!
+  @MyWrapper var wrapped: String = ""
+  var computed: String { "hi" }
+  var getSet: String {
+    get { "hi" }
+    set {}
+  }
+  var didSet: String = "ds" {
+    didSet { print("didSet") }
+  }
 }
 
 struct Place {
-internal init(person: Person, street: String, apartment: Optional<String>, city: String, state: String, postalCode: Int, plusFour: Int?, callback: @escaping Place.Callback) {
+internal init(person: Person, street: String, apartment: Optional<String> = nil, city: String, state: String, postalCode: Int, plusFour: Int? = nil, callback: @escaping Place.Callback, wrapped: String, `protocol`: String) {
 self.person = person
 self.street = street
 self.apartment = apartment
@@ -20,6 +29,8 @@ self.state = state
 self.postalCode = postalCode
 self.plusFour = plusFour
 self.callback = callback
+self.wrapped = wrapped
+self.`protocol` = `protocol`
 }
 
   typealias Callback = () -> ()
@@ -31,6 +42,8 @@ self.callback = callback
   let postalCode: Int
   let plusFour: Int?
   let callback: Callback
+  @MyWrapper var wrapped: String
+  var `protocol`: String
 }
 
 protocol Thing {
@@ -41,6 +54,17 @@ enum Idea {
   var subject: String { fatalError() }
 }
 
+struct OnlyComputed {
+  lazy var idea: Idea = { fatalError() }()
+  var computed: String { "hi" }
+}
+
+@propertyWrapper
+struct MyWrapper {
+  let wrappedValue: String
+}
+
+
 
 
 

test/refactoring/MemberwiseInit/generate_memberwise.swift

@@ -8,6 +8,15 @@ class Person {
   lazy var idea: Idea = { fatalError() }()
   var location: () -> Place = { fatalError() }
   var secondLocation: (() -> Place)!
+  @MyWrapper var wrapped: String = ""
+  var computed: String { "hi" }
+  var getSet: String {
+    get { "hi" }
+    set {}
+  }
+  var didSet: String = "ds" {
+    didSet { print("didSet") }
+  }
 }
 
 struct Place {
@@ -20,6 +29,8 @@ struct Place {
   let postalCode: Int
   let plusFour: Int?
   let callback: Callback
+  @MyWrapper var wrapped: String
+  var `protocol`: String
 }
 
 protocol Thing {
@@ -30,13 +41,26 @@ enum Idea {
   var subject: String { fatalError() }
 }
 
+struct OnlyComputed {
+  lazy var idea: Idea = { fatalError() }()
+  var computed: String { "hi" }
+}
+
+@propertyWrapper
+struct MyWrapper {
+  let wrappedValue: String
+}
+
 // RUN: %empty-directory(%t.result)
 // RUN: %refactor -memberwise-init -source-filename %s -pos=1:8 > %t.result/generate_memberwise.swift
 // RUN: diff -u %S/Outputs/generate_memberwise/class_members.swift.expected %t.result/generate_memberwise.swift
 
-// RUN: %refactor -memberwise-init -source-filename %s -pos=13:8 > %t.result/struct_members.swift
+// RUN: %refactor -memberwise-init -source-filename %s -pos=22:8 > %t.result/struct_members.swift
 // RUN: diff -u %S/Outputs/generate_memberwise/struct_members.swift.expected %t.result/struct_members.swift
 
-// RUN: not %refactor -memberwise-init -source-filename %s -pos=21:10 > %t.result/protocol_members.swift
-// RUN: not %refactor -memberwise-init -source-filename %s -pos=25:6 > %t.result/enum_members.swift
+// RUN: %refactor -memberwise-init -source-filename %s -pos=44:8 > %t.result/only_computed_members.swift
+// RUN: diff -u %S/Outputs/generate_memberwise/only_computed_members.swift.expected %t.result/only_computed_members.swift
+
+// RUN: not %refactor -memberwise-init -source-filename %s -pos=36:10 > %t.result/protocol_members.swift
+// RUN: not %refactor -memberwise-init -source-filename %s -pos=40:6 > %t.result/enum_members.swift