consider-using-with assigning TemporaryDirectory() to class attribute

[ppiasek]

Bug description

class Foo:
    def __init__():
        self.tmp_dir = TemporaryDirectory()

Above implementation causes Pylint to raise "Consider using 'with' for resource-allocating operations" warning. It's true that TemporaryDirectory can be used with context manager, but when user needs random tmp path, that is exists for the whole life of class object then it needs to be assigned to attribute. Also, TemporaryDirectory class has it's own implementation for cleanup outside context manager via weakref.

Configuration

No response

Command used

pylint file.py

Pylint output

************* Module file.Foo
file.py:16:24: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

Expected behavior

TemporaryDirectory class have implementations that allows delete created directories automatically at the end of object existence, so requiring usage of context manager, when we are not opening the files, but just creating directories shouldn't be necessary.

Pylint version

pylint 2.11.1
pylint 2.15.4

OS / Environment

Windows 10

Additional dependencies

No response

[clavedeluna]

I can see the same argument made for other tempfile objects. I think this makes sense.

[clavedeluna]

I propose we remove these lines and any relevant testing from the checker.

You could argue some of the others in that list are also fine not as context managers, but if we remove them all this check would be moot 😛

[DudeNr33]

@clavedeluna the original enhancement request was focussing on the standard open request. I added other context managers from the standard library that release resources in their __exit__ as well, but if we have examples like tempfile here where release of the resource is ensured in some other (automated) way, then I am totally fine with removing them.

What other instances would you propose to remove?

[clavedeluna]

I don't think we should over-do it unless more use cases appear, so removing just the tempfile ones would be fine right now. My comment was a general one, to indicate that if we were to believe authors would always remember to do the cleanup work, then we wouldn't have this check in the first place.

[GideonBear]

This also happens with NamedTemporaryFile(delete=False). The python docs state:

If delete is true (the default), the file is deleted as soon as it is closed.

So this also applies for NamedTemporaryFile if delete=False.

In my case, this occurs in a utility function that just returns tempfile.name

[mschmitzer]

I feel like this is part of a more general problem with consider-using-with that would ideally be fixed by detecting that the context manager object is somehow preserved beyond the scope of the function it is created in. I guess the most typical case is the one shown here, were the context manager object is managed by an instance and thus stored on self, but there could be others.

Another example we have is using a class instance to manage a running process, i.e.:

class Foo:
  ...

  def start(self):
    self._proc = subprocess.Popen(...)

  def stop(self):
    self._proc.terminate()

This likewise currently triggers the consider-using-with warning, but I don't think there's a good way to avoid this. I also don't think this is sufficiently unusual to always add a "disable" comment. So it would be great if Pylint could recognize this and omit the warning.