C408 recomendations sometimes end in hard to find bugs

[klausmyrseth]

I checked for C408 which is several issues on but not this perticular issue which the recommendation potentialy introduse grave bugs in your code.

C408 asks you to use the literal hence {} instead of dict() when creating a dict. This can work very well but for instance when using this together with attrs or as default values for method parameters this is a critical and hard to track bug.

I can describe this with some code:

import attrs

@attrs.define(kw_only=True)
class BigBug:
  struct_1: dict[str, str] | None = field(init=True, default={})
  struct_2: dict[str, str] | None = field(init=True, default={})

  def horrid_buggy_method(self, struct_3: dict = {}, struct_4: dict = {}) -> tuple[dict, dict]:
    struct_3['foo'] = 'bar'
    struct_4['bar'] = 'foo'
    # magically struct 3 and 4 will contain both keys :P
    return (struct_3, struct_4)

foo = BigBug()
foo.struct_1['bar'] = 'bas'
dict_3, dict_4 = foo.horrid_buggy_method()

In this scenarion both struct_1 and struct_2 will contain {'bar': 'bas'} as the literal will imply same class reference as default.

It's not always the case but in general you quickly learn to just use dict() instead as you avoid those edge cases, but we started using ruff in our projects now and our jrs is flooding the code with this bugs as ruff is "correcting" their code.

Edit: added the method too to illustrate the most common facepalm when it comes to this.

[klausmyrseth]

Just adding some explanation of what im trying to indicate and why C408 is not a simple case Comprehensive explanation of the potential bugs that can arise from using {} to introduce dictionaries in Python, along with insights into why this occurs:

Shared Instance Issues

• Unexpected Modifications: When using {} to create dictionaries, multiple instances may inadvertently share the same underlying dictionary object. This can lead to unexpected modifications across instances, as changes made to one instance's dictionary will also affect the others.
• Difficult Debugging: Identifying and debugging these shared instance issues can be challenging, as the root cause may not be immediately apparent. It can involve tracing through the code to understand how dictionaries are being created and shared.

Reasons for Shared Instances

• Default Values: If {} is used as a default value for a function parameter or class attribute, all instances of that function or class will share the same dictionary. This is because default values are evaluated only once, and the same object is reused for subsequent calls.
• Global Variables: If {} is assigned to a global variable, all modules that import that variable will share the same dictionary. This can lead to unintended side effects if the dictionary is modified in one module.
• Mutable Default Arguments: When using mutable objects like dictionaries as default arguments, it's important to be aware that they can be modified in-place. This can lead to unexpected behavior if the function is called multiple times with different arguments.

Avoiding Shared Instance Bugs

• Create New Dictionaries: To avoid sharing instances, always create new dictionaries using {} or the dict constructor within the scope where they are needed. This ensures that each instance has its own unique dictionary.
• Use Immutable Values: If possible, use immutable values like strings, numbers, or tuples as default arguments to prevent accidental modifications. Be Cautious with Global Variables: Use global variables sparingly and avoid modifying them within functions or classes. - If you need to share data between modules, consider using other mechanisms like classes or modules.

Edit: Yes I been stuck because of this before 🤣

[hauntsaninja]

Both default=dict() and default={} do exactly the same thing and are equally buggy. You want to use default=attr.Factory(dict) or something instead.

[klausmyrseth]

Did a full history walkthrough of the issue: First of all @hauntsaninja is wrong dict() is not the same as {} atleast pre 3.7 you would get the following result with the folowing code:

# Using dict()
dict1 = dict(default_value=0)
dict2 = dict(default_value=0)

dict1["key"] = 10

print(dict1)  # Output: {'default_value': 0, 'key': 10}
print(dict2)  # Output: {'default_value': 0}

# Using {}
dict3 = {"default_value": 0}
dict4 = {"default_value": 0}

dict3["key"] = 10

print(dict3)  # Output: {'default_value': 0, 'key': 10}
print(dict4)  # Output: {'default_value': 0, 'key': 10}  (Both dictionaries are modified)

so {} instances was cached within same scope, while dict() forces a new instance. But 3.7 did some significant changes to this so the two is more similar, and 3.8 more or less made them equal, so it works the way C408 recomends, in version pre 3.7 this is still a huge issue.

There has also been done some minor changes post 3.8 so now in 3.11 it works very very similar and does not have the old issues we have in our older code repos where this is a screaming issue. But there is no need to do attrs factory handling, you can just use {} now in newer python but remember to use dict() in pre 3.7.

Sorry for not doing the full history check on the feature but its still something that you might want to consider in the ruff test as pre 3.8 atleast it might be a very unwise recomendation.