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

Fix LSP.plugin.core.typing with py38 #2456

Merged
merged 4 commits into from
Apr 21, 2024

Conversation

jfcherng
Copy link
Contributor

@jfcherng jfcherng commented Apr 20, 2024

Since we are now in py38, we don't need most of type polyfills.

# noqa: F401 is to suppress import but no used linter warnings.

@jfcherng jfcherng force-pushed the fix/py38-typing branch 2 times, most recently from 110bbf3 to 51c719c Compare April 20, 2024 19:19
@predragnikolic
Copy link
Member

Hello,

I tested this, and it looks like the NotRequired needs more work.

Some LSP-jdtls, LSP-julia, LSP-volar import NotRequired from LSP.plugin.core.typing.

This PR breaks them:
TypeError: <class 'LSP.plugin.core.typing.NotRequired'> is not a generic class

@jfcherng
Copy link
Contributor Author

jfcherng commented Apr 20, 2024

TypeError: <class 'LSP.plugin.core.typing.NotRequired'> is not a generic class

@predragnikolic Wondering whether 1856443 (#2456) works for that.

@jfcherng jfcherng force-pushed the fix/py38-typing branch 2 times, most recently from f6304e6 to 1856443 Compare April 20, 2024 19:49
@predragnikolic
Copy link
Member

It works.

Ideally LSP-* plugins can now import types from typings or typing_extensions instead of importing from LSP.plugins.core.typing. But LSP.plugins.core.typing can exist as long as there is something using it.

And instead of List, Dict, we can use list, dict, tuple,... from __future__ import annotations

@jfcherng
Copy link
Contributor Author

jfcherng commented Apr 20, 2024

Ideally LSP-* plugins can now import types from typings or typing_extensions instead of importing from LSP.plugins.core.typing. But LSP.plugins.core.typing can exist as long as there is something using it.

And instead of List, Dict, we can use list, dict, tuple,... from __future__ import annotations

Yeah. That's what I encountered. When using lsp_utils, get complaints like tuple is not compatible with Tuple, which is imported by lsp_utils from LSP.plugins.core.typing.

deathaxe added a commit to deathaxe/LSP that referenced this pull request Apr 21, 2024
Copy link
Member

@predragnikolic predragnikolic left a comment

Choose a reason for hiding this comment

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

Tested, works.

@predragnikolic predragnikolic merged commit 0ae1c7b into sublimelsp:main Apr 21, 2024
8 checks passed
predragnikolic added a commit that referenced this pull request Apr 21, 2024
* Import __future__ annotations

This commit...

1. adds `from __future__ import annotations` to each relevant module in order
   to enable language level type annotation support.
2. as a result most quotation marks can be removed from type annotations.

* Remove quotes from return types

* Avoid conflict with PR #2456

---------

Co-authored-by: Предраг Николић <[email protected]>
@predragnikolic
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants