black icon indicating copy to clipboard operation
black copied to clipboard

Enforce parentheses around star unpacking containers modified with operators

Open jakkdl opened this issue 2 years ago • 1 comments

Describe the style change Star unpacking containers in combination with modifying the container with an operator has a somewhat unintuitive operation ordering, and I think should probably have enforced parentheses.

Examples in the current Black style

def foo(other_var: Any):
    my_set: set[str] = {"a", "b", "c"}
    my_list: list[int] = [0, 1]

    print("hello", *my_set | {d}, "world")
    bar(*my_list + [2])

    # TypeError if other_var is not a set, but could at a glance be interpreted as
    # "return other_var if my_set is empty"
    return *my_set | other_var

Desired style

def foo(other_var: Any):
    my_set: set[str] = {"a", "b", "c"}
    my_list: list[int] = [0, 1]

    print("hello", *(my_set | {d}), "world")
    bar(*(my_list + [2]))

    # very obvious that other_var is added to my_set
    return *(my_set | other_var)

Additional context Black will currently not remove the parentheses either, so if you manually add them they'll stay. I recently wrote code with foo(*container|{"value"}), and was personally not aware of how python would prioritize the operators and was a bit surprised that Black didn't have any opinions.

I'm not completely sure how common unintuitive code with this is, or if it just leads to a ton of extra parentheses that are hard to read and users can manually add parens when it's needed, but I thought it was worth bringing up either way.

jakkdl avatar Jan 12 '23 11:01 jakkdl

Seems reasonable to make this change, I think the parentheses add clarity here.

JelleZijlstra avatar Jan 12 '23 16:01 JelleZijlstra