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

internal error while formatting with-statement with tuple and none tuple argument #4642

Open
15r10nk opened this issue Apr 2, 2025 · 6 comments · Fixed by #4646
Open

internal error while formatting with-statement with tuple and none tuple argument #4642

15r10nk opened this issue Apr 2, 2025 · 6 comments · Fixed by #4646
Labels
T: bug Something isn't working

Comments

@15r10nk
Copy link

15r10nk commented Apr 2, 2025

Describe the bug

The following code can not be parsed/formatted by black:

with (name_5, name_4), name_5:
    pass

(playground)

black reported the following error:

> black -l 100 -C -t py313 bug.py
error: cannot format bug.py: INTERNAL ERROR: Black 25.1.1.dev21+g944a38e.d20250317 on Python (CPython) 3.13.1 produced code that is not equivalent to the source.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /tmp/blk_sxhcobwt.log

Oh no! 💥 💔 💥
1 file failed to reformat.

the reported diff in /tmp/blk_sxhcobwt.log is:

--- src
+++ dst
@@ -5,30 +5,29 @@
         Pass(
         )  # /Pass
         items=
         withitem(
             context_expr=
-            Tuple(
+            Name(
                 ctx=
                 Load(
                 )  # /Load
-                elts=
-                Name(
-                    ctx=
-                    Load(
-                    )  # /Load
-                    id=
-                    'name_5',  # str
-                )  # /Name
-                Name(
-                    ctx=
-                    Load(
-                    )  # /Load
-                    id=
-                    'name_4',  # str
-                )  # /Name
-            )  # /Tuple
+                id=
+                'name_5',  # str
+            )  # /Name
+            optional_vars=
+            None,  # NoneType
+        )  # /withitem
+        withitem(
+            context_expr=
+            Name(
+                ctx=
+                Load(
+                )  # /Load
+                id=
+                'name_4',  # str
+            )  # /Name
             optional_vars=
             None,  # NoneType
         )  # /withitem
         withitem(
             context_expr=

but it can be parsed by cpython:

from ast import parse
parse(
    'with (name_5, name_4), name_5:\n'
    '    pass\n'
)

The code can be formatted with black -l 100 -C -t py313 bug.py --fast:

with name_5, name_4, name_5:
    pass

Environment

  • Black's version: current main (950ec38)
  • OS and Python version: Linux/Python 3.13.1 (main, Jan 14 2025, 22:47:38) [Clang 19.1.6 ]

Additional context

The bug was found by pysource-codegen (see #3908)
The above problem description was created from a script,
let me know if you think it can be improved.

@15r10nk 15r10nk added the T: bug Something isn't working label Apr 2, 2025
@Pedro-Muller29
Copy link
Contributor

Pedro-Muller29 commented Apr 8, 2025

I believe this doesn't need a fix since this code will always fail at runtime, as tuples don't support the context manager protocol.

In fact, running a code such as:

with (open("a.txt", "r"), open("b.txt", "r")), open("c.txt", "r"):
    pass

Will throw the following error:

Traceback (most recent call last):
  File "/anon_path/with_tuple.py", line 1, in <module>
    with (open("a.txt", "r"), open("b.txt", "r")), open("c.txt", "r"):
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'tuple' object does not support the context manager protocol

@15r10nk
Copy link
Author

15r10nk commented Apr 8, 2025

To be honest, I don't like to create these kind of pedantic issues. But this are the things which are found by pysource-codegen.

I can understand when you do not want to fix it, but I have a example where this code can be useful.

def test_tuple_as_contextmanager():
    from contextlib import nullcontext

    try:
        with (nullcontext(),nullcontext()),nullcontext():
            pass
    except TypeError: 
        # test passed
        pass
    else:
        # this should be a type error
        assert False

Code like this could be used in tests of an python implementation. I have no real world example, but this is the only use case I can think of where this syntax is "useful".

@Pedro-Muller29
Copy link
Contributor

Fair point, I guess there is some niche where this would be useful. Working on a PR.

@JelleZijlstra
Copy link
Collaborator

I do think we should fix this sort of thing even if the code always fails—if nothing else so that we can continue fuzzing, because fuzzing is a good way to find bugs!

Thanks @15r10nk for doing this fuzzing and thanks @Pedro-Muller29 for the fix.

@15r10nk
Copy link
Author

15r10nk commented Apr 9, 2025

@Pedro-Muller29 I think your fix solves only 50% of the cases.

This is fixed:

with (a, b), c:
    pass

This not:

with c,(a, b):
    pass

I have looked at your merge request, but I can't see the problem.

@Pedro-Muller29
Copy link
Contributor

@Pedro-Muller29 I think your fix solves only 50% of the cases.

This is fixed:

with (a, b), c:
    pass

This not:

with c,(a, b):
    pass

I have looked at your merge request, but I can't see the problem.

Nice catch! Yeah, it's weird that my PR didn't fix that since it looks for commas both ways. I can try to fix it this weekend.

@JelleZijlstra JelleZijlstra reopened this Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants