Skip to content
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.

Sign up for GitHub

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

Jump to bottom

De-duplicate errors from sourcekitd and swiftc in the Problems view #653

Closed
ahoppen opened this issue Dec 5, 2023 · 6 comments · Fixed by #821
Closed

De-duplicate errors from sourcekitd and swiftc in the Problems view #653

ahoppen opened this issue Dec 5, 2023 · 6 comments · Fixed by #821
Assignees
@award999
Labels
enhancement New feature or request

Comments

@ahoppen
Copy link

ahoppen commented Dec 5, 2023

In the problems pane, it would be nice if we could de-duplicate errors reported by sourcekitd and swiftc. My suggestion for this would be: If sourcekitd reports an error at the same location with the same message as swiftc, hide it.
@adam-fowler
Copy link
Contributor

I would love to be able to do this, but VSCode considers these two class of errors as coming from different sources and will not merge them.

If I had access to the full problems list I could filter the sourcekitd errors, but I haven't found a way to access the problems list. Also the problem with filtering the sourcekitd errors is I would lose the codeActions attached to these errors.

@ahoppen
Copy link
Author

ahoppen commented Dec 6, 2023

And I suppose (sorry if this is a stupid question) you can’t intercept the errors coming from sourcekit-lsp, right to merge them with build issues and then publish them using your own diagnostics provider.

Interestingly, I just checked what clangd + CMake does and it doesn’t show the build errors in the Problems View at all. I don’t think that’s the way to go but I find at an interesting data point nonetheless.

@adam-fowler
Copy link
Contributor

I can't intercept the build errors, but I can intercept the sourcekit-lsp errors. But without knowing what the build errors are I'm not sure what I can do with the sklsp errors.

Yeah not showing build errors would be bad. Sourcekit-LSP only picks up the errors for the files that are open, if I understand correctly

@daveyc123 daveyc123 added the needs investigation Issue requires some investigation before it can be worked on label Apr 19, 2024
@matthewbastien
Copy link
Member

Seems related to #750. I can investigate this one as well. I know that TypeScript handles this well. Would be nice to see how they do it.

@adam-fowler
Copy link
Contributor

I don't think the Typescript extension displays errors from the compiler. The only errors displayed come from the LSP server.

@adam-fowler
Copy link
Contributor

See #759

award999 added a commit to award999/vscode-swift that referenced this issue May 6, 2024
@award999
Add a CustomExecution for swift tasks
6cac38f 
To give us more control of the swift process and allow us
to consume the output from swift, introduce our own
CustomExecution. This will set us up to address swiftlang#750, swiftlang#653
and swiftlang#694.

The CustomExecution will load the asar bundled node-pty
module that ships with vscode. This way we don't need to
build for every platform and arch
@award999 award999 mentioned this issue May 6, 2024
Merged
award999 added a commit to award999/vscode-swift that referenced this issue May 6, 2024
@award999
Add a CustomExecution for swift tasks
3a409b1 
To give us more control of the swift process and allow us
to consume the output from swift, introduce our own
CustomExecution. This will set us up to address swiftlang#750, swiftlang#653
and swiftlang#694.

The CustomExecution will load the asar bundled node-pty
module that ships with vscode. This way we don't need to
build for every platform and arch
@daveyc123 daveyc123 assigned award999 and unassigned matthewbastien May 8, 2024
adam-fowler pushed a commit that referenced this issue May 9, 2024
@award999
Add a CustomExecution for swift tasks (#781)
47a3f1d 
* Add a CustomExecution for `swift` tasks

To give us more control of the swift process and allow us
to consume the output from swift, introduce our own
CustomExecution. This will set us up to address #750, #653
and #694.

The CustomExecution will load the asar bundled node-pty
module that ships with vscode. This way we don't need to
build for every platform and arch

* Disable conpty when debugging on windows

Only an issue when debugging extension. Packaged extension
works fine

* Fix review comments

Biggest change is making sure 'cwd' is set for custom execution

* Add some docstring comments

Add comments to make the relationship/usage between SwiftExecution,
SwiftPseudoterminal, and SwiftProcess more clear
award999 added a commit to award999/vscode-swift that referenced this issue May 28, 2024
@award999
Deduplicate swift diagnostics
e5ea297 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
award999 added a commit to award999/vscode-swift that referenced this issue May 28, 2024
@award999
Deduplicate swift diagnostics
e19d6bc 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
award999 added a commit to award999/vscode-swift that referenced this issue May 28, 2024
@award999
Deduplicate swift diagnostics
3d793dd 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
@award999 award999 mentioned this issue May 28, 2024
Merged
@award999 award999 added enhancement New feature or request and removed needs investigation Issue requires some investigation before it can be worked on labels May 28, 2024
award999 added a commit to award999/vscode-swift that referenced this issue May 29, 2024
@award999
Deduplicate swift diagnostics
4199559 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
award999 added a commit to award999/vscode-swift that referenced this issue May 30, 2024
@award999
Deduplicate swift diagnostics
a9f4b0e 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
award999 added a commit to award999/vscode-swift that referenced this issue May 31, 2024
@award999
Deduplicate swift diagnostics
7a8fda3 
Fixes swiftlang#653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself
adam-fowler pushed a commit that referenced this issue Jun 5, 2024
@award999
Deduplicate swift diagnostics (#821)
9c1069d 
* Deduplicate swift diagnostics

Fixes #653

* Keeping "$swiftc" problem matcher around in case it is still being used
* Add "diagnosticsCollection" setting to give more control on which
  diagnostics are kept around and provided to the user
* Setup diagnostics from task output and hooking into sourcekit-lsp
* Handle merging based on "diagnosticsCollection" setting

This does not include serialized diagnostics which will involve changes
in swift itself

* Add some more diagnostics tests

Have tests for parsing from task output, controlling the parsing of a
diagnostic across multiple onWrite callbacks, and for handling the
merging of diagnostics based on the diagnosticsCollection setting

* Fix some issues with cleaning up old diagnostics

* Makes sure always cleanup swiftc diagnostics in case
  diagnosticsCollection setting changed
* De-duplicate error messages until swift 6 is fixed

* Make sure diagnostic message capitalization is consistent

Whether swiftc or SourceKit capitalize the first letter of their message
or not, we'll make sure always capitalize so message user sees is
consistent, and we can properly merge from the different sources

* Parse note diagnotics as nested related information

Notes are attached to the preceding error or warning, so when we come
across a note, add it as RelatedInformation to the previous Diagnostic

Additionally let the user configure the -diagnostic-style option, using
"llvm" as the default so we can parse these notes

* Only cleanup swiftc diagnostics after new build is done

Will listen to change in settings to clear SourceKit or swiftc errors if
they are respectfully disabled

* Simplify diagnostic merging logic

* Fix rebase error

* Fix failing CI tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@award999 award999

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deduplicate swift diagnostics award999/vscode-swift
5 participants
@ahoppen @daveyc123 @adam-fowler @matthewbastien @award999