types: use enum class for enums

[perrinjerome]

Description

Using enums allows more precise type checking. The current code in types.py was already using *Kind classes as if they were enums, which mypy reported as type errors:

pygls/types.py:377: error: Incompatible default for argument "severity" (default has type "int", argument has type "DiagnosticSeverity")
pygls/types.py:663: error: Incompatible default for argument "trace" (default has type "str", argument has type "Optional[Trace]")

WatchKind is handled a bit differently, because valid values can be combined with |, like the default value 7.

Code review checklist (for code reviewer to complete)

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Commit messages are meaningful (see this for details)
• Tests have been included and/or updated, as appropriate
• Docstrings have been included and/or updated, as appropriate
• Standalone docs have been updated accordingly
• CONTRIBUTORS.md was updated, as appropriate
• Changelog has been updated, as needed (see CHANGELOG.md)

[danixeee]

Looks great. I didn't run mypy for type checking before, so I am glad you have found and fixed those issues. Thanks much!

After we merge #90, you can update the changelog as well (to avoid conflicts).

[danixeee]

Could you just move this function under call_user_feature(line 70.) and perhaps rename it to default_serializer or something similar?

[perrinjerome]

Thanks, good idea, I pushed more fixup commits to address this.

[danixeee]

@perrinjerome PR #90 is merged. If you feel comfortable, you can rebase branch with master and update the changelog for this PR.

[perrinjerome]

Thanks @danixeee . I was not familiar with enum module, and after doing a bit more research, I realised that using enum.Enum was incorrect, because protocol is not deserialised as actual objects from pygls.types module.

For example when language server is implemented as:

@server.feature(COMPLETION, trigger_characters=[','])
def completions(params: CompletionParams = None) -> CompletionList:
    """Returns completion items."""
    return CompletionList(False, [])

what this function receive as params argument is not a CompletionParams, but a "simple" NamedTuple, as protocol messages are deserialized here:

pygls/pygls/protocol.py

Line 83 in df8950d

return namedtuple('Object', data.keys(), rename=True)(*data.values())

This also means that params.context.triggerKind, which is supposed to be of type TriggerKind enum is not an enum but a plain int and tests such as ( this comes from cmake-language-server )

        @self.feature(COMPLETION, trigger_characters=['{', '('])
        def completions(params: CompletionParams):
            if (params.context.triggerKind ==
                    CompletionTriggerKind.TriggerCharacter):

will be always false, because int are different from enum.

This small issue also interesting in context of #81

But the problem seems that I wrongly used enum.Enum in this patch. When using enum.IntEnum, everything works fine ( reference doc: https://docs.python.org/3/library/enum.html#intenum ) :

>>> import enum
>>> class wrong(enum.Enum):
...   one = 1
... 
>>> class ok(enum.IntEnum):
...   one = 1
... 
>>> 1 == wrong.one
False
>>> 1 == ok.one
True

So I will amend this patch to use IntEnum for int.

I will also change the string enums to inherit from str as suggested in https://docs.python.org/3/library/enum.html#others

I will also revisit the WatchKind case to use an enum.IntFlag.

[perrinjerome]

I will also revisit the WatchKind case to use an enum.IntFlag.

That's what I did as part of c062341 but IntFlag is New in version 3.6., so that's was not OK.
I'll revisit this, hopefully later this week.

[perrinjerome]

I amended to support python 3.5 , the change compared to previous version was 354ff54 .

Then I rebased on master, squashed all the intermediate "fixup" commits from this branch, added an entry in change log, edited commit message to explain the need for enum.IntEnum and str subclassing and push-force the result in this branch.

Please take another look, I think this is OK now.

[danixeee]

@perrinjerome I was quite busy these days, sorry for waiting for the approval. Great work and explanation, I am now more familiar with enums. :)

EDIT: You can now update #89, so we can merge it as well.

[perrinjerome]

Thank you @danixeee . I also learned about enums in the process :) Don't worry about the delay, everybody is busy.