Merge pull request #1544 from pre-commit/wip_warning_on_old_config_style

warn on old list-style configuration

pre_commit/clientlib.py

@@ -12,8 +12,10 @@ import cfgv
 from identify.identify import ALL_TAGS
 
 import pre_commit.constants as C
+from pre_commit.color import add_color_option
 from pre_commit.error_handler import FatalError
 from pre_commit.languages.all import all_languages
+from pre_commit.logging_handler import logging_handler
 from pre_commit.util import parse_version
 from pre_commit.util import yaml_load
 
@@ -43,6 +45,7 @@ def _make_argparser(filenames_help: str) -> argparse.ArgumentParser:
     parser = argparse.ArgumentParser()
     parser.add_argument('filenames', nargs='*', help=filenames_help)
     parser.add_argument('-V', '--version', action='version', version=C.VERSION)
+    add_color_option(parser)
     return parser
 
 
@@ -92,14 +95,16 @@ load_manifest = functools.partial(
 def validate_manifest_main(argv: Optional[Sequence[str]] = None) -> int:
     parser = _make_argparser('Manifest filenames.')
     args = parser.parse_args(argv)
-    ret = 0
+
-    for filename in args.filenames:
+    with logging_handler(args.color):
-        try:
+        ret = 0
-            load_manifest(filename)
+        for filename in args.filenames:
-        except InvalidManifestError as e:
+            try:
-            print(e)
+                load_manifest(filename)
-            ret = 1
+            except InvalidManifestError as e:
-    return ret
+                print(e)
+                ret = 1
+        return ret
 
 
 LOCAL = 'local'
@@ -290,7 +295,11 @@ class InvalidConfigError(FatalError):
 def ordered_load_normalize_legacy_config(contents: str) -> Dict[str, Any]:
     data = yaml_load(contents)
     if isinstance(data, list):
-        # TODO: Once happy, issue a deprecation warning and instructions
+        logger.warning(
+            'normalizing pre-commit configuration to a top-level map.  '
+            'support for top level list will be removed in a future version.  '
+            'run: `pre-commit migrate-config` to automatically fix this.',
+        )
         return {'repos': data}
     else:
         return data
@@ -307,11 +316,13 @@ load_config = functools.partial(
 def validate_config_main(argv: Optional[Sequence[str]] = None) -> int:
     parser = _make_argparser('Config filenames.')
     args = parser.parse_args(argv)
-    ret = 0
+
-    for filename in args.filenames:
+    with logging_handler(args.color):
-        try:
+        ret = 0
-            load_config(filename)
+        for filename in args.filenames:
-        except InvalidConfigError as e:
+            try:
-            print(e)
+                load_config(filename)
-            ret = 1
+            except InvalidConfigError as e:
-    return ret
+                print(e)
+                ret = 1
+        return ret

pre_commit/color.py

@@ -1,3 +1,4 @@
+import argparse
 import os
 import sys
 
@@ -95,3 +96,12 @@ def use_color(setting: str) -> bool:
             os.getenv('TERM') != 'dumb'
         )
     )
+
+
+def add_color_option(parser: argparse.ArgumentParser) -> None:
+    parser.add_argument(
+        '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'),
+        type=use_color,
+        metavar='{' + ','.join(COLOR_CHOICES) + '}',
+        help='Whether to use color in output.  Defaults to `%(default)s`.',
+    )

pre_commit/main.py

@@ -8,8 +8,8 @@ from typing import Sequence
 from typing import Union
 
 import pre_commit.constants as C
-from pre_commit import color
 from pre_commit import git
+from pre_commit.color import add_color_option
 from pre_commit.commands.autoupdate import autoupdate
 from pre_commit.commands.clean import clean
 from pre_commit.commands.gc import gc
@@ -41,15 +41,6 @@ os.environ.pop('__PYVENV_LAUNCHER__', None)
 COMMANDS_NO_GIT = {'clean', 'gc', 'init-templatedir', 'sample-config'}
 
 
-def _add_color_option(parser: argparse.ArgumentParser) -> None:
-    parser.add_argument(
-        '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'),
-        type=color.use_color,
-        metavar='{' + ','.join(color.COLOR_CHOICES) + '}',
-        help='Whether to use color in output.  Defaults to `%(default)s`.',
-    )
-
-
 def _add_config_option(parser: argparse.ArgumentParser) -> None:
     parser.add_argument(
         '-c', '--config', default=C.CONFIG_FILE,
@@ -195,7 +186,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
         'autoupdate',
         help="Auto-update pre-commit config to the latest repos' versions.",
     )
-    _add_color_option(autoupdate_parser)
+    add_color_option(autoupdate_parser)
     _add_config_option(autoupdate_parser)
     autoupdate_parser.add_argument(
         '--bleeding-edge', action='store_true',
@@ -216,11 +207,11 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
     clean_parser = subparsers.add_parser(
         'clean', help='Clean out pre-commit files.',
     )
-    _add_color_option(clean_parser)
+    add_color_option(clean_parser)
     _add_config_option(clean_parser)
 
     hook_impl_parser = subparsers.add_parser('hook-impl')
-    _add_color_option(hook_impl_parser)
+    add_color_option(hook_impl_parser)
     _add_config_option(hook_impl_parser)
     hook_impl_parser.add_argument('--hook-type')
     hook_impl_parser.add_argument('--hook-dir')
@@ -230,7 +221,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
     hook_impl_parser.add_argument(dest='rest', nargs=argparse.REMAINDER)
 
     gc_parser = subparsers.add_parser('gc', help='Clean unused cached repos.')
-    _add_color_option(gc_parser)
+    add_color_option(gc_parser)
     _add_config_option(gc_parser)
 
     init_templatedir_parser = subparsers.add_parser(
@@ -240,7 +231,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
             '`git config init.templateDir`.'
         ),
     )
-    _add_color_option(init_templatedir_parser)
+    add_color_option(init_templatedir_parser)
     _add_config_option(init_templatedir_parser)
     init_templatedir_parser.add_argument(
         'directory', help='The directory in which to write the hook script.',
@@ -256,7 +247,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
     install_parser = subparsers.add_parser(
         'install', help='Install the pre-commit script.',
     )
-    _add_color_option(install_parser)
+    add_color_option(install_parser)
     _add_config_option(install_parser)
     install_parser.add_argument(
         '-f', '--overwrite', action='store_true',
@@ -286,32 +277,32 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
             'useful.'
         ),
     )
-    _add_color_option(install_hooks_parser)
+    add_color_option(install_hooks_parser)
     _add_config_option(install_hooks_parser)
 
     migrate_config_parser = subparsers.add_parser(
         'migrate-config',
         help='Migrate list configuration to new map configuration.',
     )
-    _add_color_option(migrate_config_parser)
+    add_color_option(migrate_config_parser)
     _add_config_option(migrate_config_parser)
 
     run_parser = subparsers.add_parser('run', help='Run hooks.')
-    _add_color_option(run_parser)
+    add_color_option(run_parser)
     _add_config_option(run_parser)
     _add_run_options(run_parser)
 
     sample_config_parser = subparsers.add_parser(
         'sample-config', help=f'Produce a sample {C.CONFIG_FILE} file',
     )
-    _add_color_option(sample_config_parser)
+    add_color_option(sample_config_parser)
     _add_config_option(sample_config_parser)
 
     try_repo_parser = subparsers.add_parser(
         'try-repo',
         help='Try the hooks in a repository, useful for developing new hooks.',
     )
-    _add_color_option(try_repo_parser)
+    add_color_option(try_repo_parser)
     _add_config_option(try_repo_parser)
     try_repo_parser.add_argument(
         'repo', help='Repository to source hooks from.',
@@ -328,7 +319,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
     uninstall_parser = subparsers.add_parser(
         'uninstall', help='Uninstall the pre-commit script.',
     )
-    _add_color_option(uninstall_parser)
+    add_color_option(uninstall_parser)
     _add_config_option(uninstall_parser)
     _add_hook_type_option(uninstall_parser)
 

tests/clientlib_test.py

@@ -30,6 +30,10 @@ def test_check_type_tag_failures(value):
         check_type_tag(value)
 
 
+def test_check_type_tag_success():
+    check_type_tag('file')
+
+
 @pytest.mark.parametrize(
     ('config_obj', 'expected'), (
         (
@@ -110,15 +114,18 @@ def test_validate_config_main_ok():
     assert not validate_config_main(('.pre-commit-config.yaml',))
 
 
-def test_validate_config_old_list_format_ok(tmpdir):
+def test_validate_config_old_list_format_ok(tmpdir, cap_out):
     f = tmpdir.join('cfg.yaml')
     f.write('-  {repo: meta, hooks: [{id: identity}]}')
     assert not validate_config_main((f.strpath,))
+    start = '[WARNING] normalizing pre-commit configuration to a top-level map'
+    assert cap_out.get().startswith(start)
 
 
 def test_validate_warn_on_unknown_keys_at_repo_level(tmpdir, caplog):
     f = tmpdir.join('cfg.yaml')
     f.write(
+        'repos:\n'
         '-   repo: https://gitlab.com/pycqa/flake8\n'
         '    rev: 3.7.7\n'
         '    hooks:\n'