black icon indicating copy to clipboard operation
black copied to clipboard

Black fails when comment exists inside of list comprehension

Open digitalresistor opened this issue 7 years ago • 8 comments

Howdy! Sorry you're having trouble. To expedite your experience, provide some basics for me:

Operating system: Python version: 3.7 Black version: 18.9b0 Does also happen on master: yes

class Something(object):
    def acceptable_offers(self, offers):
        """
        Return the offers that are acceptable according to the header.

        Any offers that cannot be parsed via
        :meth:`.Accept.parse_offer` will be ignored.

        :param offers: ``iterable`` of ``str`` media types (media types can
                       include media type parameters)
        :return: When the header is invalid, or there is no ``Accept`` header
                 in the request, all `offers` are considered acceptable, so
                 this method returns a list of (media type, qvalue) tuples
                 where each offer in `offers` is paired with the qvalue of 1.0,
                 in the same order as in `offers`.
        """
        return [
            (offers[offer_index], 1.0)
            for offer_index, _
            # avoid returning any offers that don't match the grammar so
            # that the return values here are consistent with what would be
            # returned in AcceptValidHeader
            in self._parse_and_normalize_offers(offers)
        ]

Is mis-formatted as:

class Something(object):
    def acceptable_offers(self, offers):
        """
        Return the offers that are acceptable according to the header.

        Any offers that cannot be parsed via
        :meth:`.Accept.parse_offer` will be ignored.

        :param offers: ``iterable`` of ``str`` media types (media types can
                       include media type parameters)
        :return: When the header is invalid, or there is no ``Accept`` header
                 in the request, all `offers` are considered acceptable, so
                 this method returns a list of (media type, qvalue) tuples
                 where each offer in `offers` is paired with the qvalue of 1.0,
                 in the same order as in `offers`.
        """
        return [
            (offers[offer_index], 1.0)
            for offer_index, _# avoid returning any offers that don't match the grammar so# that the return values here are consistent with what would be# returned in AcceptValidHeader in self._parse_and_normalize_offers(
                offers
            )
        ]

Which is not valid Python:

error: cannot format /Users/xistence/Projects/Pylons/webob/src/webob/acceptparse.py: INTERNAL ERROR: Black produced invalid code: invalid syntax (<unknown>, line 1346). Please report a bug on https://github.com/ambv/black/issues.  This invalid output might be helpful: /var/folders/jy/3csvdt_s3ys8sdn_123w89t00000gp/T/blk_ogrf5m4o.log

Moving the inline comment above the return statement works. This allows me to run black across the WebOb source code without issues.

digitalresistor avatar Oct 15 '18 01:10 digitalresistor

Thanks for your report. This will be fixed in the next release.

ambv avatar Oct 17 '18 10:10 ambv

I spent some time looking at this issue and I'm going to post what I found so far before I go crazy from too much staring at split_line(); I'll continue on this tomorrow. The example in this issue is fixed by this rather silly diff:

diff --git a/black.py b/black.py
index 9ecfbe1..980196e 100644
--- a/black.py
+++ b/black.py
@@ -1126,7 +1126,7 @@ class Line:
         Raises ValueError when any `leaf` is appended after a standalone comment
         or when a standalone comment is not the first leaf on the line.
         """
-        if self.bracket_tracker.depth == 0:
+        if True or self.bracket_tracker.depth == 0:
             if self.is_comment:
                 raise ValueError("cannot append to standalone comments")
 
@@ -2516,7 +2516,7 @@ def standalone_comment_split(
     line: Line, features: Collection[Feature] = ()
 ) -> Iterator[Line]:
     """Split standalone comments from the rest of the line."""
-    if not line.contains_standalone_comments(0):
+    if not line.contains_standalone_comments(1):
         raise CannotSplit("Line does not have any standalone comments")
 
     current_line = Line(depth=line.depth, inside_brackets=line.inside_brackets)

But this diff leads to other crashes in various places. The intuition is that if we're inside brackets, we have to somehow increase the depth we compare to, but I haven't yet found a way to do that correctly.

JelleZijlstra avatar May 06 '19 00:05 JelleZijlstra

I ran into this. The following is some simple code that reproduces it:

[
    x for x
    # Some comment.
    in y
]

lucaswiman avatar Jun 06 '19 23:06 lucaswiman

Also happens with

from foo import (
    bar,
    # qux
)

This version is supported:

from foo import (
    bar
    # qux
)

but is not really useful (since we only have one element) and is even unexpectedly transformed to:

from foo import (
    bar
    # qux
    ,
)

pquentin avatar Aug 28 '19 11:08 pquentin

This looks like the kind of thing that'll cause some grief. It's been over a year since this was filed so I wonder if as a temporary workaround we could make a regex people can at least use to find these issues? I'm scared to run Black now on my large codebase because while we have lots of tests, we don't have 100% test coverage, and this issue could definitely break things.

mlissner avatar Dec 14 '19 07:12 mlissner

As long as you run black with --safe, this shouldn't cause any breakages (apart from black itself crashing).

zsol avatar Dec 14 '19 18:12 zsol

Ah, indeed, and --safe is the default. So if you're running with defaults and if black isn't crashing, you're all good on this bug. Sorry for my alarm, that helps a lot.

mlissner avatar Dec 15 '19 06:12 mlissner

After looking at all the examples in this and linked tickets, I think an initial solution would be to at least make sure black doesn't crash and the syntax isn't changed. Later, we can agree on where the comment should be moved, if anywhere.

clavedeluna avatar Oct 23 '22 11:10 clavedeluna

Confirming that https://github.com/psf/black/issues/563#issuecomment-499705073 still occurs in black, 23.3.0 (compiled: yes).

I also found the strange (less so with arguments before the :)

(
    lambda
    #
    : None
)

which cause Black to crash just like the list comprehension.

Zac-HD avatar May 24 '23 19:05 Zac-HD