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

Make literal exprs have inferred type of 'Literal' based on context #5990

Merged
merged 5 commits into from
Dec 5, 2018

Conversation

Michael0x2a
Copy link
Collaborator

@Michael0x2a Michael0x2a commented Dec 3, 2018

This pull request modifies the type checking logic so that literal expressions will have an inferred type of 'Literal' if the context asks for a literal type. That is, it implements support for this:

x: Literal[1] = 1
y = 1

reveal_type(x)  # E: Revealed type is 'Literal[1]'
reveal_type(y)  # E: Revealed type is 'builtins.int'

This pull requests also implements the visit_literal_type method in the constraints.ConstraintBuilderVisitor and join.TypeJoinVisitor methods. Both visitors are exercised indirectly through the "let's use literal types in collection contexts" code, but only the latter is tested directly: I wasn't really sure how to directly test ConstraintBuilderVisitor.

The implementation is simple though -- I'm pretty sure literal types count as a "leaf type" so it's fine to return an empty list (no constraints).

This pull request modifies the type checking logic so that literal
expressions will have an inferred type of 'Literal' if the context asks
for a literal type.

This pull requests also implements the `visit_literal_type` method
in the  `constraints.ConstraintBuilderVisitor` and `join.TypeJoinVisitor`
methods. Both visitors are exercised indirectly through the "let's use
literal types in collection contexts" code, but only the latter is
tested directly: I wasn't really sure how to directly test
`ConstraintBuilderVisitor`.

The implementation is simple though -- I'm pretty sure literal types
count as a "leaf type" so it's fine to return an empty list
(no constraints).
@Michael0x2a Michael0x2a mentioned this pull request Dec 3, 2018
42 tasks
@Michael0x2a
Copy link
Collaborator Author

Note to self: add tests here making sure inference works as expected with overloads.

I guess maybe with generics as well, while we're at it? Or maybe in a follow-up diff specifically about generics.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Great, this will make literal types useful for some real use cases! Left mostly ideas about additional test cases.

mypy/checkexpr.py Outdated Show resolved Hide resolved
self.assert_simple_join(lit1, lit2, a)
self.assert_simple_join(lit1, lit3, self.fx.o)
self.assert_simple_join(lit1, self.fx.anyt, self.fx.anyt)
self.assert_simple_join(self.fx.anyt, lit1, self.fx.anyt)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test also joins with unions containing literals types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I also didn't see until now that assert_join internally already tests symmetry and calls assert_simple_join, so I went ahead and removed some existing duplicate tests.

test-data/unit/check-literal.test Show resolved Hide resolved
test-data/unit/check-literal.test Show resolved Hide resolved
reveal_type(f2) # E: Revealed type is 'def (x: Literal[' foo bar ']) -> Literal[' foo bar ']'
reveal_type(f3) # E: Revealed type is 'def (x: Literal[' foo bar ']) -> Literal[' foo bar ']'
reveal_type(f4) # E: Revealed type is 'def (x: Literal['foo']) -> Literal['foo']'
reveal_type(f5) # E: Revealed type is 'def (x: Literal['foo']) -> Literal['foo']'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test calls to a function accepting a union of string literal types. Call it with each supported literal and also union argument types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added to a test named 'testLiteralCallingUnionFunction'.

y: Literal['a', 'b']
f(x, y) # E: Argument 1 to "f" has incompatible type "Union[Literal['a'], Literal['b']]"; expected "Literal['a']" \
# E: Argument 2 to "f" has incompatible type "Union[Literal['a'], Literal['b']]"; expected "Literal['a']" \
[out]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test overload where there are no literal types and one of the overload items is T -> T. This shouldn't produce literal types when called with a literal argument.

Test overload where one item accepts a literal 'x' and another takes T -> T. When called with an unrelated literal 'y', will this produce a literal type or a non-literal type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I attempted adding a test case named "testLiteralInferredInOverloadContextWithTypevars" that exercises these. Let me know if those tests match what you had in mind.

test-data/unit/check-literal.test Show resolved Hide resolved
mypy/test/testtypes.py Outdated Show resolved Hide resolved
@Michael0x2a
Copy link
Collaborator Author

@JukkaL -- this should be ready for a second look whenever!

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Looks good now!

@Michael0x2a Michael0x2a merged commit ad2d4ba into python:master Dec 5, 2018
@Michael0x2a Michael0x2a deleted the add-literal-expr-inference branch December 5, 2018 17:17
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