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.

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

Support for flexible named arg matching #288

Open
AArnott opened this issue Jun 4, 2019 · 4 comments
Open

Support for flexible named arg matching #288

AArnott opened this issue Jun 4, 2019 · 4 comments

Comments

@AArnott
Copy link
Member

AArnott commented Jun 4, 2019

The LSP spec always uses named arguments. It assumes that adding arguments is a non-breaking change. For StreamJsonRpc though, a client adding or removing a named argument is a breaking change for a server because our method overload selector works by matching every single named argument to every single method parameter.

Today, StreamJsonRpc users that implement LSP servers do so by accepting a single JToken parameter. It may offer a preferable experience if server methods could actually use the named parameters in the method signature, without becoming brittle to LSP spec changes.

The proposal here is to add an option to JsonRpc that allows it to "fuzzy match" target methods based on named arguments. When no named argument specifies a value for a method parameter, the default value for that parameter type is provided. When no parameter on the target method matches the name of a named argument, that argument is ignored.

@AArnott
Copy link
Member Author

AArnott commented Jun 4, 2019

The good part of this proposal (compared to #289) is that it remains largely true to the JSON-RPC spec with regard to its intended treatment of named arguments. The possible negative is that server methods have to change their signature (and the author must refer to the spec to see what parameters it might accept) in order to receive additional data, so it's less discoverable than a single strongly-typed object as described in #289.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 4, 2019

I would actually go with this proposal since it matches the original idea of having named arguments. However we could never unfold them correctly in JS/TS (no reflection) but it would be the right thing to do.

When no named argument specifies a value for a method parameter, the default value for that parameter type is provided.

Could these be nullable types in C# and null be passed in the absence instead of the default value?

@AArnott
Copy link
Member Author

AArnott commented Jun 4, 2019

Yes, the default value for nullable types is null. So if a server method took an int parameter it would get 0 in the absence of an argument from the client. If that server method took int? instead, it would get null in the same circumstance, and could thus discern between a 0 and no value from the client.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 5, 2019

Wasn't aware of this.

AArnott pushed a commit that referenced this issue Oct 30, 2024
Bumps [dotnet-coverage](https://github.com/microsoft/codecoverage) from 17.12.2 to 17.12.3.
- [Commits](https://github.com/microsoft/codecoverage/commits)

---
updated-dependencies:
- dependency-name: dotnet-coverage
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants