diff options
Diffstat (limited to 'python/tidy.py')
| -rw-r--r-- | python/tidy.py | 150 |
1 files changed, 111 insertions, 39 deletions
diff --git a/python/tidy.py b/python/tidy.py index 9ea9de24f30..95691893525 100644 --- a/python/tidy.py +++ b/python/tidy.py @@ -16,26 +16,18 @@ import StringIO import sys from licenseck import licenses -filetypes_to_check = [".rs", ".rc", ".cpp", ".c", ".h", ".py", ".toml", ".webidl"] +filetypes_to_check = [".rs", ".rc", ".cpp", ".c", ".h", ".lock", ".py", ".toml", ".webidl"] reftest_dir = "./tests/ref" reftest_filetype = ".list" -python_dependencies = [ - "./python/dependencies/flake8-2.4.1-py2.py3-none-any.whl", - "./python/dependencies/pep8-1.5.7-py2.py3-none-any.whl", - "./python/dependencies/pyflakes-0.9.0-py2.py3-none-any.whl", -] ignored_files = [ # Upstream "./support/*", "./tests/wpt/*", "./python/mach/*", - "./python/mozdebug/*", - "./python/mozinfo/*", - "./python/mozlog/*", - "./python/toml/*", "./components/script/dom/bindings/codegen/parser/*", "./components/script/dom/bindings/codegen/ply/*", + "./python/_virtualenv/*", # Generated and upstream code combined with our own. Could use cleanup "./target/*", @@ -70,7 +62,7 @@ VIM_HEADER = "/* vim:" def check_license(file_name, contents): - if file_name.endswith(".toml"): + if file_name.endswith(".toml") or file_name.endswith(".lock"): raise StopIteration while contents.startswith(EMACS_HEADER) or contents.startswith(VIM_HEADER): _, _, contents = contents.partition("\n") @@ -148,6 +140,46 @@ def check_flake8(file_name, contents): yield line_num, message.strip() +def check_lock(file_name, contents): + if not file_name.endswith(".lock"): + raise StopIteration + contents = contents.splitlines(True) + idx = 1 + packages = {} + exceptions = ["glutin", "wayland-kbd"] # package names to be neglected (as named by cargo) + + while idx < len(contents): + content = contents[idx].strip() + if 'name' in content: + base_name = content.split('"')[1] + # we need the base package because some other package might demand a new version in the following lines + packages[base_name] = contents[idx + 1].split('"')[1], idx + 2, base_name + if 'dependencies' in content: + idx += 1 + while contents[idx].strip() != ']': + package = contents[idx].strip().strip('",').split() + name, version = package[0], package[1] + if name not in packages: # store the line number & base package name for future comparison + packages[name] = (version, idx + 1, base_name) + elif all([packages[name][0] != version, name not in exceptions, base_name not in exceptions]): + line = idx + 1 + version_1 = tuple(map(int, packages[name][0].split('.'))) + version_2 = tuple(map(int, version.split('.'))) + if version_1 < version_2: # get the line & base package containing the older version + packages[name], (version, line, base_name) = (version, line, base_name), packages[name] + + message = 'conflicting versions for package "%s"' % name + error = '\n\t\033[93mexpected maximum version "{}"\033[0m'.format(packages[name][0]) + \ + '\n\t\033[91mbut, "{}" demands "{}"\033[0m' \ + .format(base_name, version) + suggest = "\n\t\033[93mtry upgrading with\033[0m " + \ + "\033[96m./mach cargo-update -p {}:{}\033[0m" \ + .format(name, version) + yield (line, message + error + suggest) + idx += 1 + idx += 1 + + def check_toml(file_name, contents): if not file_name.endswith(".toml"): raise StopIteration @@ -188,33 +220,35 @@ def check_rust(file_name, contents): line = merged_lines + line merged_lines = '' - # get rid of strings and chars because cases like regex expression - line = re.sub('".*?"|\'.*?\'', '', line) + # get rid of strings and chars because cases like regex expression, keep attributes + if not line_is_attribute(line): + line = re.sub('".*?"|\'.*?\'', '', line) - # get rid of comments and attributes - line = re.sub('//.*?$|/\*.*?$|^\*.*?$|^#.*?$', '', line) + # get rid of comments + line = re.sub('//.*?$|/\*.*?$|^\*.*?$', '', line) - match = re.search(r",[A-Za-z0-9]", line) - if match: + # get rid of attributes that do not contain = + line = re.sub('^#[A-Za-z0-9\(\)\[\]_]*?$', '', line) + + match = re.search(r",[^\s]", line) + if match and '$' not in line: yield (idx + 1, "missing space after ,") - # Avoid flagging <Item=Foo> constructs - def is_associated_type(match, line, index): - open_angle = line[0:match.end()].rfind('<') - close_angle = line[open_angle:].find('>') if open_angle != -1 else -1 - is_equals = match.group(0)[index] == '=' - generic_open = open_angle != -1 and open_angle < match.start() - generic_close = close_angle != -1 and close_angle + open_angle >= match.end() - return is_equals and generic_open and generic_close - - # - not included because of scientific notation (1e-6) - match = re.search(r"[A-Za-z0-9][\+/\*%=]", line) + if line_is_attribute(line): + pre_space_re = r"[A-Za-z0-9]=" + post_space_re = r"=[A-Za-z0-9\"]" + else: + # - not included because of scientific notation (1e-6) + pre_space_re = r"[A-Za-z0-9][\+/\*%=]" + # * not included because of dereferencing and casting + # - not included because of unary negation + post_space_re = r"[\+/\%=][A-Za-z0-9\"]" + + match = re.search(pre_space_re, line) if match and not is_associated_type(match, line, 1): yield (idx + 1, "missing space before %s" % match.group(0)[1]) - # * not included because of dereferencing and casting - # - not included because of unary negation - match = re.search(r"[\+/\%=][A-Za-z0-9]", line) + match = re.search(post_space_re, line) if match and not is_associated_type(match, line, 0): yield (idx + 1, "missing space after %s" % match.group(0)[0]) @@ -243,6 +277,16 @@ def check_rust(file_name, contents): if match: yield (idx + 1, "missing space before {") + # ignored cases like {} and }} + match = re.search(r"[^\s{}]}", line) + if match and not (line.startswith("use") or line.startswith("pub use")): + yield (idx + 1, "missing space before }") + + # ignored cases like {} and {{ + match = re.search(r"{[^\s{}]", line) + if match and not (line.startswith("use") or line.startswith("pub use")): + yield (idx + 1, "missing space after {") + # imports must be in the same line and alphabetically sorted if line.startswith("use "): use = line[4:] @@ -260,6 +304,20 @@ def check_rust(file_name, contents): uses = [] +# Avoid flagging <Item=Foo> constructs +def is_associated_type(match, line, index): + open_angle = line[0:match.end()].rfind('<') + close_angle = line[open_angle:].find('>') if open_angle != -1 else -1 + is_equals = match.group(0)[index] == '=' + generic_open = open_angle != -1 and open_angle < match.start() + generic_close = close_angle != -1 and close_angle + open_angle >= match.end() + return is_equals and generic_open and generic_close + + +def line_is_attribute(line): + return re.search(r"#\[.*\]", line) + + def check_webidl_spec(file_name, contents): # Sorted by this function (in pseudo-Rust). The idea is to group the same # organization together. @@ -313,7 +371,17 @@ def check_spec(file_name, contents): raise StopIteration file_name = os.path.relpath(os.path.splitext(file_name)[0], base_path) patt = re.compile("^\s*\/\/.+") - pattern = "impl<'a> %sMethods for &'a %s {" % (file_name, file_name) + + # Pattern representing a line with a macro + macro_patt = re.compile("^\s*\S+!(.*)$") + + # Pattern representing a line with comment containing a spec link + link_patt = re.compile("^\s*///? https://.+$") + + # Pattern representing a line with comment + comment_patt = re.compile("^\s*///?.+$") + + pattern = "impl %sMethods for %s {" % (file_name, file_name) contents = contents.splitlines(True) brace_count = 0 in_impl = False @@ -323,9 +391,16 @@ def check_spec(file_name, contents): if not patt.match(line): if pattern.lower() in line.lower(): in_impl = True - if "fn " in line and brace_count == 1: - if "// https://" not in contents[idx - 1] and "// https://" not in contents[idx - 2]: - yield (idx + 1, "method declared in webidl is missing a comment with a specification link") + if ("fn " in line or macro_patt.match(line)) and brace_count == 1: + for up_idx in range(1, idx + 1): + up_line = contents[idx - up_idx] + if link_patt.match(up_line): + # Comment with spec link exists + break + if not comment_patt.match(up_line): + # No more comments exist above, yield warning + yield (idx + 1, "method declared in webidl is missing a comment with a specification link") + break if '{' in line and in_impl: brace_count += 1 if '}' in line and in_impl: @@ -335,7 +410,6 @@ def check_spec(file_name, contents): def collect_errors_for_files(files_to_check, checking_functions): - base_path = "components/script/dom/" for file_name in files_to_check: with open(file_name, "r") as fp: contents = fp.read() @@ -384,13 +458,11 @@ def check_reftest_html_files_in_basic_list(reftest_dir): def scan(): - sys.path += python_dependencies - all_files = (os.path.join(r, f) for r, _, files in os.walk(".") for f in files) files_to_check = filter(should_check, all_files) checking_functions = [check_license, check_by_line, check_flake8, check_toml, - check_rust, check_webidl_spec, check_spec] + check_lock, check_rust, check_webidl_spec, check_spec] errors = collect_errors_for_files(files_to_check, checking_functions) reftest_files = (os.path.join(r, f) for r, _, files in os.walk(reftest_dir) for f in files) |