Fix false-positive 'consider-using-with' for ternary inside 'with' (#4679)

As things like ternary conditionals may be used inside the items of a 
with, it is not enough to just check the first parent node to determine
if the call is happening inside a with.

4 files changed, 39 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index ca15b4a78..c2046d29f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -17,6 +17,10 @@ Release date: TBA
 ..
   Put bug fixes that should not wait for a new minor version here
 
+* Fix false-positive ``consider-using-with`` (R1732) if a ternary conditional is used together with ``with``
+
+  Closes #4676
+
 * Fix false-positive ``deprecated-module`` when relative import uses deprecated module name.
 
   Closes #4629
diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst
index a9a3336a8..e3af4322b 100644
--- a/doc/whatsnew/2.9.rst
+++ b/doc/whatsnew/2.9.rst
@@ -62,6 +62,8 @@ New checkers
 Other Changes
 =============
 
+* Fix false-positive ``consider-using-with`` (R1732) if a ternary conditional is used together with ``with``
+
 * Fix false-positive ``consider-using-with`` (R1732) if ``contextlib.ExitStack`` takes care of calling the ``__exit__`` method
 
 * Add type annotations to pyreverse dot files
diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index 0ce6aa360..2225a7e4a 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -114,6 +114,22 @@ def _is_inside_context_manager(node: astroid.Call) -> bool:
     )
 
 
+def _is_part_of_with_items(node: astroid.Call) -> bool:
+    """
+    Checks if one of the node's parents is an ``astroid.With`` node and that the node itself is located
+    somewhere under its ``items``.
+    """
+    frame = node.frame()
+    current = node
+    while current != frame:
+        if isinstance(current, astroid.With):
+            items_start = current.items[0][0].lineno
+            items_end = current.items[-1][0].tolineno
+            return items_start <= node.lineno <= items_end
+        current = current.parent
+    return False
+
+
 def _will_be_released_automatically(node: astroid.Call) -> bool:
     """Checks if a call that could be used in a ``with`` statement is used in an alternative
     construct which would ensure that its __exit__ method is called."""
@@ -1313,7 +1329,7 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             or (
                 # things like ``open("foo")`` which are not already inside a ``with`` statement
                 inferred.qname() in CALLS_RETURNING_CONTEXT_MANAGERS
-                and not isinstance(node.parent, astroid.With)
+                and not _is_part_of_with_items(node)
             )
         )
         if could_be_used_in_with and not (
diff --git a/tests/functional/c/consider/consider_using_with_open.py b/tests/functional/c/consider/consider_using_with_open.py
index 9001a5279..e93b619b7 100644
--- a/tests/functional/c/consider/consider_using_with_open.py
+++ b/tests/functional/c/consider/consider_using_with_open.py
@@ -1,5 +1,5 @@
 # pylint: disable=missing-function-docstring, missing-module-docstring, invalid-name, import-outside-toplevel
-# pylint: disable=missing-class-docstring, too-few-public-methods, unused-variable
+# pylint: disable=missing-class-docstring, too-few-public-methods, unused-variable, multiple-statements
 """
 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.
@@ -49,3 +49,18 @@ def test_open_outside_assignment():
 def test_open_inside_with_block():
     with open("foo") as fh:
         open("bar")  # [consider-using-with]
+
+
+def test_ternary_if_in_with_block(file1, file2, which):
+    """Regression test for issue #4676 (false positive)"""
+    with (open(file1) if which else open(file2)) as input_file:  # must not trigger
+        return input_file.read()
+
+
+def test_single_line_with(file1):
+    with open(file1): return file1.read()  # must not trigger
+
+
+def test_multiline_with_items(file1, file2, which):
+    with (open(file1) if which
+          else open(file2)) as input_file:  return input_file.read()