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

Consistent handling of too long lines due to directives (no special treatment of # type: ignore) #3569

Open
ruestefa opened this issue Feb 14, 2023 · 2 comments
Labels
T: enhancement New feature or request

Comments

@ruestefa
Copy link

(I initially wrote the issue as a bug report, hence the issue template, but it's probably closer to a feature request.)

Describe the bug

Lines that are too long because of trailing comments are handled inconsistently depending on whether they start with a mypy type: ignore directive.

Specifically, lines where the comment begins with # type: ignore are left as they are, whereas other such lines are broken up over multiple lines, which includes lines that contain # type: ignore but not in the beginning, and others that only contain directives from other tools.

Input:

from my_module import some_function  # type: ignore [import]  # pylint: ignore=no-name-in-module
from my_module import some_function  # pylint: ignore=no-name-in-module  # type: ignore [import]
from my_module import some_function  # type: ignore [import]  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule
from my_module import some_function  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule  # type: ignore [import]
from my_module import some_function  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule

Output:

from my_module import some_function  # type: ignore [import]  # pylint: ignore=no-name-in-module
from my_module import (
    some_function,
)  # pylint: ignore=no-name-in-module  # type: ignore [import]
from my_module import some_function  # type: ignore [import]  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule
from my_module import (
    some_function,
)  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule  # type: ignore [import]
from my_module import (
    some_function,
)  # pylint: ignore=no-name-in-module  # pylint: ignore=a_rule

I'm using import statements in the example because that's how I became aware of this behavior (and because using # pylint: disable-next:... instead is complicated due to how isort handles comments, not that that's black's problem), but I assume it is universal.

Expected behavior

Ideally, I'd like the # pylint: disable directives to be treated like the # type: ignore directives, such that all lines remain unchanged. Also, I'd like the # type: ignore directive also to be detected at the end of the line. However, I'm aware that this would require parsing the comments for directives beyond a simple .startswith("# type: ignore"), which you might not want to get into.

However, singling out mypy directives (in response to this) while ignoring directives of other commonly used third-party tools (none of which, for the record, I'm involved with) like pylint seems inconsistent to me. In my opinion, either all directives should be ignored (and treated like any other comments), or there should at least be rudimentary support equivalent to ignoring comments starting with # type: ignore for commonly used directives such as # pylint: disable.

Proposition

My suggestion would be to specify directives to be ignored in a list and not change lines that are too long because of a trailing comment if the latter contains (or starts with) any of those directives. To illustrate the logic (without knowing about the inner working of black):

ignored_directives = ["noqa", "type: ignore", "pylint: disable"]  # ...
for directive in directives:
    if f"# {directive}" in comment:  # or `if comment.startswith(f"# {directive}"):`
        break
else:
    # handle too long line

By making ignored_directives configurable, no tool-specific directives would have to be hardcoded into black, albeit at a cost of an additional configuration parameter, which I'm aware is probably a deal breaker.

Environment

  • Black v23.1.0 in online tool
  • Black v22.10.0 on Linux with Python v3.10.8
@ruestefa ruestefa added the T: enhancement New feature or request label Feb 14, 2023
@Jackenmen
Copy link
Contributor

In some cases, linting tools or other comments may be needed on the same line as a type comment. In these cases, the type comment should be before other comments and linting markers:

https://peps.python.org/pep-0484/#type-comments

@bersbersbers
Copy link

Adding another pretty inconsistent example here.

Lines 3/4: longer comments gets broken, shorter one does not.
Lines 5/6: shorter comments gets broken, longer one does not.

if True:
    if True:
        xxxx /= yyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator] # /
        xxxx /= yyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator]
        xxxxx /= yyyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator] # /
        xxxxx /= yyyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator]

becomes

if True:
    if True:
        xxxx /= (
            yyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator] # /
        )
        xxxx /= yyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator]
        xxxxx /= yyyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator] # /
        xxxxx /= (
            yyyyy  # pyright: ignore[reportGeneralTypeIssues] # type:ignore[operator]
        )

Playground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants