Bug 1367092 - [flake8] Take exclusion handling away from flake8, r=egao

The motivations for this are:

1) Apply global excludes. This merges the exclusion rules defined in
tools/lint/mach_commands.py with the ones in .flake8.

2) Improve performance. Switching to a blacklist will result in a much longer
runtime for linting the entire tree and flake8 handles exclusions incredibly
slowly. Without this patch (and the blacklist change applied), I gave up
waiting after 30 minutes. With this patch, it takes 30 seconds.

Depends on D20495

Differential Revision: https://phabricator.services.mozilla.com/D20496

--HG--
rename : tools/lint/test/files/flake8/bad.py => tools/lint/test/files/flake8/subdir/exclude/bad.py
extra : moz-landing-system : lando

python/mozlint/mozlint/pathutils.py

@@ -124,7 +124,7 @@ def collapse(paths, base=None, dotfiles=False):
         # Every file under this base was covered, so we can collapse them all
         # up into the base path.
         return [base]
-    return covered
+    return list(covered)
 
 
 def filterpaths(root, paths, include, exclude=None, extensions=None):

tools/lint/python/flake8.py

@@ -13,6 +13,7 @@ import sys
 import mozfile
 
 from mozlint import result
+from mozlint.pathutils import expand_exclusions
 from mozlint.util import pip
 
 here = os.path.abspath(os.path.dirname(__file__))
@@ -63,6 +64,12 @@ else:
     bindir = os.path.join(sys.prefix, 'bin')
 
 
+class NothingToLint(Exception):
+    """Exception used to bail out of flake8's internals if all the specified
+    files were excluded.
+    """
+
+
 def setup(root):
     if not pip.reinstall_program(FLAKE8_REQUIREMENTS_PATH):
         print(FLAKE8_INSTALL_ERROR)
@@ -72,8 +79,8 @@ def setup(root):
 def lint(paths, config, **lintargs):
     from flake8.main.application import Application
 
-    config_path = os.path.join(lintargs['root'], '.flake8')
+    root = lintargs['root']
-    exclude = config.get('exclude', [])
+    config_path = os.path.join(root, '.flake8')
 
     if lintargs.get('fix'):
         fix_cmd = [
@@ -82,25 +89,56 @@ def lint(paths, config, **lintargs):
             '--in-place', '--recursive',
         ]
 
-        if exclude:
+        if config.get('exclude'):
-            fix_cmd.extend(['--exclude', ','.join(exclude)])
+            fix_cmd.extend(['--exclude', ','.join(config['exclude'])])
 
         subprocess.call(fix_cmd + paths)
 
+    # Run flake8.
+    app = Application()
+
     output_file = mozfile.NamedTemporaryFile()
     flake8_cmd = [
-        os.path.join(bindir, 'flake8'),
         '--config', config_path,
         '--output-file', output_file.name,
         '--format', '{"path":"%(path)s","lineno":%(row)s,'
                     '"column":%(col)s,"rule":"%(code)s","message":"%(text)s"}',
         '--filename', ','.join(['*.{}'.format(e) for e in config['extensions']]),
-    ] + paths
+    ]
+
+    orig_make_file_checker_manager = app.make_file_checker_manager
+
+    def wrap_make_file_checker_manager(self):
+        """Flake8 is very inefficient when it comes to applying exclusion
+        rules, using `expand_exclusions` to turn directories into a list of
+        relevant python files is an order of magnitude faster.
+
+        Hooking into flake8 here also gives us a convenient place to merge the
+        `exclude` rules specified in the root .flake8 with the ones added by
+        tools/lint/mach_commands.py.
+        """
+        config.setdefault('exclude', []).extend(self.options.exclude)
+        self.options.exclude = None
+        self.args = self.args + list(expand_exclusions(paths, config, root))
+
+        if not self.args:
+            raise NothingToLint
+        return orig_make_file_checker_manager()
+
+    app.make_file_checker_manager = wrap_make_file_checker_manager.__get__(app, Application)
+
+    # Make sure to run from repository root so exlusions are joined to the
+    # repository root and not the current working directory.
+    oldcwd = os.getcwd()
+    os.chdir(root)
+    try:
+        app.run(flake8_cmd)
+    except NothingToLint:
+        pass
+    finally:
+        os.chdir(oldcwd)
 
-    # Run flake8.
     results = []
-    app = Application()
-    app.run(flake8_cmd)
 
     def process_line(line):
         # Escape slashes otherwise JSON conversion will not work

tools/lint/test/files/flake8/.flake8

@@ -1,2 +1,4 @@
 [flake8]
 max-line-length = 100
+exclude =
+    subdir/exclude,

tools/lint/test/files/flake8/subdir/exclude/bad.py

@@ -0,0 +1,5 @@
+# Unused import
+import distutils
+
+print("This is a line that is over 80 characters but under 100. It shouldn't fail.")
+print("This is a line that is over not only 80, but 100 characters. It should most certainly cause a failure.")

tools/lint/test/test_flake8.py

@@ -1,7 +1,6 @@
 import os
 
 import mozunit
-import pytest
 
 LINTER = 'flake8'
 
@@ -64,16 +63,19 @@ foo = ['A list of strings', 'that go over 80 characters', 'to test if autopep8 f
         assert fh.read() == contents
 
 
-@pytest.mark.xfail(reason="Bug 1277851 - custom configs are ignored if specifying a parent path")
+def test_lint_excluded_file(lint, paths, config):
-def test_lint_custom_config_from_parent_path(lint, paths):
+    # First file is globally excluded, second one is from .flake8 config.
-    results = lint(paths(), collapse_results=True)
+    files = paths('bad.py', 'subdir/exclude/bad.py')
-    assert paths('custom/good.py')[0] not in results
+    config['exclude'] = paths('bad.py')
+    results = lint(files, config)
+    print(results)
+    assert len(results) == 0
 
+    # Make sure excludes also apply when running from a different cwd.
+    cwd = paths('subdir')[0]
+    os.chdir(cwd)
 
-@pytest.mark.xfail(reason="Bug 1277851 - 'exclude' argument is ignored")
+    results = lint(paths('subdir/exclude'))
-def test_lint_excluded_file(lint, paths):
-    paths = paths('bad.py')
-    results = lint(paths, exclude=paths)
     assert len(results) == 0