flake8-bugbear icon indicating copy to clipboard operation
flake8-bugbear copied to clipboard

B006: ignore when type hints suggests immutability.

Open randolf-scholz opened this issue 2 years ago • 4 comments

When passing configurations to functions, it is quite annoying to have to do the whole Optional[Mapping[str, Any]] = None dance. It would be nice if it was possible to allow mutable defaults, when the type hints indicate immutability. (possibly via some --strict/--non-strict flag.)

from collections.abc import *

def foo(
    configA: Mapping[str, Any] = {},         # needn't raise B006
    configB: MutableMapping[str, Any] = {},  # should raise B006
    configC: dict[str, Any] = {},            # should raise B006
    seqA: Sequence[str] = [],                # needn't raise B006
    seqB: MutableSequence[str] = [],         # should raise B006
    seqC: list[str] = [],                    # should raise B006
    setA: Set[str] = set(),                  # needn't raise B006
    setB: MutableSet[str] = set(),           # should raise B006
    setB: set[str] = set(),                  # should raise B006
): ...

The reasoning is that the type checker should catch mutable behavior, and the benefit is obviously shorter function bodies that avoid the configA = {} if configA is None else configA boilerplate.

randolf-scholz avatar Mar 02 '23 15:03 randolf-scholz

Hm, it is more complicated: When the container itself holds mutable objects, things can still go bad:

from collections.abc import Mapping

def function(arg: Mapping[str, list[int]] = {"foo": []}) -> Mapping[str, list[int]]:
    arg["foo"].append(1)
    print(arg)
    return arg

function()
function()

I don't think flake8 has the capabilities to check for immutability in this case as one would need to evaluate the type hints recursively. I.e. the above example would be ok if list[int] were replaced with Sequence[int], then mypy would catch the bug.

randolf-scholz avatar Mar 02 '23 15:03 randolf-scholz

Can you re-open this? It's still a case from a performance point of view if you really know that your placeholder will be read only.

stalkerg avatar Oct 10 '23 04:10 stalkerg

@stalkerg I can reopen it, though personally I have found that like 95% of my needs are satisfied by either

  • Sequence[T] = ()
  • Mapping[K, V] = EMPTY_MAP

Where I use a module level constant EMPTY_MAP: Final[Mapping[K,V]] = types.MappingProxyType({}). Possibly in the future this can be replaced by frozenmap() if PEP 603 comes to fruition.

randolf-scholz avatar Oct 10 '23 10:10 randolf-scholz

@randolf-scholz thanks! It also seems like PEP 603 is promising.

stalkerg avatar Oct 11 '23 01:10 stalkerg