Skip to content

Commit 968828b

Browse files
committed
Don’t fail rename if there are multiple index entries for the same USR
The test case that I’m adding right now is that we have a lingering index entry from a file that has been moved on disk. While the proper fix here would be to ignore such an entry from the index, we should also be more resilient and not fail rename altogether just because there are multiple entries for a USR – the chances are very high that they all translate to the same Swift and Objective-C name. rdar://123937371
1 parent b3d2df7 commit 968828b

File tree

2 files changed

+81
-6
lines changed

2 files changed

+81
-6
lines changed

Sources/SourceKitLSP/Rename.swift

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -554,12 +554,14 @@ extension SourceKitServer {
554554
index: IndexStoreDB
555555
) async throws -> CrossLanguageName? {
556556
let definitions = index.occurrences(ofUSR: usr, roles: [.definition])
557-
guard let definitionSymbol = definitions.only else {
558-
if definitions.isEmpty {
559-
logger.error("no definitions for \(usr) found")
560-
} else {
561-
logger.error("Multiple definitions for \(usr) found")
562-
}
557+
if definitions.count > 1 {
558+
logger.log("Multiple definitions for \(usr) found")
559+
}
560+
// There might be multiple definitions of the same symbol eg. in different `#if` branches. In this case pick any of
561+
// them because with very high likelihood they all translate to the same clang and Swift name. Sort the entries to
562+
// ensure that we deterministically pick the same entry every time.
563+
guard let definitionSymbol = definitions.sorted().first else {
564+
logger.error("no definitions for \(usr) found")
563565
return nil
564566
}
565567
let definitionLanguage: Language =

Tests/SourceKitLSPTests/RenameTests.swift

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,4 +1081,77 @@ final class RenameTests: XCTestCase {
10811081
"""
10821082
)
10831083
}
1084+
1085+
func testRenameAfterFileMove() async throws {
1086+
let ws = try await SwiftPMTestWorkspace(
1087+
files: [
1088+
"definition.swift": """
1089+
func 1️⃣foo2️⃣() {}
1090+
""",
1091+
"caller.swift": """
1092+
func test() {
1093+
3️⃣foo4️⃣()
1094+
}
1095+
""",
1096+
],
1097+
build: true
1098+
)
1099+
1100+
let definitionUri = try ws.uri(for: "definition.swift")
1101+
let (callerUri, callerPositions) = try ws.openDocument("caller.swift")
1102+
1103+
// Validate that we get correct rename results before moving the definition file.
1104+
let resultBeforeFileMove = try await ws.testClient.send(
1105+
RenameRequest(textDocument: TextDocumentIdentifier(callerUri), position: callerPositions["3️⃣"], newName: "bar")
1106+
)
1107+
XCTAssertEqual(
1108+
resultBeforeFileMove,
1109+
WorkspaceEdit(
1110+
changes: [
1111+
definitionUri: [
1112+
TextEdit(
1113+
range: try ws.position(of: "1️⃣", in: "definition.swift")..<ws.position(of: "2️⃣", in: "definition.swift"),
1114+
newText: "bar"
1115+
)
1116+
],
1117+
callerUri: [TextEdit(range: callerPositions["3️⃣"]..<callerPositions["4️⃣"], newText: "bar")],
1118+
]
1119+
)
1120+
)
1121+
1122+
let movedDefinitionUri =
1123+
DocumentURI(
1124+
definitionUri.fileURL!
1125+
.deletingLastPathComponent()
1126+
.appendingPathComponent("movedDefinition.swift")
1127+
)
1128+
1129+
try FileManager.default.moveItem(at: XCTUnwrap(definitionUri.fileURL), to: XCTUnwrap(movedDefinitionUri.fileURL))
1130+
1131+
ws.testClient.send(
1132+
DidChangeWatchedFilesNotification(changes: [
1133+
FileEvent(uri: definitionUri, type: .deleted), FileEvent(uri: movedDefinitionUri, type: .created),
1134+
])
1135+
)
1136+
1137+
try await SwiftPMTestWorkspace.build(at: ws.scratchDirectory)
1138+
1139+
let resultAfterFileMove = try await ws.testClient.send(
1140+
RenameRequest(textDocument: TextDocumentIdentifier(callerUri), position: callerPositions["3️⃣"], newName: "bar")
1141+
)
1142+
XCTAssertEqual(
1143+
resultAfterFileMove,
1144+
WorkspaceEdit(
1145+
changes: [
1146+
movedDefinitionUri: [
1147+
TextEdit(
1148+
range: try ws.position(of: "1️⃣", in: "definition.swift")..<ws.position(of: "2️⃣", in: "definition.swift"),
1149+
newText: "bar"
1150+
)
1151+
],
1152+
callerUri: [TextEdit(range: callerPositions["3️⃣"]..<callerPositions["4️⃣"], newText: "bar")],
1153+
]
1154+
)
1155+
)
1156+
}
10841157
}

0 commit comments

Comments
 (0)