Bug 1373368 - [mozlint] Raise if a non-existant path is specified in a lint config, r?Standard8
Since I left the next two patches to bitrot, I realized that a path I had added
didn't exist anymore. We should definitely error out if non-existant paths are
specified, otherwise the lists will become outdated and it will be possible to
accidentally disable linting on some files.
I discovered a few instances of this already in our existing definitions.
MozReview-Commit-ID: 8jsTKLI0nFE
--- a/python/mozlint/mozlint/errors.py
+++ b/python/mozlint/mozlint/errors.py
@@ -1,27 +1,25 @@
# 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
-
class LintException(Exception):
pass
class LinterNotFound(LintException):
def __init__(self, path):
LintException.__init__(self, "Could not find lint file '{}'".format(path))
class LinterParseError(LintException):
def __init__(self, path, message):
- LintException.__init__(self, "{}: {}".format(os.path.basename(path), message))
+ LintException.__init__(self, "{}: {}".format(path, message))
class LintersNotConfigured(LintException):
def __init__(self):
LintException.__init__(self, "No linters registered! Use `LintRoller.read` "
"to register a linter.")
--- a/python/mozlint/mozlint/parser.py
+++ b/python/mozlint/mozlint/parser.py
@@ -16,43 +16,67 @@ class Parser(object):
"""Reads and validates lint configuration files."""
required_attributes = (
'name',
'description',
'type',
'payload',
)
+ def __init__(self, root):
+ self.root = root
+
def __call__(self, path):
return self.parse(path)
def _validate(self, linter):
+ relpath = os.path.relpath(linter['path'], self.root)
+
missing_attrs = []
for attr in self.required_attributes:
if attr not in linter:
missing_attrs.append(attr)
if missing_attrs:
- raise LinterParseError(linter['path'], "Missing required attribute(s): "
- "{}".format(','.join(missing_attrs)))
+ raise LinterParseError(relpath, "Missing required attribute(s): "
+ "{}".format(','.join(missing_attrs)))
if linter['type'] not in supported_types:
- raise LinterParseError(linter['path'], "Invalid type '{}'".format(linter['type']))
+ raise LinterParseError(relpath, "Invalid type '{}'".format(linter['type']))
for attr in ('include', 'exclude'):
- if attr in linter and (not isinstance(linter[attr], list) or
- not all(isinstance(a, basestring) for a in linter[attr])):
- raise LinterParseError(linter['path'], "The {} directive must be a "
- "list of strings!".format(attr))
+ if attr not in linter:
+ continue
+
+ if not isinstance(linter[attr], list) or \
+ not all(isinstance(a, basestring) for a in linter[attr]):
+ raise LinterParseError(relpath, "The {} directive must be a "
+ "list of strings!".format(attr))
+ invalid_paths = set()
+ for path in linter[attr]:
+ if '*' in path:
+ continue
+
+ abspath = path
+ if not os.path.isabs(abspath):
+ abspath = os.path.join(self.root, path)
+
+ if not os.path.exists(abspath):
+ invalid_paths.add(' ' + path)
+
+ if invalid_paths:
+ raise LinterParseError(relpath, "The {} directive contains the following "
+ "paths that don't exist:\n{}".format(
+ attr, '\n'.join(sorted(invalid_paths))))
if 'setup' in linter:
if linter['setup'].count(':') != 1:
- raise LinterParseError(linter['path'], "The setup attribute '{!r}' must have the "
- "form 'module:object'".format(
- linter['setup']))
+ raise LinterParseError(relpath, "The setup attribute '{!r}' must have the "
+ "form 'module:object'".format(
+ linter['setup']))
if 'extensions' in linter:
linter['extensions'] = [e.strip('.') for e in linter['extensions']]
def parse(self, path):
"""Read a linter and return its LINTER definition.
:param path: Path to the linter.
--- a/python/mozlint/mozlint/roller.py
+++ b/python/mozlint/mozlint/roller.py
@@ -85,17 +85,17 @@ class LintRoller(object):
:param root: Path to which relative paths will be joined. If
unspecified, root will either be determined from
version control or cwd.
:param lintargs: Arguments to pass to the underlying linter(s).
"""
MAX_PATHS_PER_JOB = 50 # set a max size to prevent command lines that are too long on Windows
def __init__(self, root, **lintargs):
- self.parse = Parser()
+ self.parse = Parser(root)
try:
self.vcs = get_repository_object(root)
except InvalidRepoPath:
self.vcs = None
self.linters = []
self.lintargs = lintargs
self.lintargs['root'] = root
--- a/python/mozlint/test/linters/explicit_path.yml
+++ b/python/mozlint/test/linters/explicit_path.yml
@@ -1,8 +1,8 @@
---
ExplicitPathLinter:
description: Only lint a specific file name
rule: no-foobar
include:
- - no_foobar.js
+ - files/no_foobar.js
type: string
payload: foobar
new file mode 100644
--- /dev/null
+++ b/python/mozlint/test/linters/non_existing_exclude.yml
@@ -0,0 +1,7 @@
+---
+BadExcludeLinter:
+ description: Has an invalid exclude directive.
+ exclude:
+ - files/does_not_exist
+ type: string
+ payload: foobar
new file mode 100644
--- /dev/null
+++ b/python/mozlint/test/linters/non_existing_include.yml
@@ -0,0 +1,7 @@
+---
+BadIncludeLinter:
+ description: Has an invalid include directive.
+ include:
+ - files/does_not_exist
+ type: string
+ payload: foobar
new file mode 100644
--- /dev/null
+++ b/python/mozlint/test/linters/non_existing_support_files.yml
@@ -0,0 +1,7 @@
+---
+BadSupportFilesLinter:
+ description: Has an invalid support-files directive.
+ support-files:
+ - files/does_not_exist
+ type: string
+ payload: foobar
--- a/python/mozlint/test/test_filterpaths.py
+++ b/python/mozlint/test/test_filterpaths.py
@@ -1,17 +1,17 @@
# 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 mozunit
import pytest
from mozlint import pathutils
here = os.path.abspath(os.path.dirname(__file__))
@pytest.fixture
@@ -85,9 +85,9 @@ def test_include_exclude(filterpaths):
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__]))
+ mozunit.main()
--- a/python/mozlint/test/test_parser.py
+++ b/python/mozlint/test/test_parser.py
@@ -10,20 +10,22 @@ import mozunit
import pytest
from mozlint.parser import Parser
from mozlint.errors import (
LinterNotFound,
LinterParseError,
)
+here = os.path.abspath(os.path.dirname(__file__))
+
@pytest.fixture(scope='module')
def parse(lintdir):
- parser = Parser()
+ parser = Parser(here)
def _parse(name):
path = os.path.join(lintdir, name)
return parser(path)
return _parse
def test_parse_valid_linter(parse):
@@ -43,16 +45,19 @@ def test_parse_valid_linter(parse):
@pytest.mark.parametrize('linter', [
'invalid_type.yml',
'invalid_extension.ym',
'invalid_include.yml',
'invalid_exclude.yml',
'missing_attrs.yml',
'missing_definition.yml',
+ 'non_existing_include.yml',
+ 'non_existing_exclude.yml',
+ 'non_existing_support_files.yml',
])
def test_parse_invalid_linter(parse, linter):
with pytest.raises(LinterParseError):
parse(linter)
def test_parse_non_existent_linter(parse):
with pytest.raises(LinterNotFound):
--- a/tools/lint/mingw-capitalization.yml
+++ b/tools/lint/mingw-capitalization.yml
@@ -6,17 +6,16 @@ mingw-capitalization:
include: ['.']
exclude:
# We do not compile WebRTC with MinGW yet
- media/webrtc
- media/mtransport/third_party/nrappkit/src/util/util.c
- gfx/angle/checkout/src/common/tls.cpp
- gfx/cairo/cairo/src/cairo-atomic-private.h
- gfx/harfbuzz/src/hb-directwrite.cc
- - gfx/skia/skia/src/views/win/SkOSWindow_win.cpp
- gfx/skia/skia/src/xps/SkXPSDevice.cpp
- gfx/skia/skia/src/xps/SkXPSDevice.h
- gfx/skia/skia/src/xps/SkXPSDocument.h
- ipc/chromium/src/third_party/libevent
- modules/freetype2/builds/mac/ftmac.c
- other-licenses/7zstub
- security/nss/nss-tool/common/util.cc
- toolkit/crashreporter/google-breakpad/src/tools/windows
--- a/tools/lint/py2.yml
+++ b/tools/lint/py2.yml
@@ -21,23 +21,21 @@ py2:
- media
- memory
- mobile
- modules
- mozglue
- netwerk
- nsprpub
- other-licenses
- - probes/trace-gen.py
- python/devtools
- python/mach
- python/mozbuild
- python/mozversioncontrol
- security
- - services/common/tests/mach_commands.py
- servo
- taskcluster/docker/funsize-update-generator
- testing/awsy
- testing/firefox-ui
- testing/geckodriver
- testing/gtest
- testing/mochitest
- testing/mozharness
--- a/tools/lint/py3.yml
+++ b/tools/lint/py3.yml
@@ -21,17 +21,16 @@ py3:
- layout/style
- layout/tools/reftest
- media
- memory/replace
- modules/freetype2
- nsprpub
- security/manager/ssl
- security/nss
- - services/common/tests/mach_commands.py
- servo
- testing/awsy
- testing/firefox-ui/harness/firefox_ui_harness/runners/update.py
- testing/gtest
- testing/mochitest
- testing/mozharness
- testing/tools/iceserver
- testing/tps
--- a/tools/lint/test/conftest.py
+++ b/tools/lint/test/conftest.py
@@ -50,17 +50,17 @@ def config(request):
used to test a single linter. If no LINTER variable is defined, the test
will fail.
"""
if not hasattr(request.module, 'LINTER'):
pytest.fail("'config' fixture used from a module that didn't set the LINTER variable")
name = request.module.LINTER
config_path = os.path.join(lintdir, '{}.yml'.format(name))
- parser = Parser()
+ parser = Parser(build.topsrcdir)
# TODO Don't assume one linter per yaml file
return parser.parse(config_path)[0]
@pytest.fixture(scope='module', autouse=True)
def run_setup(config):
"""Make sure that if the linter named in the LINTER global variable has a
setup function, it gets called before running the tests.