Import from typing, typing_extensions and enum modules on Python 3.8

[jwortmann]

On Python 3.8 enum and typing are part of the standard library.

.core.typing still contains all the fake typing classes, because many LSP-* plugins import from LSP.plugin.core.typing.

[jfcherng]

Fwiw, with from __future__ import annotations, type annotations are just ignored at runtime. People can use something like tuple[str, ...] | dict[str, Any] | None for type annotations in py38. Most of from typing import ... can be removed (later, or maybe not interested). ruff can do this automatically for a whole project but it's unsafe if there is one used outside type annotation.

[jwortmann]

I would say that we can make those changes step by step in separate PRs. So the next steps could be

• use modern type union syntax
  • replace Union[Foo, Bar] with Foo | Bar
  • replace Optional[Foo] with Foo | None
• replace generic typing classes like Dict, List, Tuple with builtins dict, list, tuple

The pipe operator was introduced in Python 3.10 and the syntax with dict etc. in Python 3.9, so we would need to have from __future__ import annotations in all files.

And this is probably significantly more work if done manually throughout the whole code base, so we should check how well it would work to do it automatically.

[jwortmann]

By the way, with this PR there are still 3 classes required to be imported from the local .core.typings, because they were introduced after Python 3.8: TypeGuard, NotRequired, and ParamSpec.
I've noticed that typing_extensions is on Package Control (https://github.com/packagecontrol/channel/blob/6faa8ffb43475f829ee95936955a962cce47d6d3/repository.json#L1387), so I wonder whether we could use that to avoid those 3 remaining imports.

[predragnikolic]

Nice!

I would rather not rush into merging this in feat/py38,
I hope that you are OK if this PR is merged sometime after the 19th April? :)

[jwortmann]

Okay, I can change the target branch then and rebase.

---

Regarding typing_extensions, I can see that it was merged two weeks ago in packagecontrol/channel@cd8ec01, but it's not (yet?) in the https://packagecontrol.github.io/channel/channel_v4.json file used by Package Control. So maybe we need to wait a bit until it gets updated.

I think with from __future__ import annotations we could alternatively also do

if TYPE_CHECKING:
    from typing import NotRequired, TypeGuard

but it wouldn't work with ParamSpec because that one is used outside of type annotations.

[predragnikolic]

Will check the typing_extensions ,
if it works, then NotRequired, TypeGuard could be imported from there without the if TYPE_CHECKING: check... I think.

[jwortmann]

typing_extensions seems to be fixed upstream now. So the question would be if we want to have it as an additional dependency or not. With it, NotRequired, TypeGuard and ParamSpec can all be imported directly.

I missed to mention above that there is still StrEnum (used in protocol.py) that we need to import from .core.typings, but this is regardless whether typing_extensions is available or not.

[jfcherng]

I think more or less, StrEnum is just

class StrEnum(str, Enum):
    def __str__(self) -> str:
        return self.value.__str__()

[jwortmann]

There is another problem with LSP's fake enums. Currently we can use the syntax SomeEnum.SomeValue to get the value, because SomeEnum as an derived class from the fake enum is just a regular class, so we can access the attribute value directly. But if we import from the enum module, SomeEnum now is a real enum and SomeEnum.SomeValue would give something like <SomeEnum.SomeValue: 42> instead of just 42. In other words we would need to use SomeEnum.SomeValue.value instead throughout the code.

I guess I'll revert using the "real" enums, for now...

[jfcherng]

There is another problem with LSP's fake enums. Currently we can use the syntax SomeEnum.SomeValue to get the value, because SomeEnum as an derived class from the fake enum is just a regular class, so we can access the attribute value directly. But if we import from the enum module, SomeEnum now is a real enum and SomeEnum.SomeValue would give something like <SomeEnum.SomeValue: 42> instead of just 42. In other words we would need to use SomeEnum.SomeValue.value instead throughout the code.

Maybe you can try

from enum import IntEnum


class Color(IntEnum):
    RED = 10

    def __str__(self) -> str:
        return self.value.__str__()

In ST console, it works. But I am not sure about other APIs.

>>> from enum import IntEnum


class Color(IntEnum):
    RED = 10
>>> Color.RED
<Color.RED: 10>
>>> r = sublime.Region(Color.RED)
>>> r
Region(10, 10)

---

ref: https://docs.python.org/3.12/library/enum.html#enum.IntEnum

[jwortmann]

Maybe you can try [...]

The IntEnum classes are actually not problematic, because comparisons work out of the box for them. But a problem is that some other classes in protocol.py inherit from Enum with their values being strings. Unfortunately you can't compare via SomeEnum.SomeValue == 'something' if SomeEnum is just an Enum. But it works if it is a StrEnum.

And there are some problems in the initialize request, where we access the enum members directly in the code (and not via .value suffix). But I'll push a fix for that.

[jfcherng]

Maybe you can try [...]

The IntEnum classes are actually not problematic, because comparisons work out of the box for them. But a problem is that some other classes in protocol.py inherit from Enum with their values being strings. Unfortunately you can't compare via SomeEnum.SomeValue == 'something' if SomeEnum is just an Enum. But it works if it is a StrEnum.

#2446 (comment) works in my test.

>>> from enum import Enum


class StrEnum(str, Enum):
    def __str__(self) -> str:
        return self.value.__str__()


class Kind(StrEnum):
    KEYWORD = "1"
    CONSTANT = "2"


>>> Kind.KEYWORD == '1'
True
>>> Kind.KEYWORD < '2'
True

Would it be problematic if it inherits from this class StrEnum(str, Enum) rather than Enum? And replace

LSP/plugin/core/typing.py

Lines 81 to 82 in 5891ff4

	class StrEnum(Type): # type: ignore
	pass

with

class StrEnum(str, Enum):
    def __str__(self) -> str:
        return self.value.__str__()

[jwortmann]

Would it be problematic if it inherits from this class StrEnum(str, Enum) rather than Enum?

No, your suggestion should work too. But it's not much different in practice from what we currently have with

class StrEnum:
    pass

The problem was that the enums in protocol.py are automatically generated from https://github.com/microsoft/lsprotocol and they used Enum instead of StrEnum. The PR linked somewhere above should workaround that.

[jfcherng]

But then do we still have Enum, IntEnum or StrEnum issues? What are them?

[jwortmann]

Problem 1 was that for the initialize request we used for example

LSP/plugin/core/sessions.py

Line 299 in 5891ff4

"valueSet": [InsertTextMode.AdjustIndentation]

which would give us something like [<InsertTextMode.AdjustIndentation: 1>] (which is not JSON serializable) if we import IntEnum from the enum module instead of using a fake class.

Problem 2 was that comparisons of enums which have string values would not work anymore in the code, because those enums only used the Enum class and not StrEnum.

But both should be fixed now in 8c01147

[jwortmann]

@predragnikolic I wonder can you restore the feat/py38 branch temporarily? Otherwise it seems to be impossible to change the target branch of this PR

[predragnikolic]

Will do, I saw what happened.
I didn't expect that.

I thought that it would just switch to the main branch.

[jwortmann]

Ok I think now the feat/py38 branch can be deleted.

And I think we discovered a bug in GitHub's UI 😆

[predragnikolic]

Well, I died a bit inside when I saw this :D