Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion elpy/rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import json
import sys
import traceback
from pathlib import Path



class JSONRPCServer(object):
Expand Down Expand Up @@ -67,14 +69,18 @@ def read_json(self):
raise EOFError()
return json.loads(line)

def json_defaults(self, x):
if isinstance(x, Path):
return str(x)

def write_json(self, **kwargs):
"""Write an JSON object on a single line.

The keyword arguments are interpreted as a single JSON object.
It's not possible with this method to write non-objects.

"""
self.stdout.write(json.dumps(kwargs) + "\n")
self.stdout.write(json.dumps(kwargs, default=self.json_defaults) + "\n")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for json.dumps say that it should raise a TypeError if the object cannot be serializable, (In this case, all other objects that are not Path and are not primitive types will be encoded as null since json_defaults is implicitly returning None.

>>> import json
>>> json.dumps(None)
'null'
>>> 

Also relevant CPython source code that shows this will happen

I don't think this is the intended outcome?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for json.dumps say that it should raise a TypeError if the object cannot be serializable, (In this case, all other objects that are not Path and are not primitive types will be encoded as null since json_defaults is implicitly returning None.

>>> import json
>>> json.dumps(None)
'null'
>>> 

Also relevant CPython source code that shows this will happen

I don't think this is the intended outcome?

Hi @gopar. Sorry I've been extremely busy. Yes, definitely that shouldn't be there. I had completely overlooked that and forgot to return the default value. It should be:

def json_defaults(x):
    if isinstance(x, Path): 
         return str(x) 
    else: 
        return json.dumps(x)

See docstring of default in JSONEncoder.

Actually pydantic would be a good approach to go for it as it is primarily a parsing library. Though I think @gfederix's PR (See discussion in #1895) requires a few fixes.

At this point I'm not sure which things to refactor, for example in rpc.py, as there are parallel changes in #1895 which are not yet merged. Any suggestions?

self.stdout.flush()

def handle_request(self):
Expand Down