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

Black produced different code on the 2nd pass of the formatter #1297

Closed
AnjoMan opened this issue Mar 5, 2020 · 2 comments
Closed

Black produced different code on the 2nd pass of the formatter #1297

AnjoMan opened this issue Mar 5, 2020 · 2 comments
Labels
C: unstable formatting Formatting changed on the second pass T: bug Something isn't working

Comments

@AnjoMan
Copy link

AnjoMan commented Mar 5, 2020

Getting the following error running black:

error: cannot format /home/alodder/Development/mkautodoc/mkautodoc/typing.py: INTERNAL ERROR: Black produced different code on the second pass of the formatter.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /tmp/blk_mh_2_e6u.log
Oh no! 💥 💔 💥
2 files left unchanged, 1 file failed to reformat.

To Reproduce Steps to reproduce the behavior:

  1. Take this file
  2. Run Black on it with no additional arguments
  3. See error

Expected behavior A clear and concise description of what you expected to happen.

I expect black to reformat the file (or leave it unchanged, if it does not find anything incorrect.

Environment (please complete the following information):

Does this bug also happen on master?

using 'black.now' i find that it does not occur on master, and if i replace the contents of the file with the output of black.now, the error no longer occurs

Additional context Add any other context about the problem here.

black logs the following to a temporary file /tmp/blk_mh_2_e6u.log:

--- source
+++ first pass
@@ -44,148 +44,164 @@
     elif isinstance(annotation, TypeVar):  # type: ignore
         return annotation.__name__
     elif not annotation:
         return repr(annotation)
     elif annotation is NoneType:  # type: ignore
-        return 'None'
-    elif (getattr(annotation, '__module__', None) == 'builtins' and
-          hasattr(annotation, '__qualname__')):
+        return "None"
+    elif getattr(annotation, "__module__", None) == "builtins" and hasattr(
+        annotation, "__qualname__"
+    ):
         return annotation.__qualname__
     elif annotation is Ellipsis:
-        return '...'
+        return "..."
 
     if sys.version_info >= (3, 7):  # py37+
         return _stringify_py37(annotation)
     else:
         return _stringify_py36(annotation)
 
 
 def _stringify_py37(annotation: Any) -> str:
     """stringify() for py37+."""
-    module = getattr(annotation, '__module__', None)
-    if module == 'typing':
-        if getattr(annotation, '_name', None):
+    module = getattr(annotation, "__module__", None)
+    if module == "typing":
+        if getattr(annotation, "_name", None):
             qualname = annotation._name
-        elif getattr(annotation, '__qualname__', None):
+        elif getattr(annotation, "__qualname__", None):
             qualname = annotation.__qualname__
-        elif getattr(annotation, '__forward_arg__', None):
+        elif getattr(annotation, "__forward_arg__", None):
             qualname = annotation.__forward_arg__
         else:
             qualname = stringify(annotation.__origin__)  # ex. Union
-    elif hasattr(annotation, '__qualname__'):
-        qualname = '%s.%s' % (module, annotation.__qualname__)
+    elif hasattr(annotation, "__qualname__"):
+        qualname = "%s.%s" % (module, annotation.__qualname__)
     else:
         qualname = repr(annotation)
 
-    if getattr(annotation, '__args__', None):
-        if qualname == 'Union':
+    if getattr(annotation, "__args__", None):
+        if qualname == "Union":
             if len(annotation.__args__) == 2 and annotation.__args__[1] is NoneType:  # type: ignore  # NOQA
-                return 'Optional[%s]' % stringify(annotation.__args__[0])
+                return "Optional[%s]" % stringify(annotation.__args__[0])
             else:
-                args = ', '.join(stringify(a) for a in annotation.__args__)
-                return '%s[%s]' % (qualname, args)
-        elif qualname == 'Callable':
-            args = ', '.join(stringify(a) for a in annotation.__args__[:-1])
+                args = ", ".join(stringify(a) for a in annotation.__args__)
+                return "%s[%s]" % (qualname, args)
+        elif qualname == "Callable":
+            args = ", ".join(stringify(a) for a in annotation.__args__[:-1])
             returns = stringify(annotation.__args__[-1])
-            return '%s[[%s], %s]' % (qualname, args, returns)
-        elif str(annotation).startswith('typing.Annotated'):  # for py39+
+            return "%s[[%s], %s]" % (qualname, args, returns)
+        elif str(annotation).startswith("typing.Annotated"):  # for py39+
             return stringify(annotation.__args__[0])
         elif annotation._special:
             return qualname
         else:
-            args = ', '.join(stringify(a) for a in annotation.__args__)
-            return '%s[%s]' % (qualname, args)
+            args = ", ".join(stringify(a) for a in annotation.__args__)
+            return "%s[%s]" % (qualname, args)
 
     return qualname
 
 
 def _stringify_py36(annotation: Any) -> str:
     """stringify() for py35 and py36."""
-    module = getattr(annotation, '__module__', None)
-    if module == 'typing':
-        if getattr(annotation, '_name', None):
+    module = getattr(annotation, "__module__", None)
+    if module == "typing":
+        if getattr(annotation, "_name", None):
             qualname = annotation._name
-        elif getattr(annotation, '__qualname__', None):
+        elif getattr(annotation, "__qualname__", None):
             qualname = annotation.__qualname__
-        elif getattr(annotation, '__forward_arg__', None):
+        elif getattr(annotation, "__forward_arg__", None):
             qualname = annotation.__forward_arg__
-        elif getattr(annotation, '__origin__', None):
+        elif getattr(annotation, "__origin__", None):
             qualname = stringify(annotation.__origin__)  # ex. Union
         else:
-            qualname = repr(annotation).replace('typing.', '')
-    elif hasattr(annotation, '__qualname__'):
-        qualname = '%s.%s' % (module, annotation.__qualname__)
+            qualname = repr(annotation).replace("typing.", "")
+    elif hasattr(annotation, "__qualname__"):
+        qualname = "%s.%s" % (module, annotation.__qualname__)
     else:
         qualname = repr(annotation)
 
-    if (isinstance(annotation, typing.TupleMeta) and  # type: ignore
-            not hasattr(annotation, '__tuple_params__')):  # for Python 3.6
+    if isinstance(
+        annotation, typing.TupleMeta
+    ) and not hasattr(  # type: ignore
+        annotation, "__tuple_params__"
+    ):  # for Python 3.6
         params = annotation.__args__
         if params:
-            param_str = ', '.join(stringify(p) for p in params)
-            return '%s[%s]' % (qualname, param_str)
+            param_str = ", ".join(stringify(p) for p in params)
+            return "%s[%s]" % (qualname, param_str)
         else:
             return qualname
     elif isinstance(annotation, typing.GenericMeta):
         params = None
-        if hasattr(annotation, '__args__'):
+        if hasattr(annotation, "__args__"):
             # for Python 3.5.2+
             if annotation.__args__ is None or len(annotation.__args__) <= 2:  # type: ignore  # NOQA
                 params = annotation.__args__  # type: ignore
             else:  # typing.Callable
-                args = ', '.join(stringify(arg) for arg
-                                 in annotation.__args__[:-1])  # type: ignore
+                args = ", ".join(
+                    stringify(arg) for arg in annotation.__args__[:-1]
+                )  # type: ignore
                 result = stringify(annotation.__args__[-1])  # type: ignore
-                return '%s[[%s], %s]' % (qualname, args, result)
-        elif hasattr(annotation, '__parameters__'):
+                return "%s[[%s], %s]" % (qualname, args, result)
+        elif hasattr(annotation, "__parameters__"):
             # for Python 3.5.0 and 3.5.1
             params = annotation.__parameters__  # type: ignore
         if params is not None:
-            param_str = ', '.join(stringify(p) for p in params)
-            return '%s[%s]' % (qualname, param_str)
-    elif (hasattr(typing, 'UnionMeta') and
-          isinstance(annotation, typing.UnionMeta) and  # type: ignore
-          hasattr(annotation, '__union_params__')):  # for Python 3.5
+            param_str = ", ".join(stringify(p) for p in params)
+            return "%s[%s]" % (qualname, param_str)
+    elif (
+        hasattr(typing, "UnionMeta")
+        and isinstance(annotation, typing.UnionMeta)
+        and hasattr(  # type: ignore
+            annotation, "__union_params__"
+        )
+    ):  # for Python 3.5
         params = annotation.__union_params__
         if params is not None:
             if len(params) == 2 and params[1] is NoneType:  # type: ignore
-                return 'Optional[%s]' % stringify(params[0])
+                return "Optional[%s]" % stringify(params[0])
             else:
-                param_str = ', '.join(stringify(p) for p in params)
-                return '%s[%s]' % (qualname, param_str)
-    elif (hasattr(annotation, '__origin__') and
-          annotation.__origin__ is typing.Union):  # for Python 3.5.2+
+                param_str = ", ".join(stringify(p) for p in params)
+                return "%s[%s]" % (qualname, param_str)
+    elif (
+        hasattr(annotation, "__origin__") and annotation.__origin__ is typing.Union
+    ):  # for Python 3.5.2+
         params = annotation.__args__
         if params is not None:
             if len(params) == 2 and params[1] is NoneType:  # type: ignore
-                return 'Optional[%s]' % stringify(params[0])
+                return "Optional[%s]" % stringify(params[0])
             else:
-                param_str = ', '.join(stringify(p) for p in params)
-                return 'Union[%s]' % param_str
-    elif (isinstance(annotation, typing.CallableMeta) and  # type: ignore
-          getattr(annotation, '__args__', None) is not None and
-          hasattr(annotation, '__result__')):  # for Python 3.5
+                param_str = ", ".join(stringify(p) for p in params)
+                return "Union[%s]" % param_str
+    elif (
+        isinstance(annotation, typing.CallableMeta)
+        and getattr(  # type: ignore
+            annotation, "__args__", None
+        )
+        is not None
+        and hasattr(annotation, "__result__")
+    ):  # for Python 3.5
         # Skipped in the case of plain typing.Callable
         args = annotation.__args__
         if args is None:
             return qualname
         elif args is Ellipsis:
-            args_str = '...'
+            args_str = "..."
         else:
             formatted_args = (stringify(a) for a in args)
-            args_str = '[%s]' % ', '.join(formatted_args)
-        return '%s[%s, %s]' % (qualname,
-                               args_str,
-                               stringify(annotation.__result__))
-    elif (isinstance(annotation, typing.TupleMeta) and  # type: ignore
-          hasattr(annotation, '__tuple_params__') and
-          hasattr(annotation, '__tuple_use_ellipsis__')):  # for Python 3.5
+            args_str = "[%s]" % ", ".join(formatted_args)
+        return "%s[%s, %s]" % (qualname, args_str, stringify(annotation.__result__))
+    elif (
+        isinstance(annotation, typing.TupleMeta)
+        and hasattr(  # type: ignore
+            annotation, "__tuple_params__"
+        )
+        and hasattr(annotation, "__tuple_use_ellipsis__")
+    ):  # for Python 3.5
         params = annotation.__tuple_params__
         if params is not None:
             param_strings = [stringify(p) for p in params]
             if annotation.__tuple_use_ellipsis__:
-                param_strings.append('...')
-            return '%s[%s]' % (qualname,
-                               ', '.join(param_strings))
+                param_strings.append("...")
+            return "%s[%s]" % (qualname, ", ".join(param_strings))
 
     return qualname
 
--- first pass
+++ second pass
@@ -116,13 +116,11 @@
     elif hasattr(annotation, "__qualname__"):
         qualname = "%s.%s" % (module, annotation.__qualname__)
     else:
         qualname = repr(annotation)
 
-    if isinstance(
-        annotation, typing.TupleMeta
-    ) and not hasattr(  # type: ignore
+    if isinstance(annotation, typing.TupleMeta) and not hasattr(  # type: ignore
         annotation, "__tuple_params__"
     ):  # for Python 3.6
         params = annotation.__args__
         if params:
             param_str = ", ".join(stringify(p) for p in params)
@AnjoMan AnjoMan added the T: bug Something isn't working label Mar 5, 2020
@Tecuya
Copy link

Tecuya commented May 6, 2020

"Me too"

I have a more compact file that can be formatted to expose this bug (below). OP's example and mine have a common thread: # type: ignore comments. If you replace my # type: ignore with # foo the error goes away; clearly black is handling # type: ignore differently than other comments (to preserve what line they are on) and thats causing the bug.

class Test:
    def a(self, arg):
        return self

    def b(self):
        return self

x = Test()

(
    x.a(1)  # type: ignore
    .b()
)

@ichard26 ichard26 added the C: unstable formatting Formatting changed on the second pass label Sep 27, 2020
@ichard26
Copy link
Collaborator

Closing as the latest version of Black (20.8b1) no longer produces different code on the 2nd pass for both examples.

AnjoMan's example:

(black) ichard26@acer-ubuntu:~/programming/oss/black$ cat temp.py
"""
    sphinx.util.typing
    ~~~~~~~~~~~~~~~~~~

    The composit types for Sphinx.

    :copyright: Copyright 2007-2020 by the Sphinx team, see AUTHORS.
    :license: BSD, see LICENSE for details.
"""

import sys
import typing
from typing import Any, Callable, Dict, Generator, List, Tuple, TypeVar, Union

from docutils import nodes
from docutils.parsers.rst.states import Inliner

[the rest snipped for brevity]
(black) ichard26@acer-ubuntu:~/programming/oss/black$ black temp.py --check
would reformat temp.py
Oh no! 💥 💔 💥
1 file would be reformatted.

and Tecuya's example:

(black) ichard26@acer-ubuntu:~/programming/oss/black$ cat temp.py
class Test:
    def a(self, arg):
        return self

    def b(self):
        return self

x = Test()

(
    x.a(1)  # type: ignore
    .b()
)
(black) ichard26@acer-ubuntu:~/programming/oss/black$ black temp.py --check
would reformat temp.py
Oh no! 💥 💔 💥
1 file would be reformatted.
My local environment
  • Black version: 20.8b1
  • Python version: CPython 3.8.5
  • OS version: Ubuntu 20.04.1 LTS

Bisecting tells us that commit 4b449e7 (from PR #1113) fixed both of these examples.

Thank you @AnjoMan and @Tecuya for reporting and providing examples!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: unstable formatting Formatting changed on the second pass T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants