diff --git a/python/mozlint/mozlint/parser.py b/python/mozlint/mozlint/parser.py index bd81027abc10..66ba3e1b6e5a 100644 --- a/python/mozlint/mozlint/parser.py +++ b/python/mozlint/mozlint/parser.py @@ -42,7 +42,7 @@ class Parser(object): if linter['type'] not in supported_types: raise LinterParseError(relpath, "Invalid type '{}'".format(linter['type'])) - for attr in ('include', 'exclude'): + for attr in ('include', 'exclude', 'support-files'): if attr not in linter: continue @@ -100,5 +100,6 @@ class Parser(object): linter['name'] = name linter['path'] = path self._validate(linter) + linter.setdefault('support-files', []).append(path) linters.append(linter) return linters diff --git a/python/mozlint/mozlint/roller.py b/python/mozlint/mozlint/roller.py index 3b6c382ecf66..865e67090b3d 100644 --- a/python/mozlint/mozlint/roller.py +++ b/python/mozlint/mozlint/roller.py @@ -15,6 +15,7 @@ from multiprocessing import cpu_count from multiprocessing.queues import Queue from subprocess import CalledProcessError +import mozpack.path as mozpath from mozversioncontrol import get_repository_object, MissingUpstreamRepo, InvalidRepoPath from .errors import LintersNotConfigured @@ -144,13 +145,25 @@ class LintRoller(object): return 1 return 0 - def _generate_jobs(self, paths, num_procs): + def _generate_jobs(self, paths, vcs_paths, num_procs): """A job is of the form (, ).""" - chunk_size = min(self.MAX_PATHS_PER_JOB, int(ceil(float(len(paths)) / num_procs))) - while paths: - for linter in self.linters: - yield linter, paths[:chunk_size] - paths = paths[chunk_size:] + for linter in self.linters: + if any(os.path.isfile(p) and mozpath.match(p, pattern) + for pattern in linter.get('support-files', []) for p in vcs_paths): + lpaths = [self.root] + print("warning: {} support-file modified, linting entire tree " + "(press ctrl-c to cancel)".format(linter['name'])) + else: + lpaths = paths.union(vcs_paths) + + lpaths = lpaths or ['.'] + lpaths = map(os.path.abspath, lpaths) + chunk_size = min(self.MAX_PATHS_PER_JOB, int(ceil(len(lpaths) / num_procs))) or 1 + assert chunk_size > 0 + + while lpaths: + yield linter, lpaths[:chunk_size] + lpaths = lpaths[chunk_size:] def _collect_results(self, future): if future.cancelled(): @@ -192,12 +205,13 @@ class LintRoller(object): "--workdir or --outgoing".format(self.lintargs['root'])) # Calculate files from VCS + vcs_paths = set() try: if workdir: - paths.update(self.vcs.get_changed_files('AM', mode=workdir)) + vcs_paths.update(self.vcs.get_changed_files('AM', mode=workdir)) if outgoing: try: - paths.update(self.vcs.get_outgoing_files('AM', upstream=outgoing)) + vcs_paths.update(self.vcs.get_outgoing_files('AM', upstream=outgoing)) except MissingUpstreamRepo: print("warning: could not find default push, specify a remote for --outgoing") except CalledProcessError as e: @@ -205,21 +219,15 @@ class LintRoller(object): if e.output: print(e.output) - if not paths and (workdir or outgoing): + if not (paths or vcs_paths) and (workdir or outgoing): print("warning: no files linted") return {} - paths = paths or ['.'] - - # This will convert paths back to a list, but that's ok since - # we're done adding to it. - paths = map(os.path.abspath, paths) - num_procs = num_procs or cpu_count() - jobs = list(self._generate_jobs(paths, num_procs)) + jobs = list(self._generate_jobs(paths, vcs_paths, num_procs)) # Make sure we never spawn more processes than we have jobs. - num_procs = min(len(jobs), num_procs) + num_procs = min(len(jobs), num_procs) or 1 signal.signal(signal.SIGINT, _worker_sigint_handler) executor = ProcessPoolExecutor(num_procs) diff --git a/python/mozlint/test/conftest.py b/python/mozlint/test/conftest.py index bf6945d4f464..4e7f0c3fc9c3 100644 --- a/python/mozlint/test/conftest.py +++ b/python/mozlint/test/conftest.py @@ -6,6 +6,7 @@ from __future__ import absolute_import import os import sys +from argparse import Namespace import pytest @@ -18,7 +19,19 @@ here = os.path.abspath(os.path.dirname(__file__)) @pytest.fixture def lint(request): lintargs = getattr(request.module, 'lintargs', {}) - return LintRoller(root=here, **lintargs) + lint = LintRoller(root=here, **lintargs) + + # Add a few super powers to our lint instance + def mock_vcs(files): + def _fake_vcs_files(*args, **kwargs): + return files + + setattr(lint.vcs, 'get_changed_files', _fake_vcs_files) + setattr(lint.vcs, 'get_outgoing_files', _fake_vcs_files) + + setattr(lint, 'vcs', Namespace()) + setattr(lint, 'mock_vcs', mock_vcs) + return lint @pytest.fixture(scope='session') diff --git a/python/mozlint/test/linters/external.py b/python/mozlint/test/linters/external.py index 3d3a68c387d4..177be52b027d 100644 --- a/python/mozlint/test/linters/external.py +++ b/python/mozlint/test/linters/external.py @@ -56,6 +56,10 @@ def structured(files, config, logger, **kwargs): rule="no-foobar") +def passes(files, config, **lintargs): + return [] + + def setup(root): print('setup passed') diff --git a/python/mozlint/test/linters/invalid_support_files.yml b/python/mozlint/test/linters/invalid_support_files.yml new file mode 100644 index 000000000000..db39597d682f --- /dev/null +++ b/python/mozlint/test/linters/invalid_support_files.yml @@ -0,0 +1,6 @@ +--- +BadSupportFilesLinter: + description: Has an invalid support files directive. + support-files: should be a list + type: string + payload: foobar diff --git a/python/mozlint/test/linters/support_files.yml b/python/mozlint/test/linters/support_files.yml new file mode 100644 index 000000000000..0c278d51faef --- /dev/null +++ b/python/mozlint/test/linters/support_files.yml @@ -0,0 +1,10 @@ +--- +SupportFilesLinter: + description: Linter that has a few support files + include: + - files + support-files: + - '**/*.py' + type: external + extensions: ['.js', '.jsm'] + payload: external:passes diff --git a/python/mozlint/test/test_parser.py b/python/mozlint/test/test_parser.py index bde5fcf08e42..b3a0b924c8e2 100644 --- a/python/mozlint/test/test_parser.py +++ b/python/mozlint/test/test_parser.py @@ -48,6 +48,7 @@ def test_parse_valid_linter(parse): 'invalid_extension.ym', 'invalid_include.yml', 'invalid_exclude.yml', + 'invalid_support_files.yml', 'missing_attrs.yml', 'missing_definition.yml', 'non_existing_include.yml', diff --git a/python/mozlint/test/test_roller.py b/python/mozlint/test/test_roller.py index 7b9825dc156a..f3fc4db080b1 100644 --- a/python/mozlint/test/test_roller.py +++ b/python/mozlint/test/test_roller.py @@ -138,6 +138,44 @@ def test_keyboard_interrupt(): assert 'Traceback' not in out +def test_support_files(lint, lintdir, filedir, monkeypatch): + jobs = [] + + # Replace the original _generate_jobs with a new one that simply + # adds jobs to a list (and then doesn't return anything). + orig_generate_jobs = lint._generate_jobs + + def fake_generate_jobs(*args, **kwargs): + jobs.extend([job[1] for job in orig_generate_jobs(*args, **kwargs)]) + return [] + + monkeypatch.setattr(lint, '_generate_jobs', fake_generate_jobs) + + linter_path = os.path.join(lintdir, 'support_files.yml') + lint.read(linter_path) + + # Modified support files only lint entire root if --outgoing or --workdir + # are used. + path = os.path.join(filedir, 'foobar.js') + lint.mock_vcs([os.path.join(filedir, 'foobar.py')]) + lint.roll(path) + assert jobs[0] == [path] + + jobs = [] + lint.roll(path, workdir=True) + assert jobs[0] == [lint.root] + + jobs = [] + lint.roll(path, outgoing=True) + assert jobs[0] == [lint.root] + + # Lint config file is implicitly added as a support file + lint.mock_vcs([linter_path]) + jobs = [] + lint.roll(path, outgoing=True, workdir=True) + assert jobs[0] == [lint.root] + + linters = ('setup.yml', 'setupfailed.yml', 'setupraised.yml') diff --git a/tools/lint/docs/create.rst b/tools/lint/docs/create.rst index 26f42af26072..8f1dcf18149b 100644 --- a/tools/lint/docs/create.rst +++ b/tools/lint/docs/create.rst @@ -79,6 +79,7 @@ Each ``.yml`` file must have at least one linter defined in it. Here are the sup * 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) +* support-files - A list of glob patterns matching configuration files (optional) In addition to the above, some ``.yml`` files correspond to a single lint rule. For these, the following additional keys may be specified: @@ -168,6 +169,8 @@ Now here is the linter definition that would call it: - '**/*.py' type: external payload: py.flake8:lint + support-files: + - '**/.flake8' Notice the payload has two parts, delimited by ':'. The first is the module path, which ``mozlint`` will attempt to import. The second is the object path @@ -175,6 +178,11 @@ within that module (e.g, the name of a function to call). It is up to consumers of ``mozlint`` to ensure the module is in ``sys.path``. Structured log linters use the same import mechanism. +The ``support-files`` key is used to list configuration files or files related +to the running of the linter itself. If using ``--outgoing`` or ``--workdir`` +and one of these files was modified, the entire tree will be linted instead of +just the modified files. + Bootstrapping Dependencies --------------------------