Bug 1392787 - [mozlint] Fix bugs in path filtering and add a test, r=jmaher
☠☠ backed out by c1fdb1c43538 ☠ ☠
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Wed, 23 Aug 2017 06:33:06 -0400
changeset 376653 bf5a6ff97a8f55899d11610b4046e7fbe6bd3bc6
parent 376652 834bbf6193e9d2a3a302f23fdcc461678a535187
child 376654 2ce8a387fa80ac5ba6781505f206dd8de4cd217f
push id49573
push userahalberstadt@mozilla.com
push dateThu, 24 Aug 2017 19:08:03 +0000
treeherderautoland@e774700fe070 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjmaher
bugs1392787
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1392787 - [mozlint] Fix bugs in path filtering and add a test, r=jmaher MozReview-Commit-ID: LG47ASBMA17
python/mozlint/mozlint/pathutils.py
python/mozlint/test/filter/a.js
python/mozlint/test/filter/a.py
python/mozlint/test/filter/subdir1/b.js
python/mozlint/test/filter/subdir1/b.py
python/mozlint/test/filter/subdir1/subdir3/d.js
python/mozlint/test/filter/subdir1/subdir3/d.py
python/mozlint/test/filter/subdir2/c.js
python/mozlint/test/filter/subdir2/c.py
python/mozlint/test/python.ini
python/mozlint/test/test_filterpaths.py
--- a/python/mozlint/mozlint/pathutils.py
+++ b/python/mozlint/mozlint/pathutils.py
@@ -37,17 +37,17 @@ class FilterPath(object):
     def isfile(self):
         return os.path.isfile(self.path)
 
     @property
     def isdir(self):
         return os.path.isdir(self.path)
 
     def join(self, *args):
-        return FilterPath(os.path.join(self, *args))
+        return FilterPath(os.path.join(self.path, *args))
 
     def match(self, patterns):
         return any(mozpath.match(self.path, pattern.path) for pattern in patterns)
 
     def contains(self, other):
         """Return True if other is a subdirectory of self or equals self."""
         if isinstance(other, FilterPath):
             other = other.path
@@ -77,17 +77,17 @@ def filterpaths(paths, linter, **lintarg
     exclude = lintargs.get('exclude', [])
     exclude.extend(linter.get('exclude', []))
     root = lintargs['root']
 
     if not lintargs.get('use_filters', True) or (not include and not exclude):
         return paths
 
     def normalize(path):
-        if not os.path.isabs(path):
+        if '*' not in path and not os.path.isabs(path):
             path = os.path.join(root, path)
         return FilterPath(path)
 
     include = map(normalize, include)
     exclude = map(normalize, exclude)
 
     # Paths with and without globs will be handled separately,
     # pull them apart now.
@@ -134,26 +134,28 @@ def filterpaths(paths, linter, **lintarg
                     keep.add(path)
 
         # Next handle include/exclude directives that
         # contain globs.
         if path.isfile:
             # If the specified path is a file it must be both
             # matched by an include directive and not matched
             # by an exclude directive.
-            if not path.match(includeglobs):
+            if not path.match(includeglobs) or any(e.contains(path) for e in excludepaths):
                 continue
 
             keep.add(path)
         elif path.isdir:
             # If the specified path is a directory, use a
             # FileFinder to resolve all relevant globs.
-            path.exclude = [e.path for e in excludeglobs]
+            path.exclude = [os.path.relpath(e.path, root) for e in exclude]
             for pattern in includeglobs:
                 for p, f in path.finder.find(pattern.path):
+                    if extensions and os.path.splitext(p)[1][1:] not in extensions:
+                        continue
                     keep.add(path.join(p))
 
     # Only pass paths we couldn't exclude here to the underlying linter
     lintargs['exclude'] = [f.path for f in discard]
     return [f.path for f in keep]
 
 
 def findobject(path):
new file mode 100644
new file mode 100644
new file mode 100644
new file mode 100644
new file mode 100644
new file mode 100644
new file mode 100644
new file mode 100644
--- a/python/mozlint/test/python.ini
+++ b/python/mozlint/test/python.ini
@@ -1,12 +1,13 @@
 [DEFAULT]
 subsuite = mozlint, os == "linux"
 
 [test_cli.py]
+[test_filterpaths.py]
 [test_formatters.py]
 [test_parser.py]
 [test_roller.py]
 [test_types.py]
 [test_vcs.py]
 # these tests run in the build images on non-linux, which have old
 # versions of mercurial and git installed
 skip-if = os != "linux"
new file mode 100644
--- /dev/null
+++ b/python/mozlint/test/test_filterpaths.py
@@ -0,0 +1,91 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import os
+import sys
+
+import pytest
+
+from mozlint import pathutils
+
+here = os.path.abspath(os.path.dirname(__file__))
+
+
+@pytest.fixture
+def filterpaths():
+    lintargs = {
+        'root': os.path.join(here, 'filter'),
+        'use_filters': True,
+    }
+    os.chdir(lintargs['root'])
+
+    def inner(paths, include, exclude, extensions=None, **kwargs):
+        linter = {
+            'include': include,
+            'exclude': exclude,
+            'extensions': extensions,
+        }
+        lintargs.update(kwargs)
+        return pathutils.filterpaths(paths, linter, **lintargs)
+
+    return inner
+
+
+def assert_paths(a, b):
+    assert set(a) == set([os.path.normpath(t) for t in b])
+
+
+def test_no_filter(filterpaths):
+    args = {
+        'paths': ['a.py', 'subdir1/b.py'],
+        'include': ['.'],
+        'exclude': ['**/*.py'],
+        'use_filters': False,
+    }
+
+    paths = filterpaths(**args)
+    assert set(paths) == set(args['paths'])
+
+
+def test_extensions(filterpaths):
+    args = {
+        'paths': ['a.py', 'a.js', 'subdir2'],
+        'include': ['**'],
+        'exclude': [],
+        'extensions': ['py'],
+    }
+
+    paths = filterpaths(**args)
+    assert_paths(paths, ['a.py', 'subdir2/c.py'])
+
+
+def test_include_exclude(filterpaths):
+    args = {
+        'paths': ['a.js', 'subdir1/subdir3/d.js'],
+        'include': ['.'],
+        'exclude': ['subdir1'],
+    }
+
+    paths = filterpaths(**args)
+    assert_paths(paths, ['a.js'])
+
+    args['include'] = ['subdir1/subdir3']
+    paths = filterpaths(**args)
+    assert_paths(paths, ['subdir1/subdir3/d.js'])
+
+    args = {
+        'paths': ['.'],
+        'include': ['**/*.py'],
+        'exclude': ['**/c.py', 'subdir1/subdir3'],
+    }
+    paths = filterpaths(**args)
+    assert_paths(paths, ['a.py', 'subdir1/b.py'])
+
+    args['paths'] = ['a.py', 'a.js', 'subdir1/b.py', 'subdir2/c.py', 'subdir1/subdir3/d.py']
+    paths = filterpaths(**args)
+    assert_paths(paths, ['a.py', 'subdir1/b.py'])
+
+
+if __name__ == '__main__':
+    sys.exit(pytest.main(['--verbose', __file__]))