Implement basic control flow checks for ``consider-using-with`` (#4929)

* Add some control flow checks to consider-using-with in order to handle assignments in different branches
* Use existing function from ``astroid`` to determine whether two statements are exclusive
* Add unit tests for new utility methods
* Add ChangeLog and whatsnew entry

2 files changed, 98 insertions, 6 deletions

diff --git a/tests/functional/c/consider/consider_using_with_open.py b/tests/functional/c/consider/consider_using_with_open.py
index ca807459e..7c584d8f5 100644
--- a/tests/functional/c/consider/consider_using_with_open.py
+++ b/tests/functional/c/consider/consider_using_with_open.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
+# pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel, no-self-use
 # pylint: disable=missing-class-docstring, too-few-public-methods, unused-variable, multiple-statements, line-too-long
 """
 The functional test for the standard ``open()`` function has to be moved in a separate file,
@@ -7,6 +7,7 @@ However, all remaining checks for consider-using-with work in PyPy, so we do not
 PyPy from ALL functional tests.
 """
 from contextlib import contextmanager
+from pathlib import Path
 
 myfile = open("test.txt", encoding="utf-8")  # [consider-using-with]
 
@@ -68,3 +69,93 @@ def test_multiline_with_items(file1, file2, which):
 
 def test_suppress_on_return():
     return open("foo", encoding="utf8")  # must not trigger
+
+
+class TestControlFlow:
+    """
+    The message is triggered if a context manager is assigned to a variable, which name is later
+    reassigned without the variable being used inside a ``with`` first.
+    E.g. the following would trigger the message:
+
+        a = open("foo")  # <-- would trigger here
+        a = "something new"
+
+    But it must not happen that the logic which checks if the same variable is assigned multiple
+    times in different code branches where only one of those assign statements is hit at runtime.
+    For example, the variable could be assigned in an if-else construct.
+
+    These tests check that the message is not triggered in those circumstances.
+    """
+
+    def test_defined_in_if_and_else(self, predicate):
+        if predicate:
+            file_handle = open("foo", encoding="utf8")  # must not trigger
+        else:
+            file_handle = open("bar", encoding="utf8")  # must not trigger
+        with file_handle:
+            return file_handle.read()
+
+    def test_defined_in_else_only(self, predicate):
+        if predicate:
+            result = "shiny watermelon"
+        else:
+            file_handle = open("foo", encoding="utf8")  # must not trigger
+            with file_handle:
+                result = file_handle.read()
+        return result
+
+    def test_defined_in_if_only(self, predicate):
+        if predicate:
+            file_handle = open("foo", encoding="utf8")  # must not trigger
+            with file_handle:
+                result = file_handle.read()
+        else:
+            result = "shiny watermelon"
+        return result
+
+    def test_triggers_if_reassigned_after_if_else(self, predicate):
+        if predicate:
+            file_handle = open("foo", encoding="utf8")
+        else:
+            file_handle = open(  # [consider-using-with]
+                "bar", encoding="utf8"
+            )
+        file_handle = None
+        return file_handle
+
+    def test_defined_in_try_and_except(self):
+        try:
+            file_handle = open("foo", encoding="utf8")  # must not trigger
+        except FileNotFoundError:
+            file_handle = open("bar", encoding="utf8")  # must not trigger
+        with file_handle:
+            return file_handle.read()
+
+    def test_defined_in_try_and_finally(self):
+        try:
+            file_handle = open("foo", encoding="utf8")  # must not trigger
+        except FileNotFoundError:
+            Path("foo").touch()
+        finally:
+            file_handle.open("foo", encoding="utf")  # must not trigger
+        with file_handle:
+            return file_handle.read()
+
+    def test_defined_in_different_except_handlers(self, a, b):
+        try:
+            result = a/b
+        except ZeroDivisionError:
+            logfile = open("math_errors.txt", encoding="utf8")  # must not trigger
+            result = "Can't divide by zero"
+        except TypeError:
+            logfile = open("type_errors.txt", encoding="utf8")  # must not trigger
+            result = "Wrong types"
+        else:
+            logfile = open("results.txt", encoding="utf8")  # must not trigger
+        with logfile:
+            logfile.write(result)
+
+    def test_multiple_return_statements(self, predicate):
+        if predicate:
+            return open("foo", encoding="utf8")  # must not trigger
+        return open("bar", encoding="utf8")  # must not trigger
diff --git a/tests/functional/c/consider/consider_using_with_open.txt b/tests/functional/c/consider/consider_using_with_open.txt
index ea3503123..cf3fdc1c8 100644
--- a/tests/functional/c/consider/consider_using_with_open.txt
+++ b/tests/functional/c/consider/consider_using_with_open.txt
@@ -1,5 +1,6 @@
-consider-using-with:11:9::Consider using 'with' for resource-allocating operations
-consider-using-with:15:9:test_open:Consider using 'with' for resource-allocating operations
-consider-using-with:45:4:test_open_outside_assignment:Consider using 'with' for resource-allocating operations
-consider-using-with:46:14:test_open_outside_assignment:Consider using 'with' for resource-allocating operations
-consider-using-with:51:8:test_open_inside_with_block:Consider using 'with' for resource-allocating operations
+consider-using-with:12:9::Consider using 'with' for resource-allocating operations:HIGH
+consider-using-with:16:9:test_open:Consider using 'with' for resource-allocating operations:HIGH
+consider-using-with:46:4:test_open_outside_assignment:Consider using 'with' for resource-allocating operations:HIGH
+consider-using-with:47:14:test_open_outside_assignment:Consider using 'with' for resource-allocating operations:HIGH
+consider-using-with:52:8:test_open_inside_with_block:Consider using 'with' for resource-allocating operations:HIGH
+consider-using-with:120:26:TestControlFlow.test_triggers_if_reassigned_after_if_else:Consider using 'with' for resource-allocating operations:HIGH