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

Initialization fails #4

Closed
kaloyan-raev opened this issue Sep 1, 2016 · 9 comments
Closed

Initialization fails #4

kaloyan-raev opened this issue Sep 1, 2016 · 9 comments

Comments

@kaloyan-raev
Copy link
Contributor

  1. I have VS Code 1.4.0 on Fedora 24.
  2. I installed the PHP IntellSense extension as described: https://github.com/felixfbecker/vscode-php-intellisense#build-and-run-from-source
  3. I opened the dev tools in VS Code from Help > Toggle Developer Tools.
  4. I opened a PHP file in the editor.

The following error is printed in the Console of Dev Tools:
[Extension Host] Response handler 'initialize' failed with message: Cannot read property 'code' of null

@felixfbecker
Copy link
Owner

@felixfbecker
Copy link
Owner

I added a fix in felixfbecker/php-advanced-json-rpc@f9d7354 and documentSymbol works now! 🎉

@kaloyan-raev
Copy link
Contributor Author

Yes. It works. So cool! :)

@kaloyan-raev
Copy link
Contributor Author

BTW, I think you should apply the same pattern for JSON serialization everywhere. If an PHP object's member is null then it should not be included in the JSON object at all.

I checked the behavior of the Gson library (the most popular for JSON serialization/deserialization in Java) and it does exactly this.

@felixfbecker
Copy link
Owner

felixfbecker commented Sep 2, 2016

@kaloyan-raev Yes, that would be ideal, but not possible in PHP. You cannot find out at serialization time if a property was set to null explicitely or if it was simply not set, because in PHP a property is by default null if it has not been set. It works in this specific case because the spec says only error or result should ever be defined, never both, but it won't work for all the structures in the protocol where null is actually an accepted value. And it would be a huge amount of work to add the JsonSerializable interface everywhere. It would be much nicer if VS Code would just treat undefined and null the same.

I implemented the fix because I wanted advanced-json-rpc to be JSON RPC spec compatible, but for the VS Code language server protocol, we might actually have some influence on how they treat different values, and this makes it easier for developers.

@kaloyan-raev
Copy link
Contributor Author

It is absolutely the same in Java (I am Java developer with some PHP knowledge). This is why I checked how the Gson library handles it.

What about if here: https://github.com/felixfbecker/php-advanced-json-rpc/blob/master/lib/Message.php#L44

instead of

return json_encode($this);

you have

return json_encode(array_filter((array) $obj));

Wouldn't be that a universal solution?

@felixfbecker
Copy link
Owner

felixfbecker commented Sep 2, 2016

No, because the spec for example says that the id in a response MUST be null if there was a parse error parsing the request and the id is unknown. It MUST be defined though, because a message without an id is considered a notification.

If we did it like this in all protocol structures, there would be no way to serialize actual null properties, that have been explicitly set to null.

Side note: you cannot cast an object directly to an array, it will include private properties. You have to do get_object_vars().

@kaloyan-raev
Copy link
Contributor Author

OK. You are right.

@felixfbecker
Copy link
Owner

I don't like this fact about PHP either. It sucks for ORMs, you have no way to see if a column is NULL in the DB or it simply is "unknown" because wasn't eager-loaded / set after construction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants