flake8-bugbear
flake8-bugbear copied to clipboard
Possible B020 false positive with instance attribute
This code:
class MyModel:
value: int
class A:
model_instance = MyModel()
test_suite = [1, 2, 3]
def method(self):
for self.model_instance.value in self.test_suite:
print(self.model_instance.value)
results in a B020, but it looks like a false positive to me since self.test_suite is not one of the iterable values. Admittedly it is a slightly unusual and ambiguous case since self is being modified and the plugin can't know what changes setting self.model_instance is making to the object and the value of self.test_suite.
The actual use case is a test where a Django model object is created once in the test set up and, in each iteration, the loop modifies model attributes, passes the model to a function, and tests the result.
I agree this seems a false positive, but it's very creative class instance iteration here. It's also a class variable, so all instances would modify the same value. Dangerous stuff.
To potentially fix this noise will need some thought. I'm not even sure if we can. Let's see what others think.
This one's not my code! It's definitely creative and I had to look twice because I'd not seen such a pattern before.
Note that the B020 is raised if it's an instance variable also, in fact it is in my case but I was trying to make the example simpler.
The real case is like this.
class MyTestCase(TestCase):
test_suite = [1, 2, 3]
def setUp(self):
self.model_instance = MyModel.objects.get(...)
def test_method(self):
for self.model_instance.value in self.test_suite:
print(self.model_instance.value)
I don't see a particularly compelling reason for it to be written this way. An alternative, as an example, would be for the test_method to instantiate a new MyModel each iteration without writing to the DB. But even if it should be written a different way it still concerns me that B020 is an inaccurate description of the style issue.
Introducing another code for this would be helpful (or just ignoring) IMHO.