Adjust test cases for fixed enum case rename

Enum case rename is fixed by changes in sourcekitd. We can remove our workaround for the issue and add test cases that test the rename behavior.

Fixes #1228
rdar://127646036

Author: ahoppen

Sources/SKTestSupport/SkipUnless.swift

@@ -156,6 +156,31 @@ public actor SkipUnless {
     }
   }
 
+  /// Checks whether the sourcekitd contains a fix to rename labels of enum cases correctly
+  /// (https://github.com/apple/swift/pull/74241).
+  public static func sourcekitdCanRenameEnumCaseLabels(
+    file: StaticString = #filePath,
+    line: UInt = #line
+  ) async throws {
+    return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 0), file: file, line: line) {
+      let testClient = try await TestSourceKitLSPClient()
+      let uri = DocumentURI(for: .swift)
+      let positions = testClient.openDocument(
+        """
+        enum MyEnum {
+          case 1️⃣myCase(2️⃣String)
+        }
+        """,
+        uri: uri
+      )
+
+      let renameResult = try await testClient.send(
+        RenameRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"], newName: "myCase(label:)")
+      )
+      return renameResult?.changes == [uri: [TextEdit(range: Range(positions["2️⃣"]), newText: "label: ")]]
+    }
+  }
+
   /// Whether clangd has support for the `workspace/indexedRename` request.
   public static func clangdSupportsIndexBasedRename(
     file: StaticString = #filePath,

Sources/SourceKitLSP/Rename.swift

@@ -1003,24 +1003,6 @@ extension SwiftLanguageService {
     return nil
   }
 
-  /// Returns `true` if the given position is inside an `EnumCaseDeclSyntax`.
-  fileprivate func isInsideEnumCaseDecl(position: Position, snapshot: DocumentSnapshot) async -> Bool {
-    let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
-    var node = Syntax(syntaxTree.token(at: snapshot.absolutePosition(of: position)))
-
-    while let parent = node?.parent {
-      if parent.is(EnumCaseDeclSyntax.self) {
-        return true
-      }
-      if parent.is(MemberBlockItemSyntax.self) || parent.is(CodeBlockItemSyntax.self) {
-        // `MemberBlockItemSyntax` and `CodeBlockItemSyntax` can't be nested inside an EnumCaseDeclSyntax. Early exit.
-        return false
-      }
-      node = parent
-    }
-    return false
-  }
-
   /// When the user requested a rename at `position` in `snapshot`, determine the position at which the rename should be
   /// performed internally, the USR of the symbol to rename and the range to rename that should be returned to the
   /// editor.
@@ -1095,17 +1077,8 @@ extension SwiftLanguageService {
 
     try Task.checkCancellation()
 
-    var requestedNewName = request.newName
-    if let openParenIndex = requestedNewName.firstIndex(of: "("),
-      await isInsideEnumCaseDecl(position: renamePosition, snapshot: snapshot)
-    {
-      // We don't support renaming enum parameter labels at the moment
-      // (https://github.com/apple/sourcekit-lsp/issues/1228)
-      requestedNewName = String(requestedNewName[..<openParenIndex])
-    }
-
     let oldName = CrossLanguageName(clangName: nil, swiftName: oldNameString, definitionLanguage: .swift)
-    let newName = CrossLanguageName(clangName: nil, swiftName: requestedNewName, definitionLanguage: .swift)
+    let newName = CrossLanguageName(clangName: nil, swiftName: request.newName, definitionLanguage: .swift)
     var edits = try await editsToRename(
       locations: renameLocations,
       in: snapshot,
@@ -1398,13 +1371,6 @@ extension SwiftLanguageService {
     if name.hasSuffix("()") {
       name = String(name.dropLast(2))
     }
-    if let openParenIndex = name.firstIndex(of: "("),
-      await isInsideEnumCaseDecl(position: renamePosition, snapshot: snapshot)
-    {
-      // We don't support renaming enum parameter labels at the moment
-      // (https://github.com/apple/sourcekit-lsp/issues/1228)
-      name = String(name[..<openParenIndex])
-    }
     guard let relatedIdentRange = response.relatedIdentifiers.first(where: { $0.range.contains(renamePosition) })?.range
     else {
       return nil

Tests/SourceKitLSPTests/RenameTests.swift

@@ -1170,14 +1170,15 @@ final class RenameTests: XCTestCase {
   }
 
   func testRenameEnumCaseWithUnlabeledAssociatedValue() async throws {
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
     try await assertSingleFileRename(
       """
       enum MyEnum {
         case 1️⃣myCase(String)
       }
       """,
       newName: "newName",
-      expectedPrepareRenamePlaceholder: "myCase",
+      expectedPrepareRenamePlaceholder: "myCase(_:)",
       expected: """
         enum MyEnum {
           case newName(String)
@@ -1187,39 +1188,127 @@ final class RenameTests: XCTestCase {
   }
 
   func testAddLabelToEnumCase() async throws {
-    // We don't support renaming enum parameter labels at the moment
-    // (https://github.com/apple/sourcekit-lsp/issues/1228)
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
     try await assertSingleFileRename(
       """
       enum MyEnum {
         case 1️⃣myCase(String)
       }
       """,
       newName: "newName(newLabel:)",
-      expectedPrepareRenamePlaceholder: "myCase",
+      expectedPrepareRenamePlaceholder: "myCase(_:)",
       expected: """
         enum MyEnum {
-          case newName(String)
+          case newName(newLabel: String)
         }
         """
     )
   }
 
-  func testRemoveLabelToEnumCase() async throws {
-    // We don't support renaming enum parameter labels at the moment
-    // (https://github.com/apple/sourcekit-lsp/issues/1228)
+  func testRemoveLabelFromEnumCase() async throws {
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
     try await assertSingleFileRename(
       """
       enum MyEnum {
-        case 1️⃣myCase(label: String)
+        case myCase(label: String)
+      }
+
+      func test() {
+        _ = MyEnum.2️⃣myCase(label: "abc")
       }
       """,
       newName: "newName(_:)",
-      expectedPrepareRenamePlaceholder: "myCase",
+      expectedPrepareRenamePlaceholder: "myCase(label:)",
+      expected: """
+        enum MyEnum {
+          case newName(_ label: String)
+        }
+
+        func test() {
+          _ = MyEnum.newName("abc")
+        }
+        """
+    )
+  }
+
+  func testRenameEnumCaseWithUnderscoreLabel() async throws {
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
+    try await assertSingleFileRename(
+      """
+      enum MyEnum {
+        case myCase(_ label: String)
+      }
+
+      func test() {
+        _ = MyEnum.2️⃣myCase("abc")
+      }
+      """,
+      newName: "newName(_:)",
+      expectedPrepareRenamePlaceholder: "myCase(_:)",
+      expected: """
+        enum MyEnum {
+          case newName(_ label: String)
+        }
+
+        func test() {
+          _ = MyEnum.newName("abc")
+        }
+        """
+    )
+  }
+
+  func testRenameEnumCaseWithUnderscoreToLabelMatchingInternalName() async throws {
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
+    try await assertSingleFileRename(
+      """
+      enum MyEnum {
+        case myCase(_ label: String)
+      }
+
+      func test() {
+        _ = MyEnum.2️⃣myCase("abc")
+      }
+      """,
+      newName: "newName(label:)",
+      expectedPrepareRenamePlaceholder: "myCase(_:)",
       expected: """
         enum MyEnum {
           case newName(label: String)
         }
+
+        func test() {
+          _ = MyEnum.newName(label: "abc")
+        }
+        """
+    )
+  }
+
+  func testRenameEnumCaseWithUnderscoreToLabelNotMatchingInternalName() async throws {
+    try await SkipUnless.sourcekitdCanRenameEnumCaseLabels()
+    // Note that the renamed code doesn't compile because enum cases can't have an external and internal parameter name.
+    // But it's probably the best thing we can do because we don't want to erase the user-specified internal name, which
+    // they didn't request to rename. Also, this is a pretty niche case and having a special case for it is probably not
+    // worth it.
+    try await assertSingleFileRename(
+      """
+      enum MyEnum {
+        case myCase(_ label: String)
+      }
+
+      func test() {
+        _ = MyEnum.2️⃣myCase("abc")
+      }
+      """,
+      newName: "newName(publicLabel:)",
+      expectedPrepareRenamePlaceholder: "myCase(_:)",
+      expected: """
+        enum MyEnum {
+          case newName(publicLabel label: String)
+        }
+
+        func test() {
+          _ = MyEnum.newName(publicLabel: "abc")
+        }
         """
     )
   }