-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Standardize type comments #2698
Conversation
This was discussed on Python Discord but the summary of it is that we can't really normalize type comments until all of the major AST libraries that provide first-class type comment support are liberal in the whitespace they understand. It wouldn't be good to have a type comment that's ignored by typed-ast transformed into one that is noticed which then affects a type checker like mypy. We might be able to normalize type comments on newer Python versions since typed-ast is mostly used to provide Python 2 support but this will need further investigatio. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember the discussion @ichard26 alludes to above. Could you provide some details on how existing tools (typed-ast, pyright, pyre, ...) interpret spaces in type comments?
diff-shades results comparing this PR (505634a) to main (b517dfb). The full diff is available in the logs under the "Generate HTML diff report" step.
|
Before I forget to link it again: https://discord.com/channels/267624335836053506/846434317021741086/921790074922864670 |
@@ -271,7 +276,7 @@ def contains_pragma_comment(comment_list: List[Leaf]) -> bool: | |||
pylint). | |||
""" | |||
for comment in comment_list: | |||
if comment.value.startswith(("# type:", "# noqa", "# pylint:")): | |||
if re.match(r"#\s*(type\s*:|pylint\s*:|noqa\s*)", comment.value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if re.match(r"#\s*(type\s*:|pylint\s*:|noqa\s*)", comment.value): | |
if re.match(r"#\s*(type\s*:|pylint\s*:|noqa\s*|nosec\s*)", comment.value): |
Would be nice if Bandit is supported as well.
@13Ducks giving up on this MR ? |
Description
Fixes #2097 and standardizes type comments to always have 1 space after the # and 1 space after the :
As per the issue, the functions
is_type_comment()
andcontains_pragma_comment()
have been modified to return true even with any extra whitespace. Type comments without non-breaking spaces are now stripped of any extra whitespace as well.Waiting on confirmation from the issue that spaces before the : are acceptable (
# type : something
).Checklist - did you ...
^ Is documentation needed for this change? If so, I can add it.