Skip to content

Prevent rename of argument labels for enum cases #1229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions Sources/SourceKitLSP/Rename.swift
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,24 @@ 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.
Expand All @@ -1013,9 +1031,9 @@ extension SwiftLanguageService {
/// For example, `position` could point to the definition of a function within the file when rename was initiated on
/// a call.
///
/// If a `range` is returned, this is an expanded range that contains both the symbol to rename as well as the
/// position at which the rename was requested. For example, when rename was initiated from the argument label of a
/// function call, the `range` will contain the entire function call from the base name to the closing `)`.
/// If a `functionLikeRange` is returned, this is an expanded range that contains both the symbol to rename as well
/// as the position at which the rename was requested. For example, when rename was initiated from the argument label
/// of a function call, the `range` will contain the entire function call from the base name to the closing `)`.
func symbolToRename(
at position: Position,
in snapshot: DocumentSnapshot
Expand Down Expand Up @@ -1069,8 +1087,17 @@ 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: request.newName, definitionLanguage: .swift)
let newName = CrossLanguageName(clangName: nil, swiftName: requestedNewName, definitionLanguage: .swift)
var edits = try await editsToRename(
locations: renameLocations,
in: snapshot,
Expand Down Expand Up @@ -1359,6 +1386,13 @@ 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
Expand Down
55 changes: 55 additions & 0 deletions Tests/SourceKitLSPTests/RenameTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1181,4 +1181,59 @@ final class RenameTests: XCTestCase {
)
)
}

func testRenameEnumCaseWithUnlabeledAssociatedValue() async throws {
try await assertSingleFileRename(
"""
enum MyEnum {
case 1️⃣myCase(String)
}
""",
newName: "newName",
expectedPrepareRenamePlaceholder: "myCase",
expected: """
enum MyEnum {
case newName(String)
}
"""
)
}

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 assertSingleFileRename(
"""
enum MyEnum {
case 1️⃣myCase(String)
}
""",
newName: "newName(newLabel:)",
expectedPrepareRenamePlaceholder: "myCase",
expected: """
enum MyEnum {
case newName(String)
}
"""
)
}

func testRemoveLabelToEnumCase() async throws {
// We don't support renaming enum parameter labels at the moment
// (https://github.com/apple/sourcekit-lsp/issues/1228)
try await assertSingleFileRename(
"""
enum MyEnum {
case 1️⃣myCase(label: String)
}
""",
newName: "newName(_:)",
expectedPrepareRenamePlaceholder: "myCase",
expected: """
enum MyEnum {
case newName(label: String)
}
"""
)
}
}