-
Notifications
You must be signed in to change notification settings - Fork 327
Add a ShowFocusedDiagnosticsRequest LSP extension and wire it up to inferred types remarks #2272
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 |
---|---|---|
@@ -0,0 +1,48 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2025 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
/// Request from the server to the client to show focused diagnostics **(LSP Extension)** | ||
/// | ||
/// This request is handled by the client to display focused diagnostic information | ||
/// related to a subset of the source. | ||
/// | ||
/// - Parameters: | ||
/// - diagnostics: Array of diagnostics to display | ||
/// - uri: Document URI in which to present the diagnostics | ||
/// | ||
/// - Returns: `ShowFocusedDiagnosticsResponse` which indicates the `success` of the request. | ||
/// | ||
/// ### LSP Extension | ||
/// | ||
/// This request is an extension to LSP supported by SourceKit-LSP. | ||
/// It requires the experimental client capability `workspace/showFocusedDiagnostics` to use. | ||
public struct ShowFocusedDiagnosticsRequest: RequestType { | ||
public static let method: String = "workspace/showFocusedDiagnostics" | ||
public typealias Response = ShowFocusedDiagnosticsResponse | ||
|
||
public var diagnostics: [Diagnostic] | ||
public var uri: DocumentURI | ||
|
||
public init(diagnostics: [Diagnostic], uri: DocumentURI) { | ||
self.diagnostics = diagnostics | ||
self.uri = uri | ||
} | ||
} | ||
|
||
/// Response to indicate the `success` of the `ShowFocusedDiagnosticsRequest` | ||
public struct ShowFocusedDiagnosticsResponse: ResponseType { | ||
public var success: Bool | ||
|
||
public init(success: Bool) { | ||
self.success = success | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2025 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
package import LanguageServerProtocol | ||
import SourceKitD | ||
|
||
/// Describes a kind of focused remarks supported by the compiler. Remarks should be exposed via | ||
/// a flag which accepts a position in the document as a <line:column> pair and only emits | ||
/// diagnostics relavant to that position (e.g. expressions or function bodies with source ranges that | ||
/// contain the position). | ||
package enum FocusedRemarksKind: String, CaseIterable, Codable { | ||
case showInferredTypes | ||
|
||
package var defaultTitle: String { | ||
switch self { | ||
case .showInferredTypes: | ||
return "Show Inferred Types" | ||
} | ||
} | ||
|
||
package func additionalCompilerArgs(line: Int, column: Int) -> [String] { | ||
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. I would prefer if we used |
||
switch self { | ||
case .showInferredTypes: | ||
return [ | ||
"-Xfrontend", | ||
"-Rinferred-types-at", | ||
"-Xfrontend", | ||
"\(line):\(column)", | ||
] | ||
} | ||
} | ||
} | ||
|
||
package struct FocusedRemarksCommand: SwiftCommand { | ||
package static let identifier: String = "focused.remarks.command" | ||
|
||
package let commandType: FocusedRemarksKind | ||
package var title: String | ||
package let position: Position | ||
package let textDocument: TextDocumentIdentifier | ||
|
||
package init(commandType: FocusedRemarksKind, position: Position, textDocument: TextDocumentIdentifier) { | ||
self.commandType = commandType | ||
self.position = position | ||
self.textDocument = textDocument | ||
self.title = commandType.defaultTitle | ||
} | ||
|
||
package init?(fromLSPDictionary dictionary: [String: LSPAny]) { | ||
guard case .dictionary(let documentDict)? = dictionary[CodingKeys.textDocument.stringValue], | ||
case .string(let title)? = dictionary[CodingKeys.title.stringValue], | ||
case .dictionary(let positionDict)? = dictionary[CodingKeys.position.stringValue], | ||
case .string(let commandTypeString)? = dictionary[CodingKeys.commandType.stringValue] | ||
else { | ||
return nil | ||
} | ||
guard let position = Position(fromLSPDictionary: positionDict), | ||
let textDocument = TextDocumentIdentifier(fromLSPDictionary: documentDict), | ||
let commandType = FocusedRemarksKind(rawValue: commandTypeString) | ||
else { | ||
return nil | ||
} | ||
|
||
self.init( | ||
commandType: commandType, | ||
title: title, | ||
position: position, | ||
textDocument: textDocument | ||
) | ||
} | ||
|
||
package init( | ||
commandType: FocusedRemarksKind, | ||
title: String, | ||
position: Position, | ||
textDocument: TextDocumentIdentifier | ||
) { | ||
self.commandType = commandType | ||
self.title = title | ||
self.position = position | ||
self.textDocument = textDocument | ||
} | ||
|
||
package func encodeToLSPAny() -> LSPAny { | ||
return .dictionary([ | ||
CodingKeys.title.stringValue: .string(title), | ||
CodingKeys.position.stringValue: position.encodeToLSPAny(), | ||
CodingKeys.textDocument.stringValue: textDocument.encodeToLSPAny(), | ||
CodingKeys.commandType.stringValue: .string(commandType.rawValue), | ||
]) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -892,6 +892,7 @@ extension SwiftLanguageService { | |
(retrieveSyntaxCodeActions, nil), | ||
(retrieveRefactorCodeActions, .refactor), | ||
(retrieveQuickFixCodeActions, .quickFix), | ||
(retrieveFocusedRemarksCodeActions, nil), | ||
] | ||
let wantedActionKinds = req.context.only | ||
let providers: [CodeActionProvider] = providersAndKinds.compactMap { | ||
|
@@ -1028,6 +1029,21 @@ extension SwiftLanguageService { | |
return codeActions | ||
} | ||
|
||
func retrieveFocusedRemarksCodeActions(_ params: CodeActionRequest) async throws -> [CodeAction] { | ||
guard self.capabilityRegistry.clientHasExperimentalCapability(ShowFocusedDiagnosticsRequest.method) else { | ||
return [] | ||
} | ||
return FocusedRemarksKind.allCases.map { commandType in | ||
let command = FocusedRemarksCommand( | ||
commandType: commandType, | ||
position: params.range.lowerBound, | ||
textDocument: params.textDocument | ||
) | ||
.asCommand() | ||
return CodeAction(title: command.title, kind: nil, command: command) | ||
} | ||
} | ||
|
||
package func inlayHint(_ req: InlayHintRequest) async throws -> [InlayHint] { | ||
let uri = req.textDocument.uri | ||
let infos = try await variableTypeInfos(uri, req.range) | ||
|
@@ -1119,13 +1135,51 @@ extension SwiftLanguageService { | |
try await semanticRefactoring(command) | ||
} else if let command = req.swiftCommand(ofType: ExpandMacroCommand.self) { | ||
try await expandMacro(command) | ||
} else if let command = req.swiftCommand(ofType: FocusedRemarksCommand.self) { | ||
try await executeFocusedRemarksCommand(command) | ||
} else { | ||
throw ResponseError.unknown("unknown command \(req.command)") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
package func executeFocusedRemarksCommand(_ command: FocusedRemarksCommand) async throws { | ||
let snapshot = try self.documentManager.latestSnapshot(command.textDocument.uri) | ||
let buildSettings = await self.buildSettings( | ||
for: command.textDocument.uri, | ||
fallbackAfterTimeout: true | ||
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. I don’t think we should use fallback build settings after a timeout. The focused remarks are not going to be useful with fallback build settings and we don’t have a way to refresh them when we would get real build settings. |
||
) | ||
|
||
guard var buildSettings else { | ||
throw ResponseError.unknown("Unable to get build settings for '\(command.textDocument.uri)'") | ||
} | ||
|
||
let line = command.position.line + 1 | ||
let column = command.position.utf16index + 1 | ||
buildSettings.compilerArguments.append( | ||
contentsOf: command.commandType.additionalCompilerArgs( | ||
line: line, | ||
column: column | ||
) | ||
) | ||
|
||
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport( | ||
for: snapshot, | ||
buildSettings: SwiftCompileCommand(buildSettings) | ||
) | ||
|
||
let showFocusedDiagnosticsRequest = ShowFocusedDiagnosticsRequest( | ||
diagnostics: diagnosticReport.items, | ||
uri: snapshot.uri | ||
) | ||
|
||
guard let sourceKitLSPServer else { | ||
throw ResponseError.unknown("Connection to the editor closed") | ||
} | ||
_ = try await sourceKitLSPServer.sendRequestToClient(showFocusedDiagnosticsRequest) | ||
} | ||
|
||
package func getReferenceDocument(_ req: GetReferenceDocumentRequest) async throws -> GetReferenceDocumentResponse { | ||
let referenceDocumentURL = try ReferenceDocumentURL(from: req.uri) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,4 +189,41 @@ final class ExecuteCommandTests: XCTestCase { | |
req.arguments = [1, 2, "", metadata.encodeToLSPAny()] | ||
XCTAssertEqual([1, 2, ""], req.argumentsWithoutSourceKitMetadata) | ||
} | ||
|
||
func testShowInferredTypesCommand() async throws { | ||
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. Could you write a |
||
let testClient = try await TestSourceKitLSPClient( | ||
capabilities: ClientCapabilities(experimental: [ | ||
"workspace/showFocusedDiagnostics": .dictionary(["supported": .bool(true)]) | ||
]) | ||
) | ||
let uri = DocumentURI(for: .swift) | ||
let positions = testClient.openDocument( | ||
""" | ||
func test() -> Int { | ||
11️⃣0 | ||
} | ||
""", | ||
uri: uri | ||
) | ||
let textDocument = TextDocumentIdentifier(uri) | ||
let command = FocusedRemarksCommand( | ||
commandType: .showInferredTypes, | ||
position: positions["1️⃣"], | ||
textDocument: textDocument | ||
) | ||
let metadata = SourceKitLSPCommandMetadata(textDocument: textDocument) | ||
let request = ExecuteCommandRequest( | ||
command: FocusedRemarksCommand.identifier, | ||
arguments: [command.encodeToLSPAny(), metadata.encodeToLSPAny()] | ||
) | ||
let expectation = self.expectation(description: "Handle ShowFocusedDiagnosticsRequest") | ||
|
||
testClient.handleSingleRequest { (req: ShowFocusedDiagnosticsRequest) in | ||
expectation.fulfill() | ||
XCTAssertEqual(req.diagnostics.map(\.message), ["integer literal was inferred to be of type 'Int'"]) | ||
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. Could you also check the diagnostic’s kind (ie. that it’s not an |
||
return ShowFocusedDiagnosticsResponse(success: true) | ||
} | ||
_ = try await testClient.send(request) | ||
try await fulfillmentOfOrThrow(expectation) | ||
} | ||
} |
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’m not sure if diagnostics are the best currency here. If I understand you correctly, you want to show additional information for source ranges, so what do you think about changing this request to
textDocument/showAnnotations
with the following parametersDescription for the request could be something like