Merge pull request #45 from ESSS/allow-ignore-patterns

Allow exclude files and directories from formatting

Author: Igor T. Ghisi

CHANGELOG.rst

@@ -2,6 +2,11 @@
 History
 =======
 
+2.1.0
+------------------
+
+* ``fix-format`` now allows exclude patterns to be configured through ``pyproject.toml``.
+
 2.0.1 (2019-06-26)
 ------------------
 

README.rst

@@ -53,18 +53,38 @@ Use ``fix-format`` (or ``ff`` for short) to reorder imports and format source co
 
 .. _black:
 
+Options
+-------
+
+Options for ``fix-format`` are defined in the section ``[tool.esss_fix_format]]`` of a ``pyproject.toml`` file. The
+TOML file should be placed in an ancestor directory of the filenames passed on the command-line.
+
+
+Exclude
+^^^^^^^
+
+A list of file name patterns to be excluded from the formatting. Patterns are matched using python ``fnmatch``:
+
+   .. code-block:: toml
+
+    [tool.esss_fix_format]
+    exclude = [
+        "src/generated/*.py",
+        "tmp/*",
+    ]
+
+
 Black
 ^^^^^
 
 Since version ``2.0.0`` it is possible to use `black <https://github.com/python/black>`__ as the
 code formatter for Python code.
 
-``fix-format`` will use ``black`` automatically if it finds a ``pyproject.toml`` with a ``[tool.black]`` section in an
-ancestor directories of the filenames passed on the command-line.
+``fix-format`` will use ``black`` automatically if it finds a ``[tool.black]`` section declared in ``pyproject.toml``
+file.
 
 See "Converting master to black" below for details.
 
-
 Migrating a project to use fix-format
 -------------------------------------
 

environment.devenv.yml

@@ -9,6 +9,7 @@ dependencies:
   - isort>=4.3.4
   - python={{ '.'.join(CONDA_PY) }}
   - pydevf==0.1.5
+  - toml>=0.8.0
 
   # develop
   - black

setup.py

@@ -11,6 +11,7 @@
     'click>=6.0',
     'isort',
     'pydevf==0.1.5',
+    'toml>=0.8.0',
 ]
 
 setup(

src/esss_fix_format/cli.py

@@ -1,15 +1,16 @@
+#!python
 import codecs
 import io
 import os
 import re
 import subprocess
 import sys
-from typing import Optional, Tuple
+from pathlib import Path
+from typing import Optional, Tuple, Iterable, List
 
 import boltons.iterutils
 import click
 import pydevf
-from pathlib import Path
 
 CPP_PATTERNS = {
     '*.cpp',
@@ -42,15 +43,27 @@ def is_cpp(filename):
     return any(fnmatch(os.path.basename(filename), p) for p in CPP_PATTERNS)
 
 
-def should_format(filename):
+def should_format(filename: str, include_patterns: Iterable[str], exclude_patterns: Iterable[str]):
     """
-    Return a tuple (fmt, reason) where fmt is True if the filename
-    is of a type that is supported by this tool.
+    Return a tuple (fmt, reason) where fmt is True if the filename should be formatted.
+
+    :param filename: file name to verify if should be formatted or not
+
+    :param include_patterns: list of file patterns to be included in the formatting
+
+    :param exclude_patterns: list of file patterns to be excluded from formatting. Has precedence
+        over `include_patterns`
+
+    :rtype: Tuple[bool, str]
     """
     from fnmatch import fnmatch
+
+    if any(fnmatch(filename, pattern) for pattern in exclude_patterns):
+        return False, 'Excluded file'
+
     filename_no_ext, ext = os.path.splitext(filename)
-    ipynb_filename = filename_no_ext + '.ipynb'
     # ignore .py file that has a jupytext configured notebook with the same base name
+    ipynb_filename = filename_no_ext + '.ipynb'
     if ext == '.py' and os.path.isfile(ipynb_filename):
         with open(ipynb_filename, 'rb') as f:
             if b'jupytext' not in f.read():
@@ -59,12 +72,14 @@ def should_format(filename):
             if b'jupytext:' not in f.read():
                 return True, ''
         return False, 'Jupytext generated file'
-    if any(fnmatch(os.path.basename(filename), p) for p in PATTERNS):
+
+    if any(fnmatch(os.path.basename(filename), pattern) for pattern in include_patterns):
         return True, ''
+
     return False, 'Unknown file type'
 
 
-def find_black_config(files_or_directories) -> Optional[Path]:
+def find_pyproject_toml(files_or_directories) -> Optional[Path]:
     """
     Searches for a valid pyproject.toml file based on the list of files/directories given.
 
@@ -76,11 +91,35 @@ def find_black_config(files_or_directories) -> Optional[Path]:
     common = Path(os.path.commonpath(files_or_directories)).resolve()
     for p in ([common] + list(common.parents)):
         fn = p / 'pyproject.toml'
-        if fn.is_file() and '[tool.black]' in fn.read_text(encoding='UTF-8'):
+        if fn.is_file():
             return fn
     return None
 
 
+def read_exclude_patterns(pyproject_toml: Path) -> List[str]:
+    import toml
+
+    toml_contents = toml.load(pyproject_toml)
+    ff_options = toml_contents.get('tool', {}).get('esss_fix_format', {})
+    excludes_option = ff_options.get('exclude', [])
+    if not isinstance(excludes_option, list):
+        raise TypeError(
+            f"pyproject.toml excludes option must be a list, got {type(excludes_option)})"
+        )
+
+    # Fix exclude paths based on cwd (exclude paths are defined relative to TOML file)
+    cwd_relpath_from_toml = os.path.relpath(os.getcwd(), pyproject_toml.parent)
+    if cwd_relpath_from_toml != '.':
+        excludes_option = [p.replace(cwd_relpath_from_toml + '/', '', 1) for p in excludes_option]
+    return excludes_option
+
+
+def has_black_config(pyproject_toml: Optional[Path]) -> bool:
+    if pyproject_toml is None:
+        return False
+    return pyproject_toml.is_file() and '[tool.black]' in pyproject_toml.read_text(encoding='UTF-8')
+
+
 # caches which directories have the `.clang-format` file, *in or above it*, to avoid hitting the
 # disk too many times
 __HAS_DOT_CLANG_FORMAT = dict()
@@ -313,7 +352,7 @@ def _process_file(filename, check, format_code, *, verbose):
     return changed, errors, formatter
 
 
-def run_black_on_python_files(files, check, verbose) -> Tuple[bool, bool]:
+def run_black_on_python_files(files, check, exclude_patterns, verbose) -> Tuple[bool, bool]:
     """
     Runs black on the given files (checking or formatting).
 
@@ -326,7 +365,7 @@ def run_black_on_python_files(files, check, verbose) -> Tuple[bool, bool]:
 
     :return: a pair (would_be_formatted, black_failed)
     """
-    py_files = [x for x in files if x.suffix == '.py' and should_format(str(x))[0]]
+    py_files = [x for x in files if should_format(str(x), ['*.py'], exclude_patterns)[0]]
     black_failed = False
     would_be_formatted = False
     if py_files:
@@ -367,27 +406,35 @@ def _main(files_or_directories, check, stdin, commit, pydevf_format_func, *, ver
                     for dirname in list(dirs):
                         if dirname in SKIP_DIRS:
                             dirs.remove(dirname)
-                    files.extend(os.path.join(root, n) for n in names if should_format(n))
+                    files.extend(
+                        os.path.join(root, n) for n in names if should_format(n, PATTERNS, [])
+                    )
             else:
                 files.append(file_or_dir)
 
     files = sorted(Path(x) for x in files)
     errors = []
 
-    black_config = find_black_config(files)
+    pyproject_toml = find_pyproject_toml(files)
+    if pyproject_toml:
+        exclude_patterns = read_exclude_patterns(pyproject_toml)
+    else:
+        exclude_patterns = []
+
     would_be_formatted = False
-    if black_config:
+    if has_black_config(pyproject_toml):
         # skip pydevf formatter
         pydevf_format_func = None
-        would_be_formatted, black_failed = run_black_on_python_files(files, check, verbose)
+        would_be_formatted, black_failed = \
+            run_black_on_python_files(files, check, exclude_patterns, verbose)
         if black_failed:
             errors.append('Error formatting black (see console)')
 
     changed_files = []
     analysed_files = []
     for filename in files:
         filename = str(filename)
-        fmt, reason = should_format(filename)
+        fmt, reason = should_format(filename, PATTERNS, exclude_patterns)
         if not fmt:
             if verbose:
                 click.secho(click.format_filename(filename) + ': ' + reason, fg='white')

tests/test_esss_fix_format.py

@@ -636,28 +636,30 @@ def check_invalid_file(input_file, formatter=None):
     output.fnmatch_lines(str(input_file) + ': Failed' + _get_formatter_msg(formatter))
 
 
-def test_find_black_config(tmp_path, monkeypatch):
+def test_find_pyproject_toml(tmp_path, monkeypatch):
     (tmp_path / 'pA/p2/p3').mkdir(parents=True)
     (tmp_path / 'pA/p2/p3/foo.py').touch()
     (tmp_path / 'pA/p2/p3/pyproject.toml').touch()
     (tmp_path / 'pX/p9').mkdir(parents=True)
     (tmp_path / 'pX/p9/pyproject.toml').touch()
 
-    assert cli.find_black_config([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) is None
-    assert cli.find_black_config([tmp_path / 'pA/p2/p3']) is None
-    assert cli.find_black_config([tmp_path]) is None
-    assert cli.find_black_config([]) is None
+    assert cli.find_pyproject_toml([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) is None
+    assert cli.find_pyproject_toml([tmp_path / 'pA/p2/p3'])
+    assert cli.has_black_config(tmp_path / 'pA/p2/p3/pyproject.toml') is False
+    assert cli.find_pyproject_toml([tmp_path]) is None
+    assert cli.find_pyproject_toml([]) is None
 
     (tmp_path / 'pX/p9/pyproject.toml').write_text('[tool.black]')
-    assert cli.find_black_config([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) is None
-    assert cli.find_black_config([tmp_path / 'pX/p9']) == tmp_path / 'pX/p9/pyproject.toml'
+    assert cli.find_pyproject_toml([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) is None
+    assert cli.find_pyproject_toml([tmp_path / 'pX/p9']) == tmp_path / 'pX/p9/pyproject.toml'
+    assert cli.has_black_config(tmp_path / 'pX/p9/pyproject.toml') is True
 
     root_toml = tmp_path / 'pyproject.toml'
     (root_toml).write_text('[tool.black]')
-    assert cli.find_black_config([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) == root_toml
+    assert cli.find_pyproject_toml([tmp_path / 'pA/p2/p3/foo.py', tmp_path / 'pX/p9']) == root_toml
 
     monkeypatch.chdir(str(tmp_path / 'pA/p2'))
-    assert cli.find_black_config(['.']) == root_toml
+    assert cli.find_pyproject_toml(['.']) == root_toml
 
 
 def test_black_integration(tmp_path, sort_cfg_to_tmpdir):
@@ -735,3 +737,49 @@ def test_black_operates_on_chunks(tmp_path, mocker, sort_cfg_to_tmpdir):
     ])
     call_list = subprocess.call.call_args_list
     assert len(call_list) == 6  # 521 files in batches of 100
+
+
+def test_exclude_patterns(tmp_path, monkeypatch):
+    config_content = '''[tool.esss_fix_format]
+    exclude = [
+        "src/drafts/*.py",
+        "tmp/*",
+    ]
+    '''
+
+    config_file = tmp_path / 'pyproject.toml'
+    config_file.write_text(config_content)
+    include_patterns = ['*.cpp', '*.py']
+    exclude_patterns = cli.read_exclude_patterns(config_file)
+    assert not cli.should_format('src/drafts/foo.py', include_patterns, exclude_patterns)[0]
+    assert cli.should_format('src/drafts/foo.cpp', include_patterns, exclude_patterns)[0]
+    assert not cli.should_format('tmp/foo.cpp', include_patterns, exclude_patterns)[0]
+    assert cli.should_format('src/python/foo.py', include_patterns, exclude_patterns)[0]
+
+
+def test_invalid_exclude_patterns(tmp_path):
+    config_content = '''[tool.esss_fix_format]
+    exclude = "src/drafts/*.py"
+    '''
+
+    config_file = tmp_path / 'pyproject.toml'
+    config_file.write_text(config_content)
+    pytest.raises(TypeError, cli.read_exclude_patterns, config_file)
+
+
+def test_exclude_patterns_relative_path_fix(tmp_path, monkeypatch):
+    config_content = '''[tool.esss_fix_format]
+    exclude = [
+        "src/drafts/*.py",
+        "tmp/*",
+    ]
+    '''
+
+    config_file = tmp_path / 'pyproject.toml'
+    run_dir = tmp_path / 'src'
+    run_dir.mkdir()
+    config_file.write_text(config_content)
+    monkeypatch.chdir(run_dir)
+    include_patterns = ['*.py']
+    exclude_patterns = cli.read_exclude_patterns(config_file)
+    assert not cli.should_format('drafts/foo.py', include_patterns, exclude_patterns)[0]