Added missing Type Adaper for RestartArguments.arguments

[cdietrich]

Added missing Type Adaper for RestartArguments.arguments

[cdietrich]

is this correct. without it will stackoverflow

[cdietrich]

is there a better way to test such things?

[pisv]

LGTM, with just a few marginal notes:

• Names of existing type adapter factories in org.eclipse.lsp4j.adapters end with Adapter rather than AdapterFactory. So it seems more appropriate to name this class RestartArgumentsTypeAdapter to match the already established naming convention, although the name RestartArgumentsTypeAdapterFactory might be more correct from the technical point of view.

• The test case looks fine to me. Note that this is the first test case for type adapter factories in LSP4J, so there are currently no established convention on how to test such things.

• I think the check ELEMENT_TYPE.equals(type) is correct and is necessary if the type adapter factory is registered globally with GsonBuilder (as done in the test case).

  Existing type adapter factories, which miss this check, don't have the mentioned problem with stackoverlow only because they are never registered globally, but are targeted precisely via annotations.

  It would probably be good to also add this check to existing type adapter factories to make the code more tolerant to usage in different contexts.

[cdietrich]

renamed the class. i wonder how to test with the annotation thing instead of register globally

[pisv]

So far, we did not have specific unit tests for type adapter factories. Rather, there are sort of 'integration' tests for parsing/serializing protocol messages like DebugMessageJsonHandlerTest.

[cdietrich]

@pisv i adapted the test to test the annotated type.
now if would work without the type check
should i remove it?

[pisv]

I would suggest removing the check now for uniformity sake, and adding it later to all existing type adapter factories if/when the need arises. Just my 2c.

[pisv]

Perhaps the test case could be slightly improved by using assertEquals and comparing the deserialized object with the expected value.

[cdietrich]

done

[cdietrich]

created #623 as follow up