-
Notifications
You must be signed in to change notification settings - Fork 327
Implement literal detection for jump-to-definition functionality #2394
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1293,3 +1293,63 @@ extension sourcekitd_api_uid_t { | |
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Literal Detection | ||
|
|
||
| extension SwiftLanguageService { | ||
| /// Check if the given position in the document is on a literal expression. | ||
| /// | ||
| /// Literal expressions (string, integer, boolean, float, array, dictionary) should not support | ||
| /// jump-to-definition since they don't have a "definition" location - they are values typed | ||
| /// directly in the code. | ||
| /// | ||
| package func isPositionOnLiteral( | ||
| _ position: Position, | ||
| in uri: DocumentURI | ||
| ) async -> Bool { | ||
| guard let snapshot = try? await latestSnapshot(for: uri) else { | ||
| return false | ||
| } | ||
|
|
||
| let tree = await syntaxTreeManager.syntaxTree(for: snapshot) | ||
| let absolutePosition = snapshot.absolutePosition(of: position) | ||
|
|
||
| // Helper function to check if a token is part of a literal expression | ||
| func isTokenInLiteral(_ token: TokenSyntax) -> Bool { | ||
| var currentNode: Syntax? = Syntax(token) | ||
| while let node = currentNode { | ||
| // Check all literal expression types | ||
| if node.is(StringLiteralExprSyntax.self) | ||
| || node.is(IntegerLiteralExprSyntax.self) | ||
| || node.is(FloatLiteralExprSyntax.self) | ||
| || node.is(BooleanLiteralExprSyntax.self) | ||
| || node.is(ArrayExprSyntax.self) | ||
| || node.is(DictionaryExprSyntax.self) | ||
| || node.is(NilLiteralExprSyntax.self) | ||
| { | ||
| return true | ||
| } | ||
| currentNode = node.parent | ||
| } | ||
| return false | ||
|
Comment on lines
+1320
to
+1334
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will also prevent all jump-to-definition functionality for code inside literals, eg. jump-to-definition will no longer work inside literals, eg. you couldn’t jump to the definition of |
||
| } | ||
|
|
||
| // Find the token at this position | ||
| if let token = tree.token(at: absolutePosition) { | ||
| if isTokenInLiteral(token) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| // Also check the token before the cursor position, as users might click right after a literal | ||
| if absolutePosition.utf8Offset > 0, | ||
| let tokenBefore = tree.token(at: AbsolutePosition(utf8Offset: absolutePosition.utf8Offset - 1)) | ||
| { | ||
| if isTokenInLiteral(tokenBefore) { | ||
| return true | ||
| } | ||
| } | ||
|
Comment on lines
+1344
to
+1351
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, that’s quite unfortunate. I would prefer if we could find this previous token using |
||
|
|
||
| return false | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do this check in
SwiftLanguageService.symbolInfo. I think we want to suppress the jump-to-definition kind of behavior as well for all the callers ofsymbolInfo(implementation, definition, call hierarchy, type hierarchy, find references).