Issue 4430 false positive consider-using-with R1732 (#4453)

* Suppress consider-using-with if used inside context manager

* Added ChangeLog entry

4 files changed, 74 insertions, 29 deletions

diff --git a/tests/functional/c/consider/consider_using_with.py b/tests/functional/c/consider/consider_using_with.py
index 234c6093d..84b394ce4 100644
--- a/tests/functional/c/consider/consider_using_with.py
+++ b/tests/functional/c/consider/consider_using_with.py
@@ -1,5 +1,6 @@
 # pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
 import codecs
+import contextlib
 import multiprocessing
 import subprocess
 import tarfile
@@ -81,14 +82,6 @@ def test_lock_acquisition():
     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()
@@ -104,6 +97,31 @@ def test_lock_acquisition():
         pass
 
 
+@contextlib.contextmanager
+def test_lock_acquisition_in_context_manager1():
+    """
+    The message must not be triggered if the resource allocation is done inside a context manager.
+    """
+    lock = threading.Lock()
+    lock.acquire()  # must not trigger
+    yield
+    lock.release()
+
+
+class MyLockContext:
+    """
+    The message must not be triggered if the resource allocation is done inside a context manager.
+    """
+    def __init__(self):
+        self.lock = threading.Lock()
+
+    def __enter__(self):
+        self.lock.acquire()  # must not trigger
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.lock.release()
+
+
 def test_multiprocessing():
     # the different Locks provided by multiprocessing would be candidates
     # for consider-using-with as well, but they lead to InferenceErrors.
diff --git a/tests/functional/c/consider/consider_using_with.txt b/tests/functional/c/consider/consider_using_with.txt
index 43ea7f752..e26d359f4 100644
--- a/tests/functional/c/consider/consider_using_with.txt
+++ b/tests/functional/c/consider/consider_using_with.txt
@@ -1,22 +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:15:4:test_codecs_open:Consider using 'with' for resource-allocating operations
+consider-using-with:20:4:test_urlopen:Consider using 'with' for resource-allocating operations
+consider-using-with:24:4:test_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:28:4:test_named_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:32:4:test_spooled_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:36:4:test_temporary_directory: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:41:4:test_zipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:45:4:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:50:4:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:57:4:test_tarfile:Consider using 'with' for resource-allocating operations
+consider-using-with:63:4:test_tarfile:Consider using 'with' for resource-allocating operations
+consider-using-with:72:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:79:4:test_lock_acquisition:Consider using 'with' for resource-allocating operations
+consider-using-with:86: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
+consider-using-with:128:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:133:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:138:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:144:4:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:148:4:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:154: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
index 6e7cb04bd..9b1a97e0d 100644
--- a/tests/functional/c/consider/consider_using_with_open.py
+++ b/tests/functional/c/consider/consider_using_with_open.py
@@ -1,10 +1,15 @@
 # pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
+# pylint: disable=missing-class-docstring, too-few-public-methods, unused-variable
 """
 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.
 """
+from contextlib import contextmanager
+
+
+myfile = open("test.txt")  # [consider-using-with]
 
 
 def test_open():
@@ -13,3 +18,24 @@ def test_open():
 
     with open("test.txt") as fh:  # must not trigger
         fh.read()
+
+
+def test_open_in_enter():
+    """Message must not trigger if the resource is allocated in a context manager."""
+    class MyContextManager:
+        def __init__(self):
+            self.file_handle = None
+
+        def __enter__(self):
+            self.file_handle = open("foo.txt", "w")  # must not trigger
+
+        def __exit__(self, exc_type, exc_value, traceback):
+            self.file_handle.close()
+
+
+@contextmanager
+def test_open_in_with_contextlib():
+    """Message must not trigger if the resource is allocated in a context manager."""
+    file_handle = open("foo.txt", "w")  # must not trigger
+    yield file_handle
+    file_handle.close()
diff --git a/tests/functional/c/consider/consider_using_with_open.txt b/tests/functional/c/consider/consider_using_with_open.txt
index 8e7c03e39..5bd0e4e8a 100644
--- a/tests/functional/c/consider/consider_using_with_open.txt
+++ b/tests/functional/c/consider/consider_using_with_open.txt
@@ -1 +1,2 @@
-consider-using-with:11:4:test_open:Consider using 'with' for resource-allocating operations
+consider-using-with:12:0::Consider using 'with' for resource-allocating operations
+consider-using-with:16:4:test_open:Consider using 'with' for resource-allocating operations