Add a ShowFocusedDiagnosticsRequest LSP extension and wire it up to inferred types remarks #2272
Conversation
…nferred types remarks
owenv
requested review from
ahoppen,
bnbarham,
hamishknight and
rintaro
as code owners
| } | ||
| } | ||
|
|
||
| package func additionalCompilerArgs(line: Int, column: Int) -> [String] { |
There was a problem hiding this comment.
I would prefer if we used SourceKitDPosition instead of line and column here to indicate whether we are talking about 1 or 0-based line/column and whether we use UTF-8 or UTF-16 columns. If you convert between Position and SourceKitDPosition using DocumentSnapshot.sourcekitdPosition, that should also take care of the UTF-16 to UTF-8 conversion that is currently not handled.
| 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.
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.
| @@ -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.
Could you write a SkipUnless function that skips this test if the compiler doesn’t support -Rinferred-types-at? This is important because people might be developing SourceKit-LSP with toolchains that are older.
|
|
||
| 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.
Could you also check the diagnostic’s kind (ie. that it’s not an error). I think that’s kind of interesting as well.
| * The `DocumentUri` of the text document in which to present the diagnostics. | ||
| */ | ||
| uri: DocumentUri; | ||
| } |
There was a problem hiding this comment.
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 parameters
interface TextDocumentAnnotation {
/**
* The range of the document to which the annotation applies.
*/
range: Range;
/**
* The annotation to show at the given range.
*/
message: string;
}
export interface ShowTextDocumentAnnotationsParams {
/**
* The `DocumentUri` of the text document in which to present the annotations.
*/
uri: DocumentUri;
/**
* Array of annotations to display
*/
diagnostics: TextDocumentAnnotation[];
}Description for the request could be something like
Request from the server to the client to display supplementary annotations for a source file.
The annotations should be displayed in a transient style that can be dismissed by the user when they are no longer of interest.
|
thanks - I'll take a closer look at the feedback in the next week or so |
The goal here is to provide a general way for the compiler to provide on-demand visualizations/markup of source focused on a particular region of code the user is working in. Initially, I've wired this up to the remark added in swiftlang/swift#84032 which emits remarks for each type inferred in an expression at the given position.