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

Improve error messages related to literal types #6149

Merged

Conversation

Michael0x2a
Copy link
Collaborator

This pull request improves how we handle error messages with Literal types in three different ways:

  1. When the user tries constructing types like Literal[3 + a], we now report an error message that says 'Invalid type: Literal[...] cannot contain arbitrary expressions'.

  2. We no longer recommend using Literal[...] when doing A = NewType('A', 4) or T = TypeVar('T', bound=4). This resolves Regression: error messages for literal expressions in invalid places are worse #5989.

    (The former suggestion is a bad one: you can't create a NewType of a Literal[...] type. The latter suggestion is a valid but stupid one: T = TypeVar('T', bound=Literal[4]) is basically the same thing as T = Literal[4].)

  3. When the user tries using complex numbers inside Literals (e.g. Literal[3j]), we now report an error message that says 'Parameter 1 of Literal[...] cannot be of type "complex"'. This is the same kind of error message we previously used to report when the user tried using floats inside of literals.

In order to accomplish bullet point 1, moved the "invalid type comment or annotation" checks from the parsing layer to the semantic analysis layer. This lets us customize which error message we report depending on whether or not the invalid type appears in the context of a Literal[...] type.

In order to accomplish this, I repurposed RawLiteralType so it can represent any arbitrary expression that does not convert directly into a type (and renamed 'RawLiteralType' to 'RawExpressionType' to better reflect this new usage). I also added an optional "note" field to that class: this lets the parsing layer attach some extra context that would be difficult to obtain up in the semantic analysis layer.

In order to accomplish bullet point 2, I modified the type analyzer so that the caller can optionally suppress the error messages that would otherwise be generated when a RawExpressionType appears outside of a Literal context.

Bullet point 3 only required a minor tweak to the parsing and error handling code.

This commit renames "RawLiteralType" to "RawExpressionType". This is to
prepare for an upcoming commit, where we repurpose RawExpressionType to
represent any expression that does not convert cleanly into a type.

This lets us defer almost all error handling to the type analysis phase,
which makes it easier for us to generate cleaner error messages when
using Literal types.
This commit moves the "invalid type comments or annotations" error
handling out of the parsing stage into the type analysis segment of
the semantic analysis stage.

It does so by modifying the parser so it no longer reports an error
and returns an AnyType when encountering an invalid type: instead,
it returns a RawExpressionType. We then handle this RawExpressionType
in the semantic analysis layer in one of two ways:

- If the RawExpressionType corresponds to a literal and appears in a
  Literal[...] context, we generate a LiteralType.

- If the RawExpressionType does *not* correspond to a literal, or
  appears outside of a Literal[...] context, we report an error and
  produce an AnyType.

This commit also adjusts the "invalid type comments or annotations"
error message so it starts with an uppercase letter. This is more
consistent with the style of error messages the rest of the semantic
analysis layer typically returns.
This commit tweaks mypy so that we report a customized error message
when the user attempts to use complex numbers inside Literal[...] types:
e.g. if the user tries doing "Literal[4j]".

We do not attempt to report a customized error message if the user
attempts constructing types like "Literal[4j + 5]". In those cases, we
continue to report the "Invalid type: Literal[...] cannot contain
arbitrary expressions" error message.
This commit modifies mypy so that the caller of TypeAnalyzer can
optionally suppress "Invalid type" related error messages.

This resolves python#5989: mypy will
now stop recommending using Literal[...] when doing
`A = NewType('A', 4)` or `T = TypeVar('T', bound=4)`.

(The former suggestion is a bad one: you can't create a NewType of a
Literal[...] type. The latter suggestion is a valid but stupid one:
`T = TypeVar('T', bound=Literal[4])` is basically the same thing as
doing just `T = Literal[4].
@Michael0x2a
Copy link
Collaborator Author

Two meta notes:

First, I broke up these changes into 4 separate commits to try and make it a little easier to track down the non-trivial changes during code review.

Second, I experimented with and ultimately decided to reject adding TypeOfAny.invalid_type: I found it didn't really help, nor did it seem like it would help us with our existing TODOs and issues.

Specifically, adding that constant wouldn't really help with #4030: that's really more of an issue of carefully adding more checks to the type checking or semantic analysis layer.

It also didn't seem like it would help with the TODO listed here: returning AnyType instead of UnboundType there ended up making the error messages worse. We would need to attach some additional context to AnyType to preserve the current level of info in the error messages, but we more or less rejected that approach in #6121.

We can maybe look into adding TypeOfAny.invalid_type in a future PR along with additional changes that actually do end up taking advantage of that new Any kind.

@Michael0x2a Michael0x2a mentioned this pull request Jan 6, 2019
42 tasks
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM! I just have to suggestions.

mypy/fastparse.py Outdated Show resolved Hide resolved
mypy/semanal_newtype.py Show resolved Hide resolved
@Michael0x2a Michael0x2a merged commit fd048ab into python:master Jan 7, 2019
@Michael0x2a Michael0x2a deleted the improve-literal-types-error-handling branch January 7, 2019 18:07
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.

Regression: error messages for literal expressions in invalid places are worse
2 participants