зеркало из https://github.com/mozilla/gecko-dev.git
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 --HG-- extra : rebase_source : acceb0b129fc472fb456ff527e4c8c52228edd59
This commit is contained in:
Родитель
88729f16df
Коммит
e04895a098
|
@ -4,8 +4,6 @@
|
||||||
|
|
||||||
from __future__ import absolute_import
|
from __future__ import absolute_import
|
||||||
|
|
||||||
import os
|
|
||||||
|
|
||||||
|
|
||||||
class LintException(Exception):
|
class LintException(Exception):
|
||||||
pass
|
pass
|
||||||
|
@ -18,7 +16,7 @@ class LinterNotFound(LintException):
|
||||||
|
|
||||||
class LinterParseError(LintException):
|
class LinterParseError(LintException):
|
||||||
def __init__(self, path, message):
|
def __init__(self, path, message):
|
||||||
LintException.__init__(self, "{}: {}".format(os.path.basename(path), message))
|
LintException.__init__(self, "{}: {}".format(path, message))
|
||||||
|
|
||||||
|
|
||||||
class LintersNotConfigured(LintException):
|
class LintersNotConfigured(LintException):
|
||||||
|
|
|
@ -21,33 +21,57 @@ class Parser(object):
|
||||||
'payload',
|
'payload',
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def __init__(self, root):
|
||||||
|
self.root = root
|
||||||
|
|
||||||
def __call__(self, path):
|
def __call__(self, path):
|
||||||
return self.parse(path)
|
return self.parse(path)
|
||||||
|
|
||||||
def _validate(self, linter):
|
def _validate(self, linter):
|
||||||
|
relpath = os.path.relpath(linter['path'], self.root)
|
||||||
|
|
||||||
missing_attrs = []
|
missing_attrs = []
|
||||||
for attr in self.required_attributes:
|
for attr in self.required_attributes:
|
||||||
if attr not in linter:
|
if attr not in linter:
|
||||||
missing_attrs.append(attr)
|
missing_attrs.append(attr)
|
||||||
|
|
||||||
if missing_attrs:
|
if missing_attrs:
|
||||||
raise LinterParseError(linter['path'], "Missing required attribute(s): "
|
raise LinterParseError(relpath, "Missing required attribute(s): "
|
||||||
"{}".format(','.join(missing_attrs)))
|
"{}".format(','.join(missing_attrs)))
|
||||||
|
|
||||||
if linter['type'] not in supported_types:
|
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'):
|
for attr in ('include', 'exclude'):
|
||||||
if attr in linter and (not isinstance(linter[attr], list) or
|
if attr not in linter:
|
||||||
not all(isinstance(a, basestring) for a in linter[attr])):
|
continue
|
||||||
raise LinterParseError(linter['path'], "The {} directive must be a "
|
|
||||||
"list of strings!".format(attr))
|
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 'setup' in linter:
|
||||||
if linter['setup'].count(':') != 1:
|
if linter['setup'].count(':') != 1:
|
||||||
raise LinterParseError(linter['path'], "The setup attribute '{!r}' must have the "
|
raise LinterParseError(relpath, "The setup attribute '{!r}' must have the "
|
||||||
"form 'module:object'".format(
|
"form 'module:object'".format(
|
||||||
linter['setup']))
|
linter['setup']))
|
||||||
|
|
||||||
if 'extensions' in linter:
|
if 'extensions' in linter:
|
||||||
linter['extensions'] = [e.strip('.') for e in linter['extensions']]
|
linter['extensions'] = [e.strip('.') for e in linter['extensions']]
|
||||||
|
|
|
@ -90,7 +90,7 @@ class LintRoller(object):
|
||||||
MAX_PATHS_PER_JOB = 50 # set a max size to prevent command lines that are too long on Windows
|
MAX_PATHS_PER_JOB = 50 # set a max size to prevent command lines that are too long on Windows
|
||||||
|
|
||||||
def __init__(self, root, **lintargs):
|
def __init__(self, root, **lintargs):
|
||||||
self.parse = Parser()
|
self.parse = Parser(root)
|
||||||
try:
|
try:
|
||||||
self.vcs = get_repository_object(root)
|
self.vcs = get_repository_object(root)
|
||||||
except InvalidRepoPath:
|
except InvalidRepoPath:
|
||||||
|
|
|
@ -3,6 +3,6 @@ ExplicitPathLinter:
|
||||||
description: Only lint a specific file name
|
description: Only lint a specific file name
|
||||||
rule: no-foobar
|
rule: no-foobar
|
||||||
include:
|
include:
|
||||||
- no_foobar.js
|
- files/no_foobar.js
|
||||||
type: string
|
type: string
|
||||||
payload: foobar
|
payload: foobar
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
BadExcludeLinter:
|
||||||
|
description: Has an invalid exclude directive.
|
||||||
|
exclude:
|
||||||
|
- files/does_not_exist
|
||||||
|
type: string
|
||||||
|
payload: foobar
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
BadIncludeLinter:
|
||||||
|
description: Has an invalid include directive.
|
||||||
|
include:
|
||||||
|
- files/does_not_exist
|
||||||
|
type: string
|
||||||
|
payload: foobar
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
BadSupportFilesLinter:
|
||||||
|
description: Has an invalid support-files directive.
|
||||||
|
support-files:
|
||||||
|
- files/does_not_exist
|
||||||
|
type: string
|
||||||
|
payload: foobar
|
|
@ -5,8 +5,8 @@
|
||||||
from __future__ import absolute_import
|
from __future__ import absolute_import
|
||||||
|
|
||||||
import os
|
import os
|
||||||
import sys
|
|
||||||
|
|
||||||
|
import mozunit
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from mozlint import pathutils
|
from mozlint import pathutils
|
||||||
|
@ -90,4 +90,4 @@ def test_include_exclude(filterpaths):
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
sys.exit(pytest.main(['--verbose', __file__]))
|
mozunit.main()
|
||||||
|
|
|
@ -15,10 +15,12 @@ from mozlint.errors import (
|
||||||
LinterParseError,
|
LinterParseError,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
here = os.path.abspath(os.path.dirname(__file__))
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(scope='module')
|
@pytest.fixture(scope='module')
|
||||||
def parse(lintdir):
|
def parse(lintdir):
|
||||||
parser = Parser()
|
parser = Parser(here)
|
||||||
|
|
||||||
def _parse(name):
|
def _parse(name):
|
||||||
path = os.path.join(lintdir, name)
|
path = os.path.join(lintdir, name)
|
||||||
|
@ -48,6 +50,9 @@ def test_parse_valid_linter(parse):
|
||||||
'invalid_exclude.yml',
|
'invalid_exclude.yml',
|
||||||
'missing_attrs.yml',
|
'missing_attrs.yml',
|
||||||
'missing_definition.yml',
|
'missing_definition.yml',
|
||||||
|
'non_existing_include.yml',
|
||||||
|
'non_existing_exclude.yml',
|
||||||
|
'non_existing_support_files.yml',
|
||||||
])
|
])
|
||||||
def test_parse_invalid_linter(parse, linter):
|
def test_parse_invalid_linter(parse, linter):
|
||||||
with pytest.raises(LinterParseError):
|
with pytest.raises(LinterParseError):
|
||||||
|
|
|
@ -11,7 +11,6 @@ mingw-capitalization:
|
||||||
- gfx/angle/checkout/src/common/tls.cpp
|
- gfx/angle/checkout/src/common/tls.cpp
|
||||||
- gfx/cairo/cairo/src/cairo-atomic-private.h
|
- gfx/cairo/cairo/src/cairo-atomic-private.h
|
||||||
- gfx/harfbuzz/src/hb-directwrite.cc
|
- 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.cpp
|
||||||
- gfx/skia/skia/src/xps/SkXPSDevice.h
|
- gfx/skia/skia/src/xps/SkXPSDevice.h
|
||||||
- gfx/skia/skia/src/xps/SkXPSDocument.h
|
- gfx/skia/skia/src/xps/SkXPSDocument.h
|
||||||
|
|
|
@ -26,13 +26,11 @@ py2:
|
||||||
- netwerk
|
- netwerk
|
||||||
- nsprpub
|
- nsprpub
|
||||||
- other-licenses
|
- other-licenses
|
||||||
- probes/trace-gen.py
|
|
||||||
- python/devtools
|
- python/devtools
|
||||||
- python/mach
|
- python/mach
|
||||||
- python/mozbuild
|
- python/mozbuild
|
||||||
- python/mozversioncontrol
|
- python/mozversioncontrol
|
||||||
- security
|
- security
|
||||||
- services/common/tests/mach_commands.py
|
|
||||||
- servo
|
- servo
|
||||||
- taskcluster/docker/funsize-update-generator
|
- taskcluster/docker/funsize-update-generator
|
||||||
- testing/awsy
|
- testing/awsy
|
||||||
|
|
|
@ -26,7 +26,6 @@ py3:
|
||||||
- nsprpub
|
- nsprpub
|
||||||
- security/manager/ssl
|
- security/manager/ssl
|
||||||
- security/nss
|
- security/nss
|
||||||
- services/common/tests/mach_commands.py
|
|
||||||
- servo
|
- servo
|
||||||
- testing/awsy
|
- testing/awsy
|
||||||
- testing/firefox-ui/harness/firefox_ui_harness/runners/update.py
|
- testing/firefox-ui/harness/firefox_ui_harness/runners/update.py
|
||||||
|
|
|
@ -55,7 +55,7 @@ def config(request):
|
||||||
|
|
||||||
name = request.module.LINTER
|
name = request.module.LINTER
|
||||||
config_path = os.path.join(lintdir, '{}.yml'.format(name))
|
config_path = os.path.join(lintdir, '{}.yml'.format(name))
|
||||||
parser = Parser()
|
parser = Parser(build.topsrcdir)
|
||||||
# TODO Don't assume one linter per yaml file
|
# TODO Don't assume one linter per yaml file
|
||||||
return parser.parse(config_path)[0]
|
return parser.parse(config_path)[0]
|
||||||
|
|
||||||
|
|
Загрузка…
Ссылка в новой задаче