Skip to content

Commit 50f0f61

Browse files
committed
Address review comments
1 parent d73f44c commit 50f0f61

File tree

2 files changed

+22
-7
lines changed

2 files changed

+22
-7
lines changed

Sources/SourceKitLSP/Rename.swift

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,16 @@ private extension RenameLocation.Usage {
291291
}
292292
}
293293

294+
private extension IndexSymbolKind {
295+
var isMethod: Bool {
296+
switch self {
297+
case .instanceMethod, .classMethod, .staticMethod:
298+
return true
299+
default: return false
300+
}
301+
}
302+
}
303+
294304
// MARK: - Name translation
295305

296306
extension SwiftLanguageServer {
@@ -321,7 +331,8 @@ extension SwiftLanguageServer {
321331

322332
/// Translate a Swift name to the corresponding C/C++/ObjectiveC name.
323333
///
324-
/// This invokes the clang importer to perform the name translation.
334+
/// This invokes the clang importer to perform the name translation, based on the `position` and `uri` at which the
335+
/// Swift symbol is defined.
325336
///
326337
/// - Parameters:
327338
/// - position: The position at which the Swift name is defined
@@ -520,7 +531,12 @@ public actor TranslatableName {
520531
/// - Note: The symbol translation from clang to Swift requires a Swift location at which the clang symbol is
521532
/// (probably so that sourcekitd can access the necessary AST context and get the clang importer to translate the
522533
/// name). But the output does not actually matter on the specific position and URI. We can thus cache the result.
523-
/// Ideally, sourcekitd wouldn't need a position and URI to translate a clang name to Swift.
534+
/// Ideally, sourcekitd wouldn't need a position and URI to translate a clang name to Swift
535+
536+
/// - Note: The symbol translation from Clang to Swift requires a location in Swift in which the symbol is referenced.
537+
/// This is used both to retrieve the clang importer and to grab the underlying clang decl.
538+
/// The exact reference used doesn't matter though, so we can cache the result across calls, regardless of the
539+
/// position passed (assuming it is the same underlying symbol).
524540
func swiftName(
525541
at position: Position,
526542
in snapshot: DocumentSnapshot,
@@ -568,9 +584,7 @@ extension SourceKitServer {
568584
}
569585
let definitionDocumentUri = DocumentURI(URL(fileURLWithPath: definitionSymbol.location.path))
570586

571-
let isObjectiveCSelector =
572-
definitionLanguage == .objective_c
573-
&& (definitionSymbol.symbol.kind == .instanceMethod || definitionSymbol.symbol.kind == .classMethod)
587+
let isObjectiveCSelector = definitionLanguage == .objective_c && definitionSymbol.symbol.kind.isMethod
574588

575589
guard
576590
let nativeLanguageService = await self.languageService(
@@ -931,7 +945,7 @@ extension SwiftLanguageServer {
931945
newName newTranslatableName: TranslatableName
932946
) async throws -> [TextEdit] {
933947
// Pick any location for the name translation.
934-
// They should all refer to the same declaration, so sourcekitd doens't care which one we pick.
948+
// They should all refer to the same declaration, so sourcekitd doesn't care which one we pick.
935949
guard let renameLocationForNameTranslation = renameLocations.first else {
936950
return []
937951
}

Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ struct RelatedIdentifiersResponse {
5454
/// The compound decl name at the requested location. This can be used as `name` parameter to a
5555
/// `find-syntactic-rename-ranges` request.
5656
///
57-
/// `nil` if `sourcekitd` is too old and doesn't return the `name` as part of the related identifiers request.
57+
/// `nil` if `sourcekitd` is too old and doesn't return the `name` as part of the related identifiers request or
58+
/// `relatedIdentifiers` is empty (eg. when performing a related identifiers request on `self`).
5859
let name: String?
5960
}
6061

0 commit comments

Comments
 (0)