Bug 1288425 - Make sure we skip invalid extensions when linting with --rev or --workdir, r=smacleod

Some linters, such as flake8, will lint invalid file extensions if you explicitly pass them in. E.g,
|flake8 foobar.js| will result in flake8 attempting to lint a JS file. This is a problem because passing
in files explicitly is exactly what the --rev/--workdir options do. If a developer modifies a JS file
then runs |mach lint -l flake8 -w|, that JS file will get linted.

To prevent this, mozlint needs to handle file extensions instead of relying on the underlying linter to
do it. This patch adds an "extensions" config option to the LINTER dict, and will filter these files out
as part of the 'filterpaths' steps.

MozReview-Commit-ID: KYhC6SEySC3

--HG--
extra : rebase_source : 6fea2942b2db1bea7deca1d6738546362b6ebd65

python/mozlint/mozlint/pathutils.py

@@ -25,6 +25,10 @@ class FilterPath(object):
             self.path, find_executables=False, ignore=self.exclude)
         return self._finder
 
+    @property
+    def ext(self):
+        return os.path.splitext(self.path)[1]
+
     @property
     def exists(self):
         return os.path.exists(self.path)
@@ -93,9 +97,17 @@ def filterpaths(paths, linter, **lintargs):
     includeglobs = [p for p in include if not p.exists]
     excludeglobs = [p for p in exclude if not p.exists]
 
+    extensions = linter.get('extensions')
     keep = set()
     discard = set()
     for path in map(FilterPath, paths):
+        # Exclude bad file extensions
+        if extensions and path.isfile and path.ext not in extensions:
+            continue
+
+        if path.match(excludeglobs):
+            continue
+
         # First handle include/exclude directives
         # that exist (i.e don't have globs)
         for inc in includepaths:
@@ -129,13 +141,12 @@ def filterpaths(paths, linter, **lintargs):
             # by an exclude directive.
             if not path.match(includeglobs):
                 continue
-            elif path.match(excludeglobs):
-                continue
+
             keep.add(path)
         elif path.isdir:
             # If the specified path is a directory, use a
             # FileFinder to resolve all relevant globs.
-            path.exclude = excludeglobs
+            path.exclude = [e.path for e in excludeglobs]
             for pattern in includeglobs:
                 for p, f in path.finder.find(pattern.path):
                     keep.add(path.join(p))

python/mozlint/test/files/foobar.py

@@ -0,0 +1,2 @@
+# Oh no.. we called this variable foobar, bad!
+foobar = "a string"

python/mozlint/test/linters/external.lint

@@ -22,9 +22,9 @@ LINTER = {
     'name': "ExternalLinter",
     'description': "It's bad to have the string foobar in js files.",
     'include': [
-        '**/*.js',
-        '**/*.jsm',
+        'python/mozlint/test/files',
     ],
     'type': 'external',
+    'extensions': ['.js', '.jsm'],
     'payload': lint,
 }

python/mozlint/test/test_roller.py

@@ -20,8 +20,8 @@ class TestLintRoller(TestCase):
     def __init__(self, *args, **kwargs):
         TestCase.__init__(self, *args, **kwargs)
 
-        filedir = os.path.join(here, 'files')
-        self.files = [os.path.join(filedir, f) for f in os.listdir(filedir)]
+        self.filedir = os.path.join(here, 'files')
+        self.files = [os.path.join(self.filedir, f) for f in os.listdir(self.filedir)]
         self.lintdir = os.path.join(here, 'linters')
 
         names = ('string.lint', 'regex.lint', 'external.lint')
@@ -70,6 +70,11 @@ class TestLintRoller(TestCase):
 
         self.assertEqual(len(result), 0)
 
+    def test_roll_with_invalid_extension(self):
+        self.lint.read(os.path.join(self.lintdir, 'external.lint'))
+        result = self.lint.roll(os.path.join(self.filedir, 'foobar.py'))
+        self.assertEqual(len(result), 0)
+
 
 if __name__ == '__main__':
     main()

python/mozlint/test/test_types.py

@@ -71,7 +71,7 @@ class TestLinterTypes(TestCase):
 
         self.lint.lintargs['use_filters'] = False
         result = self.lint.roll(self.files)
-        self.assertEqual(len(result), 1)
+        self.assertEqual(len(result), 2)
 
 
 if __name__ == '__main__':

tools/lint/docs/create.rst

@@ -53,6 +53,7 @@ linter. Here are the supported keys:
 * payload - The actual linting logic, depends on the type (required)
 * include - A list of glob patterns that must be matched (optional)
 * exclude - A list of glob patterns that must not be matched (optional)
+* extensions - A list of file extensions to be considered (optional)
 * setup - A function that sets up external dependencies (optional)
 
 In addition to the above, some ``.lint`` files correspond to a single lint rule. For these, the

tools/lint/flake8.lint

@@ -45,6 +45,7 @@ The offset is of the form (lineno_offset, num_lines) and is passed
 to the lineoffset property of `ResultContainer`.
 """
 
+EXTENSIONS = ['.py', '.lint']
 results = []
 
 
@@ -135,6 +136,7 @@ LINTER = {
         'tools/lint',
     ],
     'exclude': [],
+    'extensions': EXTENSIONS,
     'type': 'external',
     'payload': lint,
 }