Skip to content

Commit 5640204

Browse files
committed
Prevent rename of argument labels for enum cases
Renaming an enum case currently caused invalid code to be generated because we would rename eg. `myCase(String)` to `myNewCase(_ String)`. Fixing the underlying issue requires changes to `sourcekitd`, that are out-of-scope at the moment. For now, just suppress argument label rename for enum cases in SourceKit-LSP and avoid generating invalid code even if just the base name is modified. rdar://127248157
1 parent e71aa5d commit 5640204

File tree

2 files changed

+93
-4
lines changed

2 files changed

+93
-4
lines changed

Sources/SourceKitLSP/Rename.swift

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -995,6 +995,24 @@ extension SwiftLanguageService {
995995
return nil
996996
}
997997

998+
/// Returns `true` if the given position is inside an `EnumCaseDeclSyntax`.
999+
fileprivate func isInsideEnumCaseDecl(position: Position, snapshot: DocumentSnapshot) async -> Bool {
1000+
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
1001+
var node = Syntax(syntaxTree.token(at: snapshot.absolutePosition(of: position)))
1002+
1003+
while let parent = node?.parent {
1004+
if parent.is(EnumCaseDeclSyntax.self) {
1005+
return true
1006+
}
1007+
if parent.is(MemberBlockItemSyntax.self) || parent.is(CodeBlockItemSyntax.self) {
1008+
// `MemberBlockItemSyntax` and `CodeBlockItemSyntax` can't be nested inside an EnumCaseDeclSyntax. Early exit.
1009+
return false
1010+
}
1011+
node = parent
1012+
}
1013+
return false
1014+
}
1015+
9981016
/// When the user requested a rename at `position` in `snapshot`, determine the position at which the rename should be
9991017
/// performed internally, the USR of the symbol to rename and the range to rename that should be returned to the
10001018
/// editor.
@@ -1013,9 +1031,9 @@ extension SwiftLanguageService {
10131031
/// For example, `position` could point to the definition of a function within the file when rename was initiated on
10141032
/// a call.
10151033
///
1016-
/// If a `range` is returned, this is an expanded range that contains both the symbol to rename as well as the
1017-
/// position at which the rename was requested. For example, when rename was initiated from the argument label of a
1018-
/// function call, the `range` will contain the entire function call from the base name to the closing `)`.
1034+
/// If a `functionLikeRange` is returned, this is an expanded range that contains both the symbol to rename as well
1035+
/// as the position at which the rename was requested. For example, when rename was initiated from the argument label
1036+
/// of a function call, the `range` will contain the entire function call from the base name to the closing `)`.
10191037
func symbolToRename(
10201038
at position: Position,
10211039
in snapshot: DocumentSnapshot
@@ -1069,8 +1087,17 @@ extension SwiftLanguageService {
10691087

10701088
try Task.checkCancellation()
10711089

1090+
var requestedNewName = request.newName
1091+
if let openParenIndex = requestedNewName.firstIndex(of: "("),
1092+
await isInsideEnumCaseDecl(position: renamePosition, snapshot: snapshot)
1093+
{
1094+
// We don't support renaming enum parameter labels at the moment
1095+
// (https://github.com/apple/sourcekit-lsp/issues/1228)
1096+
requestedNewName = String(requestedNewName[..<openParenIndex])
1097+
}
1098+
10721099
let oldName = CrossLanguageName(clangName: nil, swiftName: oldNameString, definitionLanguage: .swift)
1073-
let newName = CrossLanguageName(clangName: nil, swiftName: request.newName, definitionLanguage: .swift)
1100+
let newName = CrossLanguageName(clangName: nil, swiftName: requestedNewName, definitionLanguage: .swift)
10741101
var edits = try await editsToRename(
10751102
locations: renameLocations,
10761103
in: snapshot,
@@ -1359,6 +1386,13 @@ extension SwiftLanguageService {
13591386
if name.hasSuffix("()") {
13601387
name = String(name.dropLast(2))
13611388
}
1389+
if let openParenIndex = name.firstIndex(of: "("),
1390+
await isInsideEnumCaseDecl(position: renamePosition, snapshot: snapshot)
1391+
{
1392+
// We don't support renaming enum parameter labels at the moment
1393+
// (https://github.com/apple/sourcekit-lsp/issues/1228)
1394+
name = String(name[..<openParenIndex])
1395+
}
13621396
guard let relatedIdentRange = response.relatedIdentifiers.first(where: { $0.range.contains(renamePosition) })?.range
13631397
else {
13641398
return nil

Tests/SourceKitLSPTests/RenameTests.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,4 +1181,59 @@ final class RenameTests: XCTestCase {
11811181
)
11821182
)
11831183
}
1184+
1185+
func testRenameEnumCaseWithUnlabeledAssociatedValue() async throws {
1186+
try await assertSingleFileRename(
1187+
"""
1188+
enum MyEnum {
1189+
case 1️⃣myCase(String)
1190+
}
1191+
""",
1192+
newName: "newName",
1193+
expectedPrepareRenamePlaceholder: "myCase",
1194+
expected: """
1195+
enum MyEnum {
1196+
case newName(String)
1197+
}
1198+
"""
1199+
)
1200+
}
1201+
1202+
func testAddLabelToEnumCase() async throws {
1203+
// We don't support renaming enum parameter labels at the moment
1204+
// (https://github.com/apple/sourcekit-lsp/issues/1228)
1205+
try await assertSingleFileRename(
1206+
"""
1207+
enum MyEnum {
1208+
case 1️⃣myCase(String)
1209+
}
1210+
""",
1211+
newName: "newName(newLabel:)",
1212+
expectedPrepareRenamePlaceholder: "myCase",
1213+
expected: """
1214+
enum MyEnum {
1215+
case newName(String)
1216+
}
1217+
"""
1218+
)
1219+
}
1220+
1221+
func testRemoveLabelToEnumCase() async throws {
1222+
// We don't support renaming enum parameter labels at the moment
1223+
// (https://github.com/apple/sourcekit-lsp/issues/1228)
1224+
try await assertSingleFileRename(
1225+
"""
1226+
enum MyEnum {
1227+
case 1️⃣myCase(label: String)
1228+
}
1229+
""",
1230+
newName: "newName(_:)",
1231+
expectedPrepareRenamePlaceholder: "myCase",
1232+
expected: """
1233+
enum MyEnum {
1234+
case newName(label: String)
1235+
}
1236+
"""
1237+
)
1238+
}
11841239
}

0 commit comments

Comments
 (0)