Investigate Diagnostic collection.

[adam-fowler]

Currently we have a number of issues regarding Diagnostic collection. In general most issues are generated by the fact we have two sources of diagnostics (sourcekit-lsp and the compiler). This issue is to discuss methods of collating these into one list.

Paths to investigate.

• Creating our own DiagnosticGroup vscode.languages.createDiagnosticCollection("test")
• Using LanguageClient middleware to capture diagnostic coming from sourcekit-lsp
• reading the diagnostic file emitted by -serialize-diagnostics-path #670

[award999]

happy to tackle this one too

[award999]

So https://github.com/swift-server/vscode-swift/issues/694 and https://github.com/swift-server/vscode-swift/issues/750 need to be considered at the same time, as they both have a common need. For right now I would say we don't want to do anything about https://github.com/swift-server/vscode-swift/issues/670 as it would involve adding in logic similar to TSCBasic for reading the bitstream format. So for showing progress and improving diagnostics, we need access to the output from the swift process. We cannot get this using a ProcessExecution or a ShellExecution for the resolved tasks. So we need to create our own CustomExecution. Since the "swift" tasks just take an arbitrary array of "args", we'll need the resulting terminal to be interactive, for example if they "run" some executable with a CLI. VSCode already has a native bundled node-pty module we can load. It is even in the remote extension host asar bundle so would work for remote workflows. I've got a POC working for Windows, macOS, and linux. The biggest risk is making sure the Pty environment behaves the same as the current ProcessExecution, but can take a look through terminalProcess.ts for some help.

With access to the swift output, we can control the diagnostic output -fdiagnostics-format=clang/msvc/vi and parse with a similar regex as the $swiftc problem matcher. There is no way to control the format of the build output, ex. [44/57] Building SomeFile.swift but this hasn't changed. Before I get into how I envision merging parsed diagnostics with sourcekit-lsp diagnostics, what are your initial thoughts @adam-fowler?

[adam-fowler]

@award999 Setting up a CustomExecution for running the swift executable tasks will open up a number of possibilities, so yeah I think you should go ahead with it.

Since the "swift" tasks just take an arbitrary array of "args", we'll need the resulting terminal to be interactive, for example if they "run" some executable with a CLI.

Not sure what interaction we will need. This should only be for running the swift executable and its children.

You should verify that environment variables aren't changing between running tasks, as changes in the environment will cause a full rebuild when running swift build. SwiftPM already filters out the environment variable VSCODE_IPC_HOOK_CLI when deciding on whether it should do a rebuild.

[award999]

Not sure what interaction we will need. This should only be for running the swift executable and its children.

For example if running executable tries to read from stdin, ex. readLine()

You should verify that environment variables aren't changing between running tasks

The environment would be set for each PTY process spawned, not the shared pseudo terminal, but absolutely will double check this.

Thanks so much for the feedback @adam-fowler 😃

[award999]

And @adam-fowler I was going to just focus on parsing diagnostics from output for now and potentially looking at reading the serialized diagnostics down the road (https://github.com/swift-server/vscode-swift/issues/670). To synchronize the parsed diagnostics and the sourcekit-lsp diagnostics to prevent duplicates, and remove stale diagnostics, my proposal is:

1. Listen for new tasks starting (vscode.tasks.onDidStartTask) and for tasks with our custom SwiftExecution, listen to the output written from the pty process

2. Use a regex (from package.json) to parse diagnostics and add them to our swift diagnostics collection

3. Add middleware hooks for provideDiagnostics and handleDiagnostics so the language client doesn't publish these to its own diagnostic collection. The middleware will pass the diagnostics on for us to handle.

4. The whole idea is to make sure we only have one diagnostic, not duplicates. Although they'd be in the same collection, the diagnostic's source could differ and would be either swiftc or sourcekitd. As user types, we can remove old swiftc diagnostics if sourcekitd is no longer reporting them

   

5. When we have a new list of diagnostics from parsing or LSP, we will match on message, and range. i.e. only one copy of diagnostics for a given range, with the same string message

6. Precedence will always be give to the sourcekitd because they give the full range (parsing only gets us start line/column) and they have quick fixes

[adam-fowler]

@award999 Can we add a PR with just the custom SwiftExecution with some hooks such that we can add parsing of diagnostics later on. And have the swift tasks use this custom execution. This should be a fairly contained change.

Otherwise all sounds good.

I don't know if it is possible but if we could somehow match the info messages we get from the compiler that are associated with an error message, and display them together that'd be awesome. Currently the problems are separated into files only. But that'd need some sort of hierarchical display in the problems view. Something to think about in the future.

[award999]

@adam-fowler is it cool to close these off as the investigation has been done, with other PRs or issues created

[adam-fowler]

@adam-fowler is it cool to close these off as the investigation has been done, with other PRs or issues created

Just catching up, which PRs or Issues are these?

[award999]

Just copied and pasted my message from other issue. In this case it is we have the other issues raised. I'll throw up a PR later this morning