Skip to content

Address review comments from #993 #1005

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.

Sign up for GitHub

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 2 commits into from
Jan 9, 2024
Merged

Address review comments from #993 #1005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bda1387
Refactor rename to avoid a force unwrap
ahoppen Dec 19, 2023
f88c241
Refactor multi-file rename tests to specify expected renamed source i…
ahoppen Dec 19, 2023
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
4 changes: 2 additions & 2 deletions Sources/SKTestSupport/MultiFileTestWorkspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import SKCore
/// The location of a test file within test workspace.
public struct RelativeFileLocation: Hashable, ExpressibleByStringLiteral {
/// The subdirectories in which the file is located.
let directories: [String]
public let directories: [String]

/// The file's name.
let fileName: String
public let fileName: String

public init(directories: [String] = [], _ fileName: String) {
self.directories = directories
Expand Down
14 changes: 6 additions & 8 deletions Sources/SourceKitLSP/Rename.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,7 @@ extension SourceKitServer {

// Determine the local edits and the USR to rename
let renameResult = try await languageService.rename(request)
var edits = renameResult.edits
if edits.changes == nil {
// Make sure `edits.changes` is non-nil so we can force-unwrap it below.
edits.changes = [:]
}
var changes = renameResult.edits.changes ?? [:]

if let usr = renameResult.usr, let oldName = renameResult.oldName, let index = workspace.index {
// If we have a USR + old name, perform an index lookup to find workspace-wide symbols to rename.
Expand All @@ -324,7 +320,7 @@ extension SourceKitServer {
await withTaskGroup(of: (DocumentURI, [TextEdit])?.self) { taskGroup in
for (url, renameLocations) in locationsByFile {
let uri = DocumentURI(url)
if edits.changes![uri] != nil {
if changes[uri] != nil {
// We already have edits for this document provided by the language service, so we don't need to compute
// rename ranges for it.
continue
Expand Down Expand Up @@ -358,11 +354,13 @@ extension SourceKitServer {
}
}
for await case let (uri, textEdits)? in taskGroup where !textEdits.isEmpty {
precondition(edits.changes![uri] == nil, "We should create tasks for URIs that already have edits")
edits.changes![uri] = textEdits
precondition(changes[uri] == nil, "We should not create tasks for URIs that already have edits")
changes[uri] = textEdits
}
}
}
var edits = renameResult.edits
edits.changes = changes
return edits
}
}
Expand Down
Loading