diff options
| -rw-r--r-- | python/servo/lints/wpt_lint.py | 37 | ||||
| -rw-r--r-- | python/tidy/servo_tidy/tidy.py | 162 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/invalid_error_tuple.py | 5 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/no_lint.py | 5 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/no_run.py | 5 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/not_inherited.py | 2 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/not_script | 0 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/lints/proper_file.py | 6 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/test_tidy.py | 30 | ||||
| -rw-r--r-- | servo-tidy.toml | 3 |
10 files changed, 189 insertions, 66 deletions
diff --git a/python/servo/lints/wpt_lint.py b/python/servo/lints/wpt_lint.py new file mode 100644 index 00000000000..0fab47c7c35 --- /dev/null +++ b/python/servo/lints/wpt_lint.py @@ -0,0 +1,37 @@ +# Copyright 2013 The Servo Project Developers. See the COPYRIGHT +# file at the top-level directory of this distribution. +# +# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +# <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +# option. This file may not be copied, modified, or distributed +# except according to those terms. + +import os +import site + +from servo_tidy.tidy import LintRunner, filter_file + +WPT_PATH = os.path.join(".", "tests", "wpt") +SUITES = ["web-platform-tests", os.path.join("mozilla", "tests")] + + +class Lint(LintRunner): + def _get_wpt_files(self, suite): + working_dir = os.path.join(WPT_PATH, suite, '') + file_iter = self.get_files(working_dir, exclude_dirs=[]) + print '\nRunning the WPT lint on %s...' % working_dir + for f in file_iter: + if filter_file(f): + yield f[len(working_dir):] + + def run(self): + wpt_working_dir = os.path.abspath(os.path.join(WPT_PATH, "web-platform-tests")) + for suite in SUITES: + files = self._get_wpt_files(suite) + site.addsitedir(wpt_working_dir) + from tools.lint import lint + file_dir = os.path.abspath(os.path.join(WPT_PATH, suite)) + returncode = lint.lint(file_dir, files, output_json=False) + if returncode: + yield ("WPT Lint Tool", "", "lint error(s) in Web Platform Tests: exit status %s" % returncode) diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 45ad2f44638..bfb40bccd91 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -9,18 +9,19 @@ import contextlib import fnmatch +import imp import itertools import json import os import re -import site import StringIO import subprocess import sys -from licenseck import MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml import colorama import toml +from licenseck import MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml + CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml") # Default configs @@ -28,6 +29,7 @@ config = { "skip-check-length": False, "skip-check-licenses": False, "check-ordered-json-keys": [], + "lint-scripts": [], "ignore": { "files": [ "./.", # ignore hidden files @@ -86,7 +88,8 @@ def is_iter_empty(iterator): return False, iterator -# A simple wrapper for iterators to show progress (note that it's inefficient for giant iterators) +# A simple wrapper for iterators to show progress +# (Note that it's inefficient for giant iterators, since it iterates once to get the upper bound) def progress_wrapper(iterator): list_of_stuff = list(iterator) total_files, progress = len(list_of_stuff), 0 @@ -97,6 +100,49 @@ def progress_wrapper(iterator): yield thing +class FileList(object): + def __init__(self, directory, only_changed_files=False, exclude_dirs=[], progress=True): + self.directory = directory + self.excluded = exclude_dirs + iterator = self._git_changed_files() if only_changed_files else \ + self._filter_excluded() if exclude_dirs else self._default_walk() + # Raise `StopIteration` if the iterator is empty + obj = next(iterator) + self.generator = itertools.chain((obj,), iterator) + if progress: + self.generator = progress_wrapper(self.generator) + + def _default_walk(self): + for root, _, files in os.walk(self.directory): + for f in files: + yield os.path.join(root, f) + + def _git_changed_files(self): + args = ["git", "log", "-n1", "--merges", "--format=%H"] + last_merge = subprocess.check_output(args).strip() + args = ["git", "diff", "--name-only", last_merge, self.directory] + file_list = subprocess.check_output(args) + + for f in file_list.splitlines(): + if sys.platform == 'win32': + os.path.join(*f.split('/')) + if not any(os.path.join('.', os.path.dirname(f)).startswith(path) for path in self.excluded): + yield os.path.join('.', f) + + def _filter_excluded(self): + for root, dirs, files in os.walk(self.directory, topdown=True): + # modify 'dirs' in-place so that we don't do unnecessary traversals in excluded directories + dirs[:] = [d for d in dirs if not any(os.path.join(root, d).startswith(name) for name in self.excluded)] + for rel_path in files: + yield os.path.join(root, rel_path) + + def __iter__(self): + return self + + def next(self): + return next(self.generator) + + def filter_file(file_name): if any(file_name.startswith(ignored_file) for ignored_file in config["ignore"]["files"]): return False @@ -107,13 +153,8 @@ def filter_file(file_name): def filter_files(start_dir, only_changed_files, progress): - file_iter = get_file_list(start_dir, only_changed_files, config["ignore"]["directories"]) - (has_element, file_iter) = is_iter_empty(file_iter) - if not has_element: - raise StopIteration - if progress: - file_iter = progress_wrapper(file_iter) - + file_iter = FileList(start_dir, only_changed_files=only_changed_files, + exclude_dirs=config["ignore"]["directories"], progress=progress) for file_name in file_iter: base_name = os.path.basename(file_name) if not any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_CHECK): @@ -829,31 +870,6 @@ def collect_errors_for_files(files_to_check, checking_functions, line_checking_f yield (filename,) + error -def get_wpt_files(suite, only_changed_files, progress): - wpt_dir = os.path.join(".", "tests", "wpt", suite, "") - file_iter = get_file_list(os.path.join(wpt_dir), only_changed_files) - (has_element, file_iter) = is_iter_empty(file_iter) - if not has_element: - raise StopIteration - print '\nRunning the WPT lint...' - if progress: - file_iter = progress_wrapper(file_iter) - for f in file_iter: - if filter_file(f): - yield f[len(wpt_dir):] - - -def check_wpt_lint_errors(suite, files): - wpt_working_dir = os.path.abspath(os.path.join(".", "tests", "wpt", "web-platform-tests")) - if os.path.isdir(wpt_working_dir): - site.addsitedir(wpt_working_dir) - from tools.lint import lint - file_dir = os.path.abspath(os.path.join(".", "tests", "wpt", suite)) - returncode = lint.lint(file_dir, files, output_json=False) - if returncode: - yield ("WPT Lint Tool", "", "lint error(s) in Web Platform Tests: exit status {0}".format(returncode)) - - def get_dep_toml_files(only_changed_files=False): if not only_changed_files: print '\nRunning the dependency licensing lint...' @@ -876,27 +892,52 @@ def check_dep_license_errors(filenames, progress=True): yield (filename, 0, "dependency should contain a valid license.") -def get_file_list(directory, only_changed_files=False, exclude_dirs=[]): - if only_changed_files: - # only check tracked files that have been changed since the last merge - args = ["git", "log", "-n1", "--author=bors-servo", "--format=%H"] - last_merge = subprocess.check_output(args).strip() - args = ["git", "diff", "--name-only", last_merge, directory] - file_list = subprocess.check_output(args) - for f in file_list.splitlines(): - f = os.path.join(*f.split("/")) if sys.platform == "win32" else f - if not any(os.path.join('.', os.path.dirname(f)).startswith(path) for path in exclude_dirs): - yield os.path.join('.', f) - elif exclude_dirs: - for root, dirs, files in os.walk(directory, topdown=True): - # modify 'dirs' in-place so that we don't do unwanted traversals in excluded directories - dirs[:] = [d for d in dirs if not any(os.path.join(root, d).startswith(name) for name in exclude_dirs)] - for rel_path in files: - yield os.path.join(root, rel_path) - else: - for root, _, files in os.walk(directory): - for f in files: - yield os.path.join(root, f) +class LintRunner(object): + def __init__(self, lint_path=None, only_changed_files=True, exclude_dirs=[], progress=True): + self.only_changed_files = only_changed_files + self.exclude_dirs = exclude_dirs + self.progress = progress + self.path = lint_path + + def check(self): + if not os.path.exists(self.path): + yield (self.path, 0, "file does not exist") + return + if not self.path.endswith('.py'): + yield (self.path, 0, "lint should be a python script") + return + dir_name, filename = os.path.split(self.path) + sys.path.append(dir_name) + module = imp.load_source(filename[:-3], self.path) + if hasattr(module, 'Lint'): + if issubclass(module.Lint, LintRunner): + lint = module.Lint(self.path, self.only_changed_files, self.exclude_dirs, self.progress) + for error in lint.run(): + if not hasattr(error, '__iter__'): + yield (self.path, 1, "errors should be a tuple of (path, line, reason)") + return + yield error + else: + yield (self.path, 1, "class 'Lint' should inherit from 'LintRunner'") + else: + yield (self.path, 1, "script should contain a class named 'Lint'") + sys.path.remove(dir_name) + + def get_files(self, path, **kwargs): + args = ['only_changed_files', 'exclude_dirs', 'progress'] + kwargs = {k: kwargs.get(k, getattr(self, k)) for k in args} + return FileList(path, **kwargs) + + def run(self): + yield (self.path, 0, "class 'Lint' should implement 'run' method") + + +def run_lint_scripts(only_changed_files=False, progress=True): + runner = LintRunner(only_changed_files=only_changed_files, progress=progress) + for path in config['lint-scripts']: + runner.path = path + for error in runner.check(): + yield error def scan(only_changed_files=False, progress=True): @@ -912,13 +953,10 @@ def scan(only_changed_files=False, progress=True): file_errors = collect_errors_for_files(files_to_check, checking_functions, line_checking_functions) # check dependecy licenses dep_license_errors = check_dep_license_errors(get_dep_toml_files(only_changed_files), progress) - # wpt lint checks - wpt_lint_errors = [ - check_wpt_lint_errors(suite, get_wpt_files(suite, only_changed_files, progress)) - for suite in ["web-platform-tests", os.path.join("mozilla", "tests")] - ] + # other lint checks + lint_errors = run_lint_scripts(only_changed_files, progress) # chain all the iterators - errors = itertools.chain(config_errors, directory_errors, file_errors, dep_license_errors, *wpt_lint_errors) + errors = itertools.chain(config_errors, directory_errors, file_errors, dep_license_errors, lint_errors) error = None for error in errors: diff --git a/python/tidy/servo_tidy_tests/lints/invalid_error_tuple.py b/python/tidy/servo_tidy_tests/lints/invalid_error_tuple.py new file mode 100644 index 00000000000..4851cdf402c --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/invalid_error_tuple.py @@ -0,0 +1,5 @@ +from servo_tidy.tidy import LintRunner + +class Lint(LintRunner): + def run(self): + yield None diff --git a/python/tidy/servo_tidy_tests/lints/no_lint.py b/python/tidy/servo_tidy_tests/lints/no_lint.py new file mode 100644 index 00000000000..e9f84aa9f3c --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/no_lint.py @@ -0,0 +1,5 @@ +from servo_tidy.tidy import LintRunner + +class Linter(LintRunner): + def run(self): + pass diff --git a/python/tidy/servo_tidy_tests/lints/no_run.py b/python/tidy/servo_tidy_tests/lints/no_run.py new file mode 100644 index 00000000000..2acd5db1fee --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/no_run.py @@ -0,0 +1,5 @@ +from servo_tidy.tidy import LintRunner + +class Lint(LintRunner): + def some_method(self): + pass diff --git a/python/tidy/servo_tidy_tests/lints/not_inherited.py b/python/tidy/servo_tidy_tests/lints/not_inherited.py new file mode 100644 index 00000000000..fc38dff2c58 --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/not_inherited.py @@ -0,0 +1,2 @@ +class Lint(object): + pass diff --git a/python/tidy/servo_tidy_tests/lints/not_script b/python/tidy/servo_tidy_tests/lints/not_script new file mode 100644 index 00000000000..e69de29bb2d --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/not_script diff --git a/python/tidy/servo_tidy_tests/lints/proper_file.py b/python/tidy/servo_tidy_tests/lints/proper_file.py new file mode 100644 index 00000000000..acecb82abd4 --- /dev/null +++ b/python/tidy/servo_tidy_tests/lints/proper_file.py @@ -0,0 +1,6 @@ +from servo_tidy.tidy import LintRunner + +class Lint(LintRunner): + def run(self): + for _ in [None]: + yield ('path', 0, 'foobar') diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 6dd97aee7e3..a6a618d1608 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -188,14 +188,36 @@ class CheckTidiness(unittest.TestCase): self.assertEqual(msg, errors.next()[2]) self.assertNoMoreErrors(errors) + def test_lint_runner(self): + test_path = base_path + 'lints/' + runner = tidy.LintRunner(only_changed_files=False, progress=False) + runner.path = test_path + 'some-fictional-file' + self.assertEqual([(runner.path, 0, "file does not exist")], list(runner.check())) + runner.path = test_path + 'not_script' + self.assertEqual([(runner.path, 0, "lint should be a python script")], + list(runner.check())) + runner.path = test_path + 'not_inherited.py' + self.assertEqual([(runner.path, 1, "class 'Lint' should inherit from 'LintRunner'")], + list(runner.check())) + runner.path = test_path + 'no_lint.py' + self.assertEqual([(runner.path, 1, "script should contain a class named 'Lint'")], + list(runner.check())) + runner.path = test_path + 'no_run.py' + self.assertEqual([(runner.path, 0, "class 'Lint' should implement 'run' method")], + list(runner.check())) + runner.path = test_path + 'invalid_error_tuple.py' + self.assertEqual([(runner.path, 1, "errors should be a tuple of (path, line, reason)")], + list(runner.check())) + runner.path = test_path + 'proper_file.py' + self.assertEqual([('path', 0, "foobar")], list(runner.check())) + def test_file_list(self): base_path='./python/tidy/servo_tidy_tests/test_ignored' - file_list = tidy.get_file_list(base_path, only_changed_files=False, - exclude_dirs=[]) + file_list = tidy.FileList(base_path, only_changed_files=False, exclude_dirs=[]) lst = list(file_list) self.assertEqual([os.path.join(base_path, 'whee', 'test.rs'), os.path.join(base_path, 'whee', 'foo', 'bar.rs')], lst) - file_list = tidy.get_file_list(base_path, only_changed_files=False, - exclude_dirs=[os.path.join(base_path, 'whee', 'foo')]) + file_list = tidy.FileList(base_path, only_changed_files=False, + exclude_dirs=[os.path.join(base_path, 'whee', 'foo')]) lst = list(file_list) self.assertEqual([os.path.join(base_path, 'whee', 'test.rs')], lst) diff --git a/servo-tidy.toml b/servo-tidy.toml index 7992d0ae85c..194a1658a3c 100644 --- a/servo-tidy.toml +++ b/servo-tidy.toml @@ -5,6 +5,9 @@ check-ordered-json-keys = [ "./resources/prefs.json", "./resources/package-prefs.json", ] +lint-scripts = [ + "./python/servo/lints/wpt_lint.py", +] [ignore] # Ignored packages with duplicated versions |