Skip to content

[Fix #11222] Preserve string arguments in annotations #11292

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

Merged
merged 2 commits into from
Nov 7, 2021

Conversation

0x000A
Copy link
Contributor

@0x000A 0x000A commented Oct 7, 2021

Description

Fixes #11222

This PR attempts to fix the issue in the annotations by preserving quotes on string arguments when they are converted to a single string

Test Plan

I added the following test:

[case testPreserveFunctionAnnotationWithArgs]
def f(x: foo['x']) -> bar: ...
def g(x: foo[x]) -> bar: ...
def h(x: foo['x', 'y']) -> bar: ...
def i(x: foo[x, y]) -> bar: ...
def j(x: foo['x', y]) -> bar: ...
def k(x: foo[x, 'y']) -> bar: ...
def lit_str(x: Literal['str']) -> Literal['str']: ...
def lit_int(x: Literal[1]) -> Literal[1]: ...
[out]
def f(x: foo['x']) -> bar: ...
def g(x: foo[x]) -> bar: ...
def h(x: foo['x', 'y']) -> bar: ...
def i(x: foo[x, y]) -> bar: ...
def j(x: foo['x', y]) -> bar: ...
def k(x: foo[x, 'y']) -> bar: ...
def lit_str(x: Literal['str']) -> Literal['str']: ...
def lit_int(x: Literal[1]) -> Literal[1]: ...

@JelleZijlstra
Copy link
Member

This doesn't seem right; we should be looking instead at whether it's Literal. I imagine this will turn Literal["str"] into Literal[str], for example, because str is a valid type.

@0x000A
Copy link
Contributor Author

0x000A commented Oct 9, 2021

I tested with Literal["str"] it gets converted correctly into Literal["str"] because it's represented internally as an UnboundType as well. is there something wrong here? what's LiteralType used for

@sobolevn
Copy link
Member

@0x000A let's test this case explicitly 🙂

For example:

def lit_str(x: Literal['str']) -> Literal['str']: ...
def lit_int(x: Literal[1]) -> Literal[1]: ...

@0x000A
Copy link
Contributor Author

0x000A commented Oct 12, 2021

@sobolevn both cases get converted correctly. should I add these tests in the test plan as well?

@JelleZijlstra
Copy link
Member

should I add these tests in the test plan as well?

Yes please

@hauntsaninja
Copy link
Collaborator

Thanks, this is great! :-)

tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
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.

stubgen remove quotes in an argument definition that has a literal type
4 participants