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

is_type_comment() is wrong #2097

Open
JelleZijlstra opened this issue Apr 11, 2021 · 6 comments
Open

is_type_comment() is wrong #2097

JelleZijlstra opened this issue Apr 11, 2021 · 6 comments
Labels
F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. good first issue Good for newcomers T: bug Something isn't working

Comments

@JelleZijlstra
Copy link
Collaborator

Currently, the implementation of is_type_comment() looks like this:

def is_type_comment(leaf: Leaf, suffix: str = "") -> bool:
    """Return True if the given leaf is a special comment.
    Only returns true for type comments for now."""
    t = leaf.type
    v = leaf.value
    return t in {token.COMMENT, STANDALONE_COMMENT} and v.startswith("# type:" + suffix)

But this is wrong, because typed-ast also recognizes a comment with multiple spaces after the # as a type comment. Concretely this could lead Black to skip some of the checks we have to avoid moving type comments to the wrong line.

Ideally, Black should also standardize the formatting of type comments by cleaning up excessive whitespace after the # and after the :.

@Pierre-Sassoulas
Copy link
Contributor

I think there is also the pragmas in contains_pragma_comment at: comment.value.startswith(("# type:", "# noqa", "# pylint:")

@JelleZijlstra JelleZijlstra added the F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. label May 30, 2021
@ichard26 ichard26 added the good first issue Good for newcomers label Dec 14, 2021
@ichard26 ichard26 added this to the Release 22.0 milestone Dec 14, 2021
@13Ducks 13Ducks mentioned this issue Dec 15, 2021
3 tasks
@felix-hilden
Copy link
Collaborator

Should we also standardise spaces before the colon? Meaning we would accept (and fix) e.g. # type : Type or even # \t type : (some unexpected whitespace type) Type?

@ichard26 ichard26 removed this from the Release 22.0 milestone Jan 1, 2022
@itxasos23
Copy link

Hey everyone! Can I get some maintainer reivews on the PR?

Aashka1 added a commit to Aashka1/black that referenced this issue Nov 1, 2023
@IshanKamboj
Copy link

@Pierre-Sassoulas Hello... I am new here and I would like to start contributing in the repository. So can you please assign this issue to me
Thank you

@Pierre-Sassoulas
Copy link
Contributor

Hey @IshanKamboj, sorry I'm not a black maintainer. I think you can start implementing right away and open a PR though :)

@debapriyakumar
Copy link

hello i am new here and would like to contribute to the project can you help me out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. good first issue Good for newcomers T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants