Consider using with no assign (#4476)

* Emit ``consider-using-with`` also if calls like ``open()`` are used without an assignment

ChangeLog

@@ -9,6 +9,8 @@ Release date: TBA
 ..
   Put new features and bugfixes here and also in 'doc/whatsnew/2.9.rst'
 
+* Fix false negative for ``consider-using-with`` if calls like ``open()`` were used outside of assignment expressions.
+
 * The warning for ``arguments-differ`` now signals explicitly the difference it detected
   by naming the argument or arguments that changed and the type of change that occurred.
 

pylint/checkers/refactoring/refactoring_checker.py

@@ -873,7 +873,7 @@ def visit_call(self, node):
         self._check_quit_exit_call(node)
         self._check_super_with_arguments(node)
         self._check_consider_using_generator(node)
-        self._check_consider_using_with_instead_call(node)
+        self._check_consider_using_with(node)
 
     @staticmethod
     def _has_exit_in_scope(scope):
@@ -1244,11 +1244,9 @@ def _check_swap_variables(self, node):
         "simplify-boolean-expression",
         "consider-using-ternary",
         "consider-swap-variables",
-        "consider-using-with",
     )
     def visit_assign(self, node):
         self._check_swap_variables(node)
-        self._check_consider_using_with_instead_assign(node)
         if self._is_and_or_ternary(node.value):
             cond, truth_value, false_value = self._and_or_ternary_arguments(node.value)
         else:
@@ -1279,24 +1277,20 @@ def visit_assign(self, node):
 
     visit_return = visit_assign
 
-    def _check_consider_using_with_instead_assign(self, node: astroid.Assign):
-        assigned = node.value
-        if isinstance(assigned, astroid.Call):
-            inferred = utils.safe_infer(assigned.func)
-            if (
-                inferred
-                and inferred.qname() in CALLS_RETURNING_CONTEXT_MANAGERS
-                and not _is_inside_context_manager(node)
-            ):
-                self.add_message("consider-using-with", node=node)
-
-    def _check_consider_using_with_instead_call(self, node: astroid.Call):
+    def _check_consider_using_with(self, node: astroid.Call):
         inferred = utils.safe_infer(node.func)
-        if (
-            inferred
-            and inferred.qname() in CALLS_THAT_COULD_BE_REPLACED_BY_WITH
-            and not _is_inside_context_manager(node)
-        ):
+        if not inferred:
+            return
+        could_be_used_in_with = (
+            # things like ``lock.acquire()``
+            inferred.qname() in CALLS_THAT_COULD_BE_REPLACED_BY_WITH
+            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)
+            )
+        )
+        if could_be_used_in_with and not _is_inside_context_manager(node):
             self.add_message("consider-using-with", node=node)
 
     def _check_consider_using_join(self, aug_assign):

pylint/pyreverse/utils.py

@@ -29,7 +29,8 @@ def get_default_options():
     if home:
         rcfile = os.path.join(home, RCFILE)
         try:
-            options = open(rcfile).read().split()
+            with open(rcfile) as file_handle:
+                options = file_handle.read().split()
         except OSError:
             pass  # ignore if no config file found
     return options

tests/functional/b/bad_open_mode_py3.py

@@ -1,4 +1,5 @@
 """Warnings for using open() with an invalid mode string."""
+# pylint: disable=consider-using-with
 
 NAME = "foo.bar"
 open(NAME, "wb")

tests/functional/b/bad_open_mode_py3.txt

@@ -1,6 +1,6 @@
-bad-open-mode:11:0::"""rwx"" is not a valid mode for open."
-bad-open-mode:12:0::"""rr"" is not a valid mode for open."
-bad-open-mode:13:0::"""+"" is not a valid mode for open."
-bad-open-mode:14:0::"""xw"" is not a valid mode for open."
-bad-open-mode:20:0::"""Ua"" is not a valid mode for open."
-bad-open-mode:21:0::"""Ur++"" is not a valid mode for open."
+bad-open-mode:12:0::"""rwx"" is not a valid mode for open."
+bad-open-mode:13:0::"""rr"" is not a valid mode for open."
+bad-open-mode:14:0::"""+"" is not a valid mode for open."
+bad-open-mode:15:0::"""xw"" is not a valid mode for open."
+bad-open-mode:21:0::"""Ua"" is not a valid mode for open."
+bad-open-mode:22:0::"""Ur++"" is not a valid mode for open."

tests/functional/c/consider/consider_using_with.txt

@@ -1,22 +1,22 @@
-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: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:15:9:test_codecs_open:Consider using 'with' for resource-allocating operations
+consider-using-with:20:8:test_urlopen:Consider using 'with' for resource-allocating operations
+consider-using-with:24:8:test_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:28:8:test_named_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:32:8:test_spooled_temporary_file:Consider using 'with' for resource-allocating operations
+consider-using-with:36:8:test_temporary_directory:Consider using 'with' for resource-allocating operations
+consider-using-with:40:12:test_zipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:41:8:test_zipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:45:12:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:50:8:test_pyzipfile:Consider using 'with' for resource-allocating operations
+consider-using-with:57:9:test_tarfile:Consider using 'with' for resource-allocating operations
+consider-using-with:63:9: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:128:4:test_multiprocessing:Consider using 'with' for resource-allocating operations
+consider-using-with:128:8: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
+consider-using-with:144:8:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:148:8:test_futures:Consider using 'with' for resource-allocating operations
+consider-using-with:154:8:test_popen:Consider using 'with' for resource-allocating operations

tests/functional/c/consider/consider_using_with_open.py

@@ -39,3 +39,13 @@ def test_open_in_with_contextlib():
     file_handle = open("foo.txt", "w")  # must not trigger
     yield file_handle
     file_handle.close()
+
+
+def test_open_outside_assignment():
+    open("foo").read()  # [consider-using-with]
+    content = open("foo").read()  # [consider-using-with]
+
+
+def test_open_inside_with_block():
+    with open("foo") as fh:
+        open("bar")  # [consider-using-with]

tests/functional/c/consider/consider_using_with_open.txt

@@ -1,2 +1,5 @@
-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
+consider-using-with:12:9::Consider using 'with' for resource-allocating operations
+consider-using-with:16: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

tests/functional/s/subprocess_popen_preexec_fn.py

@@ -1,4 +1,4 @@
-# pylint: disable=disallowed-name,no-value-for-parameter,missing-docstring
+# pylint: disable=disallowed-name,no-value-for-parameter,missing-docstring,consider-using-with
 
 import subprocess
 

tests/lint/unittest_lint.py

@@ -154,7 +154,8 @@ def create_files(paths, chroot="."):
         if not isdir(dirpath):
             os.makedirs(dirpath)
     for filepath in files:
-        open(filepath, "w").close()
+        with open(filepath, "w"):
+            pass
 
 
 @pytest.fixture