language-server: Don't break on bad arguments

[kastermester]

When dealing with directives without arguments specified,
such as when using Relay - the argument from typeinfo can be
null/undefined.

This change simply avoids throwing errors when hovering these
arguments - and instead provides no help on hover.

I couldn't find any tests for the language server regarding these
parts, but I tried my best testing it in a local project.

TODO:

• Update CHANGELOG.md* with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[apollo-cla]

@kastermester: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[kastermester]

I'm not quite sure why the tests are breaking - as far as I can see my PR did not change anything related to these failures.

[JakeDawkins]

Hey @kastermester! Thanks for the PR :) Can you drop me a link to a repo where I can reproduce and test these changes? Or at least a snippet of a schema/operation combo that I can use?

[kastermester]

I will try to find time to put up a repo with a reproduce. But in short it happens when using this package: https://github.com/relay-tools/vscode-apollo-relay

Because it tries to teach the apollo tooling some Relay-isms, when hovering over arguments for directives that do not have documentation/types associated with them, this happens. In Relay this happens when hovering over arguments to ie. @arguments directive.

[kastermester]

So ie. create a directive like this:

directive @arguments on FRAGMENT_SPREAD

And apply it to a fragment spread adding arguments to it.

[zionts]

Hi @kastermester , thanks for pointing this out! This seems like something that should generally be done based on the null-handling in the other cases, rather than something specific to Relay's provider, unless I'm misinterpreting?

[kastermester]

Precisely. All null values should ideally be handled. I’m not entirely sure which other situations there are though.

[zionts]

@kastermester this change seems pretty safe to me; seems to be porting a pattern that's elsewhere to a code path that seems to have missed it, which can cause a poor VS Code experience. Seems like the CHANGELOG is out of date, but that's a quick fix-up. @JakeDawkins, do you have any qualms against merging this?