Bug 1392787 - [mozlint] Fix bugs in path filtering and add a test, r?jmaher draft
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Wed, 23 Aug 2017 06:33:06 -0400
changeset 665467 5d69b8be96dd724a5a8d90cfc623cfdf1c1e590b
parent 664840 144b440b6c05a3d14f5339d56af1fe2ce98d856b
child 665468 65793a1571204d5b049f96c9307a8480efaf2697
push id80069
push userahalberstadt@mozilla.com
push dateFri, 15 Sep 2017 13:29:20 +0000
reviewersjmaher
bugs1392787
milestone57.0a1
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
@@ -17,17 +17,18 @@ class FilterPath(object):
         self._finder = None
         self.exclude = exclude
 
     @property
     def finder(self):
         if self._finder:
             return self._finder
         self._finder = FileFinder(
-            self.path, ignore=self.exclude)
+            mozpath.normsep(self.path),
+            ignore=[mozpath.normsep(e) for e in self.exclude])
         return self._finder
 
     @property
     def ext(self):
         return os.path.splitext(self.path)[1].strip('.')
 
     @property
     def exists(self):
@@ -37,20 +38,27 @@ 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)
+        a = mozpath.normsep(self.path)
+        for p in patterns:
+            if isinstance(p, FilterPath):
+                p = p.path
+            p = mozpath.normsep(p)
+            if mozpath.match(a, p):
+                return True
+        return False
 
     def contains(self, other):
         """Return True if other is a subdirectory of self or equals self."""
         if isinstance(other, FilterPath):
             other = other.path
         a = os.path.abspath(self.path)
         b = os.path.normpath(os.path.abspath(other))
 
@@ -77,17 +85,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 +142,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,93 @@
+# 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/.
+
+from __future__ import absolute_import
+
+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__]))