search

eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
613 stars 145 forks source link

Add missing type adapters #669

Closed pisv closed 2 years ago

pisv commented 2 years ago

Adds type adapters for DocumentDiagnosticReport and WorkspaceDocumentDiagnosticReport.

Fixes #668.

pisv commented 2 years ago

Unfortunately, the fix didn't work properly when testing the snapshot build in my local setting. JsonParseException: Ambiguous Either type: token BEGIN_OBJECT matches both alternatives is still thrown when reading a response to textDocument/diagnostic or workspace/diagnostic requests.

The reason is that EitherTypeAdapter, which is registered in MessageJsonHandler.getDefaultGsonBuilder(), take precedence over DocumentDiagnosticReportTypeAdapter and WorkspaceDocumentDiagnosticReportTypeAdapter registered via @JsonAdapter annotation. As stated in the annotation contract, "Field annotations take precedence over GsonBuilder-registered type adapters, which in turn take precedence over annotated types."

I have changed the DocumentDiagnosticReportTypeAdapterTest and WorkspaceDocumentDiagnosticReportTypeAdapterTest to use the Gson instance obtained from MessageJsonHandler rather than a default Gson instance. Now they started to fail with the same error as observed in my local setting.

I'm going to try a different approach, which is based on using @ResponseJsonAdapter annotation on the corresponding request methods instead of @JsonAdapter on the corresponding types. Hopefully, this can be done soon.

pisv commented 2 years ago

I have verified that the current fix works fine in my local setup. All tests are now green too. The PR is ready for review.