Fix several ResourceWarning: unclosed file

pre_commit/commands/autoupdate.py

@@ -72,7 +72,8 @@ REV_LINE_FMT = '{}rev:{}{}{}'
 
 
 def _write_new_config_file(path, output):
-    original_contents = open(path).read()
+    with open(path) as f:
+        original_contents = f.read()
     output = remove_defaults(output, CONFIG_SCHEMA)
     new_contents = ordered_dump(output, **C.YAML_DUMP_KWARGS)
 

pre_commit/commands/install_uninstall.py

@@ -38,7 +38,8 @@ def _hook_paths(git_root, hook_type):
 def is_our_script(filename):
     if not os.path.exists(filename):
         return False
-    contents = io.open(filename).read()
+    with io.open(filename) as f:
+        contents = f.read()
     return any(h in contents for h in (CURRENT_HASH,) + PRIOR_HASHES)
 
 

pre_commit/git.py

@@ -70,7 +70,8 @@ def get_conflicted_files():
     logger.info('Checking merge-conflict files only.')
     # Need to get the conflicted files from the MERGE_MSG because they could
     # have resolved the conflict by choosing one side or the other
-    merge_msg = open(os.path.join(get_git_dir('.'), 'MERGE_MSG'), 'rb').read()
+    with open(os.path.join(get_git_dir('.'), 'MERGE_MSG'), 'rb') as f:
+        merge_msg = f.read()
     merge_conflict_filenames = parse_merge_msg_for_conflicts(merge_msg)
 
     # This will get the rest of the changes made after the merge.

pre_commit/repository.py

@@ -43,7 +43,8 @@ def _read_state(prefix, venv):
     if not os.path.exists(filename):
         return None
     else:
-        return json.loads(io.open(filename).read())
+        with io.open(filename) as f:
+            return json.loads(f.read())
 
 
 def _write_state(prefix, venv, state):

testing/fixtures.py

@@ -40,7 +40,8 @@ def modify_manifest(path):
     .pre-commit-hooks.yaml.
     """
     manifest_path = os.path.join(path, C.MANIFEST_FILE)
-    manifest = ordered_load(io.open(manifest_path).read())
+    with io.open(manifest_path) as f:
+        manifest = ordered_load(f.read())
     yield manifest
     with io.open(manifest_path, 'w') as manifest_file:
         manifest_file.write(ordered_dump(manifest, **C.YAML_DUMP_KWARGS))
@@ -55,7 +56,8 @@ def modify_config(path='.', commit=True):
     .pre-commit-config.yaml
     """
     config_path = os.path.join(path, C.CONFIG_FILE)
-    config = ordered_load(io.open(config_path).read())
+    with io.open(config_path) as f:
+        config = ordered_load(f.read())
     yield config
     with io.open(config_path, 'w', encoding='UTF-8') as config_file:
         config_file.write(ordered_dump(config, **C.YAML_DUMP_KWARGS))
@@ -100,7 +102,8 @@ def make_config_from_repo(repo_path, rev=None, hooks=None, check=True):
 
 def read_config(directory, config_file=C.CONFIG_FILE):
     config_path = os.path.join(directory, config_file)
-    config = ordered_load(io.open(config_path).read())
+    with io.open(config_path) as f:
+        config = ordered_load(f.read())
     return config
 
 

tests/commands/autoupdate_test.py

@@ -42,10 +42,12 @@ def test_autoupdate_up_to_date_repo(up_to_date_repo, in_tmpdir, store):
     config = make_config_from_repo(up_to_date_repo, check=False)
     write_config('.', config)
 
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     assert '^$' not in before
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 0
     assert before == after
 
@@ -68,9 +70,11 @@ def test_autoupdate_old_revision_broken(tempdir_factory, in_tmpdir, store):
 
     config['rev'] = rev
     write_config('.', config)
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 0
     assert before != after
     assert update_rev in after
@@ -106,9 +110,11 @@ def test_autoupdate_out_of_date_repo(out_of_date_repo, in_tmpdir, store):
     )
     write_config('.', config)
 
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 0
     assert before != after
     # Make sure we don't add defaults
@@ -128,10 +134,12 @@ def test_autoupdate_out_of_date_repo_with_correct_repo_name(
     write_config('.', config)
 
     runner = Runner('.', C.CONFIG_FILE)
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     repo_name = 'file://{}'.format(out_of_date_repo.path)
     ret = autoupdate(runner, store, tags_only=False, repos=(repo_name,))
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 0
     assert before != after
     assert out_of_date_repo.head_rev in after
@@ -148,10 +156,12 @@ def test_autoupdate_out_of_date_repo_with_wrong_repo_name(
     write_config('.', config)
 
     runner = Runner('.', C.CONFIG_FILE)
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     # It will not update it, because the name doesn't match
     ret = autoupdate(runner, store, tags_only=False, repos=('dne',))
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 0
     assert before == after
 
@@ -171,7 +181,8 @@ def test_does_not_reformat(in_tmpdir, out_of_date_repo, store):
         f.write(config)
 
     autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     expected = fmt.format(out_of_date_repo.path, out_of_date_repo.head_rev)
     assert after == expected
 
@@ -200,7 +211,8 @@ def test_loses_formatting_when_not_detectable(
         f.write(config)
 
     autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     expected = (
         'repos:\n'
         '-   repo: {}\n'
@@ -225,7 +237,8 @@ def test_autoupdate_tagged_repo(tagged_repo, in_tmpdir, store):
 
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
     assert ret == 0
-    assert 'v1.2.3' in open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        assert 'v1.2.3' in f.read()
 
 
 @pytest.fixture
@@ -243,7 +256,8 @@ def test_autoupdate_tags_only(tagged_repo_with_more_commits, in_tmpdir, store):
 
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=True)
     assert ret == 0
-    assert 'v1.2.3' in open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        assert 'v1.2.3' in f.read()
 
 
 @pytest.fixture
@@ -282,9 +296,11 @@ def test_autoupdate_hook_disappearing_repo(
     )
     write_config('.', config)
 
-    before = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        before = f.read()
     ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False)
-    after = open(C.CONFIG_FILE).read()
+    with open(C.CONFIG_FILE) as f:
+        after = f.read()
     assert ret == 1
     assert before == after
 

tests/commands/install_uninstall_test.py

@@ -415,7 +415,8 @@ def test_replace_old_commit_script(tempdir_factory, store):
         runner = Runner(path, C.CONFIG_FILE)
 
         # Install a script that looks like our old script
-        pre_commit_contents = io.open(resource_filename('hook-tmpl')).read()
+        with io.open(resource_filename('hook-tmpl')) as f:
+            pre_commit_contents = f.read()
         new_contents = pre_commit_contents.replace(
             CURRENT_HASH, PRIOR_HASHES[-1],
         )

tests/conftest.py

@@ -21,6 +21,26 @@ from testing.fixtures import write_config
 from testing.util import cwd
 
 
+@pytest.fixture(autouse=True)
+def no_warnings(recwarn):
+    yield
+    warnings = []
+    for warning in recwarn:  # pragma: no cover
+        message = str(warning.message)
+        # ImportWarning: Not importing directory '...' missing __init__(.py)
+        if not (
+            isinstance(warning.message, ImportWarning)
+            and message.startswith('Not importing directory ')
+            and ' missing __init__' in message
+        ):
+            warnings.append('{}:{} {}'.format(
+                warning.filename,
+                warning.lineno,
+                message,
+            ))
+    assert not warnings
+
+
 @pytest.fixture
 def tempdir_factory(tmpdir):
     class TmpdirFactory(object):

tests/error_handler_test.py

@@ -87,8 +87,8 @@ def test_log_and_exit(cap_out, mock_store_dir):
     )
 
     assert os.path.exists(log_file)
-    contents = io.open(log_file).read()
+    with io.open(log_file) as f:
-    assert contents == (
+        assert f.read() == (
             'msg: FatalError: hai\n'
             "I'm a stacktrace\n"
         )

tests/staged_files_only_test.py

@@ -48,7 +48,8 @@ def _test_foo_state(
         encoding='UTF-8',
 ):
     assert os.path.exists(path.foo_filename)
-    assert io.open(path.foo_filename, encoding=encoding).read() == foo_contents
+    with io.open(path.foo_filename, encoding=encoding) as f:
+        assert f.read() == foo_contents
     actual_status = get_short_git_status()['foo']
     assert status == actual_status
 
@@ -144,10 +145,9 @@ def img_staged(tempdir_factory):
 
 def _test_img_state(path, expected_file='img1.jpg', status='A'):
     assert os.path.exists(path.img_filename)
-    assert (
+    with io.open(path.img_filename, 'rb') as f1,\
-        io.open(path.img_filename, 'rb').read() ==
+            io.open(get_resource_path(expected_file), 'rb') as f2:
-        io.open(get_resource_path(expected_file), 'rb').read()
+        assert f1.read() == f2.read()
-    )
     actual_status = get_short_git_status()['img.jpg']
     assert status == actual_status