Merge pull request #38331 from ahoppen/pr/rename-captured-variables-once

[Refactoring] Only rename variables in capture lists once

Author: ahoppen

lib/IDE/Refactoring.cpp

@@ -654,7 +654,22 @@ class RenameRangeCollector : public IndexDataConsumer {
   Action startSourceEntity(const IndexSymbol &symbol) override {
     if (symbol.USR == USR) {
       if (auto loc = indexSymbolToRenameLoc(symbol, newName)) {
-        locations.push_back(std::move(*loc));
+        // Inside capture lists like `{ [test] in }`, 'test' refers to both the
+        // newly declared, captured variable and the referenced variable it is
+        // initialized from. Make sure to only rename it once.
+        auto existingLoc = llvm::find_if(locations, [&](RenameLoc searchLoc) {
+          return searchLoc.Line == loc->Line && searchLoc.Column == loc->Column;
+        });
+        if (existingLoc == locations.end()) {
+          locations.push_back(std::move(*loc));
+        } else {
+          assert(existingLoc->OldName == loc->OldName &&
+                 existingLoc->NewName == loc->NewName &&
+                 existingLoc->IsFunctionLike == loc->IsFunctionLike &&
+                 existingLoc->IsNonProtocolType ==
+                     existingLoc->IsNonProtocolType &&
+                 "Asked to do a different rename for the same location?");
+        }
       }
     }
     return IndexDataConsumer::Continue;

test/refactoring/rename/Outputs/local/captured_variable.swift.expected

@@ -0,0 +1,57 @@
+func test1() {
+  if true {
+    let x = 1
+    print(x)
+  } else {
+    let x = 2
+    print(x)
+  }
+}
+
+func test2(arg1: Int?, arg2: (Int, String)?) {
+  if let x = arg1 {
+    print(x)
+  } else if let (x, y) = arg2 {
+    print(x, y)
+  }
+}
+
+func test3(arg: Int?) {
+  switch arg {
+  case let .some(x) where x == 0:
+    print(x)
+  case let .some(x) where x == 1,
+       let .some(x) where x == 2:
+    print(x)
+    fallthrough
+  case let .some(x) where x == 3:
+    print(x)
+  default:
+    break
+  }
+}
+
+struct Err1 : Error { }
+func test4(arg: () throws -> Void) {
+  do {
+    try arg()
+  } catch let x as Err1 {
+    print(x)
+  } catch let x {
+    print(x)
+  }
+}
+
+func test5(_ x: Int) {
+  let x = x 
+  print(x)
+}
+
+func testCaputreVariable() {
+  let capturedVariableRenamed = 0
+
+  _ = { [capturedVariableRenamed] in
+    print(capturedVariableRenamed)
+  }
+}
+

test/refactoring/rename/Outputs/local/casebind_1.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/casebind_2.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/catch_1.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/catch_2.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/ifbind_1.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/ifbind_2.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/localvar_1.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/localvar_2.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/param_1.swift.expected

@@ -47,3 +47,11 @@ func test5(_ xRenamed: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/Outputs/local/param_2.swift.expected

@@ -47,3 +47,11 @@ func test5(_ x: Int) {
   print(xRenamed)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+

test/refactoring/rename/local.swift

@@ -47,6 +47,14 @@ func test5(_ x: Int) {
   print(x)
 }
 
+func testCaputreVariable() {
+  let capturedVariable = 0
+
+  _ = { [capturedVariable] in
+    print(capturedVariable)
+  }
+}
+
 // RUN: %empty-directory(%t.result)
 // RUN: %refactor -rename -source-filename %s -pos=3:9 -new-name="xRenamed" >> %t.result/localvar_1.swift
 // RUN: %refactor -rename -source-filename %s -pos=7:11 -new-name="xRenamed" >> %t.result/localvar_2.swift
@@ -68,3 +76,5 @@ func test5(_ x: Int) {
 // RUN: %refactor -rename -source-filename %s -pos=47:9 -new-name="xRenamed" >> %t.result/param_2.swift
 // RUN: diff -u %S/Outputs/local/param_1.swift.expected %t.result/param_1.swift
 // RUN: diff -u %S/Outputs/local/param_2.swift.expected %t.result/param_2.swift
+// RUN: %refactor -rename -source-filename %s -pos=51:7 -new-name="capturedVariableRenamed" >> %t.result/captured_variable.swift
+// RUN: diff -u %S/Outputs/local/captured_variable.swift.expected %t.result/captured_variable.swift