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

bpo-36878: Allow extra text after # type: ignore comments #13238

Merged
merged 3 commits into from
May 11, 2019

Conversation

msullivan
Copy link
Contributor

@msullivan msullivan commented May 10, 2019

In the parser, when using the type_comments=True option, recognize
a TYPE_IGNORE as anything containing # type: ignore followed by
a non-alphanumeric character. This is to allow ignores such as
# type: ignore[E1000].

This doesn't provide the text in the syntax tree yet, which might be worth also doing.
But this is a sufficient start, since tools can pull it out themselves based on line numbers.

This may also want a PEP 484 update, but I think that might want to wait on some consensus/discussion about specific details.

https://bugs.python.org/issue36878

In the parser, when using the type_comments=True option, recognize
a TYPE_IGNORE as anything containing `# type: ignore` followed by
a non-alphanumeric character. This is to allow ignores such as
`# type: ignore[E1000]`.
@msullivan
Copy link
Contributor Author

msullivan commented May 10, 2019

I didn't make a news entry because it is a small update to something that will already be new in 3.8, but I can add one if that's the right thing to do.

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.

The idea (allow anything non-alphanumeric as first char after ignore) looks good to me. I have one random tests suggestion.


def baz():
pass # type: ignore[excuse]
x = 1 # type: ignore whatever
Copy link
Member

Choose a reason for hiding this comment

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

Not that this is super-important, but I would add two other typical scenarios:

# type: ignore=excuse  <- Pylint style
# type: ignore [excuse]  <- just in case

@msullivan
Copy link
Contributor Author

It occurs to me there is one awkward thing about how the current isalnum check operates, which is that things like type: ignoreé are accepted.

@ilevkivskyi
Copy link
Member

@msullivan Well, at least # type: ignored will be caught. I suppose there may be better functions, but I am not a big unicode expert, so I am happy with this simple solution. Plus, type: ignoreé can definitely be caught by a type checker.

@ilevkivskyi
Copy link
Member

@gvanrossum If you don't have any other suggestion, I am going to merge this soon.

@ilevkivskyi ilevkivskyi merged commit d8320ec into python:master May 11, 2019
@msullivan msullivan deleted the type-ignore branch May 13, 2019 17:23
ilevkivskyi pushed a commit that referenced this pull request May 22, 2019
)

GH-13238 made extra text after a # type: ignore accepted by the parser.
This finishes the job and actually plumbs the extra text through the
parser and makes it available in the AST.
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.

4 participants