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

Use Sequence instead of List #285

Merged
merged 4 commits into from
Nov 10, 2023
Merged

Conversation

muffinmad
Copy link
Contributor

pip install lsprotocol
pip install mypy

The main.py:

from typing import List
from lsprotocol import types


def get_edits() -> List[types.TextEdit]:
    return [
        types.TextEdit(
            types.Range(types.Position(0, 0), types.Position(0, 1)),
            'new text'
        )
    ]


edits = get_edits()

types.TextDocumentEdit(
    types.OptionalVersionedTextDocumentIdentifier('file', 1),
    edits,
)

The output of mypy main.py:

main.py:18: error: Argument 2 to "TextDocumentEdit" has incompatible type "list[TextEdit]"; expected "list[TextEdit | AnnotatedTextEdit]"  [arg-type]
main.py:18: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
main.py:18: note: Consider using "Sequence" instead, which is covariant
Found 1 error in 1 file (checked 1 source file)

Replacing List with Sequence solve the error reported by Mypy:

Success: no issues found in 1 source file

@karthiknadig
Copy link
Member

@muffinmad Can you also run the generator? you can run this from project root.

python -m generator --plugin python

@muffinmad
Copy link
Contributor Author

@karthiknadig Sure. But is it ok that running generator introduces so many changes to types.py? E.g.

-Definition = Union["Location", List["Location"]]
+Definition = Union['Location',Sequence['Location']]

Not only List replaced by Sequence but quotes style also changed as well as space after , disappeared.

@karthiknadig
Copy link
Member

Install nox into your environment, then run nox --session format. That should fix everything.

@muffinmad
Copy link
Contributor Author

@microsoft-github-policy-service agree

karthiknadig
karthiknadig previously approved these changes Nov 6, 2023
@karthiknadig karthiknadig self-assigned this Nov 6, 2023
@karthiknadig karthiknadig added the bug Issue identified by VS Code Team member as probable bug label Nov 6, 2023
@karthiknadig
Copy link
Member

You might need to update _hook.py where a lot of the things are registered as List.

Tyriar
Tyriar previously approved these changes Nov 6, 2023
@muffinmad muffinmad dismissed stale reviews from Tyriar and karthiknadig via 3280f6b November 7, 2023 08:17
@karthiknadig karthiknadig enabled auto-merge (squash) November 10, 2023 02:00
@karthiknadig karthiknadig merged commit d1d8d47 into microsoft:main Nov 10, 2023
34 checks passed
@muffinmad muffinmad deleted the patch-1 branch January 29, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants