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
--- a/python/mozlint/mozlint/pathutils.py
+++ b/python/mozlint/mozlint/pathutils.py
@@ -21,16 +21,20 @@ class FilterPath(object):
def finder(self):
if self._finder:
return self._finder
self._finder = FileFinder(
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)
@property
def isfile(self):
return os.path.isfile(self.path)
@property
@@ -88,19 +92,27 @@ def filterpaths(paths, linter, **lintarg
# Paths with and without globs will be handled separately,
# pull them apart now.
includepaths = [p for p in include if p.exists]
excludepaths = [p for p in exclude if p.exists]
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:
# Only excludes that are subdirectories of the include
# path matter.
excs = [e for e in excludepaths if inc.contains(e)]
if path.contains(inc):
@@ -124,22 +136,21 @@ def filterpaths(paths, linter, **lintarg
# 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):
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))
# 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]
new file mode 100644
--- /dev/null
+++ b/python/mozlint/test/files/foobar.py
@@ -0,0 +1,2 @@
+# Oh no.. we called this variable foobar, bad!
+foobar = "a string"
--- a/python/mozlint/test/linters/external.lint
+++ b/python/mozlint/test/linters/external.lint
@@ -17,14 +17,14 @@ def lint(files, **lintargs):
LINTER, path=path, lineno=i+1, column=1, rule="no-foobar"))
return results
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,
}
--- a/python/mozlint/test/test_roller.py
+++ b/python/mozlint/test/test_roller.py
@@ -15,18 +15,18 @@ from mozlint.errors import LintersNotCon
here = os.path.abspath(os.path.dirname(__file__))
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')
self.linters = [os.path.join(self.lintdir, n) for n in names]
def setUp(self):
TestCase.setUp(self)
self.lint = LintRoller()
@@ -65,11 +65,16 @@ class TestLintRoller(TestCase):
def test_roll_with_excluded_path(self):
self.lint.lintargs.update({'exclude': ['**/foobar.js']})
self.lint.read(self.linters)
result = self.lint.roll(self.files)
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()
--- a/python/mozlint/test/test_types.py
+++ b/python/mozlint/test/test_types.py
@@ -66,13 +66,13 @@ class TestLinterTypes(TestCase):
def test_no_filter(self):
self.lint.read(os.path.join(self.lintdir, 'explicit_path.lint'))
result = self.lint.roll(self.files)
self.assertEqual(len(result), 0)
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__':
main()
--- a/tools/lint/docs/create.rst
+++ b/tools/lint/docs/create.rst
@@ -48,16 +48,17 @@ Each ``.lint`` file must have a variable
linter. Here are the supported keys:
* name - The name of the linter (required)
* description - A brief description of the linter's purpose (required)
* type - One of 'string', 'regex' or 'external' (required)
* 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
following additional keys may be specified:
* message - A string to print on infraction (optional)
* hint - A string with a clue on how to fix the infraction (optional)
* rule - An id string for the lint rule (optional)
--- a/tools/lint/flake8.lint
+++ b/tools/lint/flake8.lint
@@ -40,16 +40,17 @@ LINE_OFFSETS = {
'E302': (-2, 3),
}
"""Maps a flake8 error to a lineoffset tuple.
The offset is of the form (lineno_offset, num_lines) and is passed
to the lineoffset property of `ResultContainer`.
"""
+EXTENSIONS = ['.py', '.lint']
results = []
def process_line(line):
try:
res = json.loads(line)
except ValueError:
return
@@ -130,11 +131,12 @@ LINTER = {
'taskcluster',
'testing/firefox-ui',
'testing/marionette/client',
'testing/puppeteer',
'testing/talos/',
'tools/lint',
],
'exclude': [],
+ 'extensions': EXTENSIONS,
'type': 'external',
'payload': lint,
}