Skip to content

Conversation

@angelozerr
Copy link
Contributor

@angelozerr angelozerr commented Nov 24, 2025

This PR fixes a design error when diagnostics must be collected. The IJavaDiagnosticsParticipant defines a method like:

List<Diagnostic> collectDiagnostics(JavaDiagnosticsContext context, IProgressMonitor monitor) throws CoreException;

which returns a list of diagnostics for the given context (a given file uri)

The collectDiagnostics should not return a list of diagnostics and should use JavaDiagnosticsContext #addDiagnostic

This PR fixes that and fixes also the file uri which is used when diagnostic is created although diagnostic doesn't store some uri.

@angelozerr angelozerr marked this pull request as ready for review November 25, 2025 12:06
@angelozerr
Copy link
Contributor Author

@datho7561 please review this PR. I know it will break the current API but I have not seen some code instead in LSP4MP which implements this API.

@turkeylurkey and your team please tell me if you are OK with this change. I have done this similar changes in IJ Quarkus.

Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes a lot of sense and cleans up the code. Thanks, Angelo!

@angelozerr
Copy link
Contributor Author

This change makes a lot of sense and cleans up the code. Thanks, Angelo!

Thanks @datho7561!

@angelozerr angelozerr added this to the 0.14.2 milestone Nov 25, 2025
@turkeylurkey
Copy link
Contributor

This change is ok for us too.

@angelozerr
Copy link
Contributor Author

This change is ok for us too.

Thanks @turkeylurkey for your reply.

I merge the PR.

@angelozerr angelozerr merged commit 68fb830 into eclipse-lsp4mp:master Nov 26, 2025
4 of 6 checks passed
@angelozerr angelozerr added the enhancement New feature or request label Nov 26, 2025
@angelozerr angelozerr self-assigned this Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants