diff options
Diffstat (limited to 'python/tidy')
| -rw-r--r-- | python/tidy/servo_tidy/tidy.py | 184 | ||||
| -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 |
8 files changed, 170 insertions, 67 deletions
diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index e96e4e82a55..b40bdd67194 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -9,19 +9,20 @@ 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 import yaml +from licenseck import MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml + CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml") # Default configs @@ -29,6 +30,7 @@ config = { "skip-check-length": False, "skip-check-licenses": False, "check-ordered-json-keys": [], + "lint-scripts": [], "ignore": { "files": [ "./.", # ignore hidden files @@ -63,7 +65,7 @@ WEBIDL_STANDARDS = [ "//dvcs.w3.org/hg", "//dom.spec.whatwg.org", "//domparsing.spec.whatwg.org", - "//drafts.csswg.org/cssom", + "//drafts.csswg.org", "//drafts.fxtf.org", "//encoding.spec.whatwg.org", "//fetch.spec.whatwg.org", @@ -88,7 +90,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 @@ -99,6 +102,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 @@ -109,13 +155,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): @@ -320,6 +361,8 @@ def check_toml(file_name, lines): raise StopIteration ok_licensed = False for idx, line in enumerate(lines): + if idx == 0 and "[workspace]" in line: + raise StopIteration if line.find("*") != -1: yield (idx + 1, "found asterisk instead of minimum version number") for license_line in licenses_toml: @@ -883,31 +926,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...' @@ -930,27 +948,67 @@ 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 check_commits(path='.'): + """Gets all commits since the last merge.""" + args = ['git', 'log', '-n1', '--merges', '--format=%H'] + last_merge = subprocess.check_output(args, cwd=path).strip() + args = ['git', 'log', '{}..HEAD'.format(last_merge), '--format=%s'] + commits = subprocess.check_output(args, cwd=path).lower().splitlines() + + for commit in commits: + # .split() to only match entire words + if 'wip' in commit.split(): + yield ('.', 0, 'no commits should contain WIP') + + raise StopIteration def scan(only_changed_files=False, progress=True): @@ -966,13 +1024,13 @@ 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) + # check commits for WIP + commit_errors = check_commits() # 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, + commit_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 12d7b9ebae7..c6fe8bd83fa 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -203,14 +203,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) |