Enhancement #3413 ``consider-using-with`` (#4372)

* Implement consider-using-with check
* Fix or disable consider-using-with in codebase
* Fix ticket number in ChangeLog
* Move functional test for ``open()`` into separate testfile and exclude this test from running with PyPy

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

5 files changed, 178 insertions, 0 deletions

diff --git a/tests/functional/c/consider/consider_using_with.py b/tests/functional/c/consider/consider_using_with.py
new file mode 100644
index 000000000..234c6093d
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_with.py
@@ -0,0 +1,138 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
+import codecs
+import multiprocessing
+import subprocess
+import tarfile
+import tempfile
+import threading
+import urllib
+import zipfile
+from concurrent import futures
+
+
+def test_codecs_open():
+    fh = codecs.open("test.txt", "utf8")  # [consider-using-with]
+    fh.close()
+
+
+def test_urlopen():
+    _ = urllib.request.urlopen("http://www.python.org")  # [consider-using-with]
+
+
+def test_temporary_file():
+    _ = tempfile.TemporaryFile("r")  # [consider-using-with]
+
+
+def test_named_temporary_file():
+    _ = tempfile.NamedTemporaryFile("r")  # [consider-using-with]
+
+
+def test_spooled_temporary_file():
+    _ = tempfile.SpooledTemporaryFile("r")  # [consider-using-with]
+
+
+def test_temporary_directory():
+    _ = tempfile.TemporaryDirectory()  # [consider-using-with]
+
+
+def test_zipfile():
+    myzip = zipfile.ZipFile("spam.zip", "w")  # [consider-using-with]
+    _ = myzip.open("eggs.txt")  # [consider-using-with]
+
+
+def test_pyzipfile():
+    myzip = zipfile.PyZipFile("spam.zip", "w")  # [consider-using-with]
+
+    with zipfile.PyZipFile("spam.zip", "w"):  # must not trigger
+        pass
+
+    _ = myzip.open("eggs.txt")  # [consider-using-with]
+
+    with myzip.open("eggs.txt"):  # must not trigger
+        pass
+
+
+def test_tarfile():
+    tf = tarfile.open("/tmp/test.tar", "w")  # [consider-using-with]
+    tf.close()
+
+    with tarfile.open("/tmp/test.tar", "w"):  # must not trigger
+        pass
+
+    tf = tarfile.TarFile("/tmp/test2.tar", "w")  # [consider-using-with]
+    tf.close()
+
+    with tarfile.TarFile("/tmp/test2.tar", "w"):  # must not trigger
+        pass
+
+
+def test_lock_acquisition():
+    lock = threading.Lock()
+    lock.acquire()  # [consider-using-with]
+    lock.release()
+
+    with lock:  # must not trigger
+        pass
+
+    rlock = threading.RLock()
+    rlock.acquire()  # [consider-using-with]
+    rlock.release()
+
+    with rlock:  # must not trigger
+        pass
+
+    # Not working currently
+    # condition = threading.Condition()
+    # condition.acquire()
+    # condition.release()
+
+    # with condition:  # must not trigger
+    #     pass
+
+    sema = threading.Semaphore()
+    sema.acquire()  # [consider-using-with]
+    sema.release()
+
+    with sema:  # must not trigger
+        pass
+
+    bounded_sema = threading.BoundedSemaphore()
+    bounded_sema.acquire()  # [consider-using-with]
+    bounded_sema.release()
+
+    with bounded_sema:  # must not trigger
+        pass
+
+
+def test_multiprocessing():
+    # the different Locks provided by multiprocessing would be candidates
+    # for consider-using-with as well, but they lead to InferenceErrors.
+    _ = multiprocessing.Pool()  # [consider-using-with]
+    with multiprocessing.Pool():
+        pass
+
+    manager = multiprocessing.managers.BaseManager()
+    manager.start()  # [consider-using-with]
+    with multiprocessing.managers.BaseManager():
+        pass
+
+    manager = multiprocessing.managers.SyncManager()
+    manager.start()  # [consider-using-with]
+    with multiprocessing.managers.SyncManager():
+        pass
+
+
+def test_futures():
+    _ = futures.ThreadPoolExecutor()  # [consider-using-with]
+    with futures.ThreadPoolExecutor():
+        pass
+
+    _ = futures.ProcessPoolExecutor()  # [consider-using-with]
+    with futures.ProcessPoolExecutor():
+        pass
+
+
+def test_popen():
+    _ = subprocess.Popen("sh")  # [consider-using-with]
+    with subprocess.Popen("sh"):
+        pass
diff --git a/tests/functional/c/consider/consider_using_with.txt b/tests/functional/c/consider/consider_using_with.txt
new file mode 100644
index 000000000..43ea7f752
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_with.txt
@@ -0,0 +1,22 @@
+consider-using-with:14:4:test_codecs_open:Consider using 'with' for resource-allocating operations
+consider-using-with:19:4:test_urlopen:Consider using 'with' for resource-allocating operations
+consider-using-with:23:4:test_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:27:4:test_named_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:31:4:test_spooled_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:35:4:test_temporary_directory:Consider using 'with' for resource-allocating operations
+consider-using-with:39:4:test_zipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:40:4:test_zipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:44:4:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:49:4:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:56:4:test_tarfile:Consider using 'with' for resource-allocating operations
+consider-using-with:62:4:test_tarfile:Consider using 'with' for resource-allocating operations
+consider-using-with:71:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:78:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:93:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:100:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:110:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:115:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:120:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:126:4:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:130:4:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:136:4:test_popen:Consider using 'with' for resource-allocating operations
diff --git a/tests/functional/c/consider/consider_using_with_open.py b/tests/functional/c/consider/consider_using_with_open.py
new file mode 100644
index 000000000..6e7cb04bd
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_with_open.py
@@ -0,0 +1,15 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
+"""
+The functional test for the standard ``open()`` function has to be moved in a separate file,
+because PyPy has to be excluded for the tests as the ``open()`` function is uninferable in PyPy.
+However, all remaining checks for consider-using-with work in PyPy, so we do not want to exclude
+PyPy from ALL functional tests.
+"""
+
+
+def test_open():
+    fh = open("test.txt")  # [consider-using-with]
+    fh.close()
+
+    with open("test.txt") as fh:  # must not trigger
+        fh.read()
diff --git a/tests/functional/c/consider/consider_using_with_open.rc b/tests/functional/c/consider/consider_using_with_open.rc
new file mode 100644
index 000000000..b47a74525
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_with_open.rc
@@ -0,0 +1,2 @@
+[testoptions]
+except_implementations=PyPy
diff --git a/tests/functional/c/consider/consider_using_with_open.txt b/tests/functional/c/consider/consider_using_with_open.txt
new file mode 100644
index 000000000..8e7c03e39
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_with_open.txt
@@ -0,0 +1 @@
+consider-using-with:11:4:test_open:Consider using 'with' for resource-allocating operations