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

B006: Case where mutable default arguments are not detected

Open mflova opened this issue 2 years ago • 5 comments

I realized that when using Python built-in dataclasses, the following scenario is not detected:

from dataclasses import dataclass

@dataclass
class MyClass:
    val = []  # Mutable data structure

Given the presence of the dataclass decorator, the signature of the __init__ method for MyClass is the following one:

def __init__(self, val = []):

I have seen that the rule B006 is supposed to detect issues where mutable data structures are used in a function signature. This is a slightly different scenario but with the same output if this effect is not intended. I could not find any related issue. Was this issue reported before?

mflova avatar Apr 10 '22 15:04 mflova

Interesting. Wonder dataclasses changes the AST parsing? I guess we just just need to add a unit tests to b006.py + make it work. Will take a PR fixing this, if it's indeed a problem.

cooperlees avatar Apr 10 '22 21:04 cooperlees

Wonder dataclasses changes the AST parsing?

It doesn't appear to, but it's definitely distinct from parsing function defs:

ClassDef(
    name='MyClass',
    bases=[],
    keywords=[],
    body=[
        Assign(
            targets=[Name(id='val', ctx=Store())],
            value=List(elts=[], ctx=Load()),
            type_comment=None,
        ),
    ],
    decorator_list=[Name(id='dataclass', ctx=Load())],
)

As expected, this is the same with other class definitions in general, e.g.:

from typing import NamedTuple

class Foo(NamedTuple):
    val = []

class Foo():
    val = []

I haven't toyed with it yet but I think this should be pretty doable with most of the existing logic. Is this still something to slot under B006/B008 with a tweak to the message verbiage or would it be worth its own error code?

sco1 avatar Apr 20 '22 23:04 sco1

Thinking about this again, mutable class variables are dangerous, but I bet there is a lot of valid use cases out there. I'm kinda of worried about this being noisey ... What do others think there?

cooperlees avatar Apr 21 '22 15:04 cooperlees

I'm not sure how common intentional mutable defaults are, but function call defaults are definitely common (similar to #203, which gave us --extend-immutable-calls for B008).

sco1 avatar Apr 21 '22 16:04 sco1

I understand that this might be intentional. To me when you are using this tool to detect the default-input argument issue, you would expect to catch all of them. Detecting just some cases and not catching the dataclass case confused me a little bit, as I thought that this linter would detect these case

mflova avatar Apr 24 '22 16:04 mflova