Repository: openlawlibrary/pygls
Pull request #56 (2019-03-22 08:31:12)

Fix handling notification/request parameters that is not used for class fields name from the server
Sorry, I accidentally send incomplete request...
I will update it to make this request complete.

## Description (e.g. "Related to ...", etc.)

This request is to handle message fields that is not valid as python class fields (such as `__dummy__`) from the client.

For example, [Eglot](https://github.com/joaotavora/eglot), which is a LSP client for emacs send `initialized` notification with `__dummy__` parameter.

Currently language servers that are based on pygls fail with the following exception message:
```
Process EGLOT (v1/cwl-mode) stderr finished
Traceback (most recent call last):
  File "/Users/tanjo/repos/cwl-language-server/cwl_language_server/main.py", line 42, in <module>
    server.start_io()
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/site-packages/pygls/server.py", line 164, in start_io
    self.lsp.data_received))
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/asyncio/base_events.py", line 468, in run_until_complete
    return future.result()
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/site-packages/pygls/server.py", line 65, in aio_readline
    proxy(body)
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/site-packages/pygls/protocol.py", line 405, in data_received
    object_hook=deserialize_message))
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/json/__init__.py", line 367, in loads
    return cls(**kw).decode(s)
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/json/decoder.py", line 355, in raw_decode
    obj, end = self.scan_once(s, idx)
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/site-packages/pygls/protocol.py", line 86, in deserialize_message
    data.keys())(*data.values())
  File "/Users/tanjo/.pyenv/versions/3.6.5/lib/python3.6/collections/__init__.py", line 409, in namedtuple
    '%r' % name)
ValueError: Field names cannot start with an underscore: '__dummy__'
```

The reason is that `__dummy__` that is sent from Eglot is not valid identifier for the field name in python.
By using `rename=True` for `namedtuple`, such fields are replaced with `_1`, `_2` and so on.
Their names are very annoying but is better than server terminations.

Note: `cwl_language_server` is our project for a language server for Common Workflow Language.
We currently use [pylspclient](https://github.com/yeger00/pylspclient) but we start trying pygls as an alternative.

## Code review checklist (for code reviewer to complete)

- [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
- [x] Title summarizes what is changing
- [x] Commit messages are meaningful (see [this][commit messages] for details)
- [x] Tests have been included and/or updated, as appropriate
- [ ] Docstrings have been included and/or updated, as appropriate
  - This request does not add any functions.
- [ ] Standalone docs have been updated accordingly
  - This request does not contain the update that needs the doc update.
- [x] [CONTRIBUTORS.md][contributors] was updated, as appropriate
- [ ] Changelog has been updated, as needed (see [CHANGELOG.md][changelog])
  - No notable changes

[changelog]: https://github.com/openlawlibrary/pygls/blob/master/CHANGELOG.md
[commit messages]: https://chris.beams.io/posts/git-commit/
[contributors]: https://github.com/openlawlibrary/pygls/blob/master/CONTRIBUTORS.md

danixeee: @tom-tan Good catch and thank you for your contribution!

Maybe we should consider adding multiple clients for our _json-extension_ example, because we are testing it just with VS Code.

P.S. Check-boxes in _Code review checklist_ from the PR template should be checked by a code reviewer, so I will uncheck them.
tom-tan: Thank you for your suggestion.

I will update the code and add a test for it.


tom-tan: Done!

I added a test for `deserialized_message`.
I used an actual input for deserializing rather than deserializing `'{ "__dummy__": true }'` directly.
IMO it is easier to understand what this test wants to check.
danixeee: Looks go to me.

@tom-tan Can you please update the [CHANGELOG](https://github.com/openlawlibrary/pygls/blob/master/CHANGELOG.md) file? We decided that every PR should update the CHANGELOG as well, so we don't have to go through all PRs and see what was changed.

To make it easier for you, here is how it should look:
```
# Changelog

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog][keepachangelog],
and this project adheres to [Semantic Versioning][semver].

## [Unreleased]

### Changed

- Your message here ([#56])

[#56]: https://github.com/openlawlibrary/pygls/pull/54

## [0.7.4] - 03/21/2019

### Added

- Add Pull Request template ([#54])
```
tom-tan: Done!
I updated `CHANGELOG`.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ef3076a..1794027 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning][semver].
 
 ## [Unreleased]
 
+### Changed
+
+- Fix handling parameters whose names are reserved by Python ([#56])
+
+[#56]: https://github.com/openlawlibrary/pygls/pull/56
+
 ## [0.7.4] - 03/21/2019
 
 ### Added
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
index 48307c3..98e5a8a 100644
--- a/CONTRIBUTORS.md
+++ b/CONTRIBUTORS.md
@@ -3,4 +3,5 @@
 - [@augb](https://github.com/augb)
 - [Daniel Elero](https://github.com/danixeee)
 - [Max O'Cull](https://github.com/Maxattax97)
+- [Tomoya Tanjo](https://github.com/tom-tan)
 - [yorodm](https://github.com/yorodm)
diff --git a/pygls/protocol.py b/pygls/protocol.py
index c065d7d..181132f 100644
--- a/pygls/protocol.py
+++ b/pygls/protocol.py
@@ -82,8 +82,7 @@ def deserialize_message(data):
         else:
             return JsonRPCNotification(**data)
 
-    return namedtuple('Object',
-                      data.keys())(*data.values())
+    return namedtuple('Object', data.keys(), rename=True)(*data.values())
 
 
 def to_lsp_name(method_name):


Test patch:
diff --git a/tests/test_protocol.py b/tests/test_protocol.py
index a4ca583..8be2885 100644
--- a/tests/test_protocol.py
+++ b/tests/test_protocol.py
@@ -14,9 +14,10 @@
 # See the License for the specific language governing permissions and      #
 # limitations under the License.                                           #
 ############################################################################
+import json
 
 import pytest
-from pygls.protocol import to_lsp_name
+from pygls.protocol import to_lsp_name, deserialize_message
 from pygls.types import InitializeResult
 
 
@@ -73,3 +74,19 @@ def test_initialize_should_return_server_capabilities(client_server):
     server_capabilities = server.lsp.bf_initialize(params)
 
     assert isinstance(server_capabilities, InitializeResult)
+
+
+def test_deserialize_message_with_reserved_words_should_pass_without_errors(client_server):
+    params_with_reserved_word = '''
+    {
+        "jsonrpc": "2.0",
+        "method": "initialized",
+        "params": {
+            "__dummy__": true
+        }
+    }
+    '''
+    obj = json.loads(params_with_reserved_word,
+                     object_hook=deserialize_message)
+    assert isinstance(obj, object)
+    assert "_0" in dir(obj.params)