Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-04 13:10:20 +01:00
Code Issues Projects Releases Packages Wiki Activity

Make mach test-tidy --no-wpt compatible with Python3

Browse source 
Note: tidiness tests of Python file using flake8 was effectively
broken since a previous commit for Python3 compatibility
This commit is contained in:
marmeladema 2019-12-10 23:51:49 +00:00
parent 8d4cedb911
commit 9d04f231f4
2 changed files with 66 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

4
python/servo/testing_commands.py
View file

@ -776,7 +776,7 @@ class MachCommands(CommandBase):
def setup_clangfmt(env): def setup_clangfmt(env):
cmd = "clang-format.exe" if sys.platform == "win32" else "clang-format" cmd = "clang-format.exe" if sys.platform == "win32" else "clang-format"
try: try:
version = check_output([cmd, "--version"], env=env).rstrip() version = check_output([cmd, "--version"], env=env, universal_newlines=True).rstrip()
print(version) print(version)
if not version.startswith("clang-format version {}.".format(CLANGFMT_VERSION)): if not version.startswith("clang-format version {}.".format(CLANGFMT_VERSION)):
print("clang-format: wrong version (v{} required). Skipping CPP formatting.".format(CLANGFMT_VERSION)) print("clang-format: wrong version (v{} required). Skipping CPP formatting.".format(CLANGFMT_VERSION))
@ -785,7 +785,7 @@ def setup_clangfmt(env):
print("clang-format not installed. Skipping CPP formatting.") print("clang-format not installed. Skipping CPP formatting.")
return False, None, None return False, None, None
gitcmd = ['git', 'ls-files'] gitcmd = ['git', 'ls-files']
gitfiles = check_output(gitcmd + CLANGFMT_CPP_DIRS).splitlines() gitfiles = check_output(gitcmd + CLANGFMT_CPP_DIRS, universal_newlines=True).splitlines()
filtered = [line for line in gitfiles if line.endswith(".h") or line.endswith(".cpp")] filtered = [line for line in gitfiles if line.endswith(".h") or line.endswith(".cpp")]
return True, cmd, filtered return True, cmd, filtered

128
python/tidy/servo_tidy/tidy.py
View file

@ -36,7 +36,7 @@ def wpt_path(*args):
CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml") CONFIG_FILE_PATH = os.path.join(".", "servo-tidy.toml")
WPT_MANIFEST_PATH = wpt_path("include.ini") WPT_MANIFEST_PATH = wpt_path("include.ini")
# regex source https://stackoverflow.com/questions/6883049/ # regex source https://stackoverflow.com/questions/6883049/
URL_REGEX = re.compile('https?://(?:[-\w.]|(?:%[\da-fA-F]{2}))+') URL_REGEX = re.compile(b'https?://(?:[-\w.]|(?:%[\da-fA-F]{2}))+')
# Import wptmanifest only when we do have wpt in tree, i.e. we're not # Import wptmanifest only when we do have wpt in tree, i.e. we're not
# inside a Firefox checkout. # inside a Firefox checkout.
@ -64,7 +64,7 @@ config = {
"check_ext": {} "check_ext": {}
} }
COMMENTS = ["// ", "# ", " *", "/* "] COMMENTS = [b"// ", b"# ", b" *", b"/* "]
# File patterns to include in the non-WPT tidy check. # File patterns to include in the non-WPT tidy check.
FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c", FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c",
@ -78,40 +78,40 @@ FILE_PATTERNS_TO_IGNORE = ["*.#*", "*.pyc", "fake-ld.sh", "*.ogv", "*.webm"]
SPEC_BASE_PATH = "components/script/dom/" SPEC_BASE_PATH = "components/script/dom/"
WEBIDL_STANDARDS = [ WEBIDL_STANDARDS = [
"//www.khronos.org/registry/webgl/extensions", b"//www.khronos.org/registry/webgl/extensions",
"//www.khronos.org/registry/webgl/specs", b"//www.khronos.org/registry/webgl/specs",
"//developer.mozilla.org/en-US/docs/Web/API", b"//developer.mozilla.org/en-US/docs/Web/API",
"//dev.w3.org/2006/webapi", b"//dev.w3.org/2006/webapi",
"//dev.w3.org/csswg", b"//dev.w3.org/csswg",
"//dev.w3.org/fxtf", b"//dev.w3.org/fxtf",
"//dvcs.w3.org/hg", b"//dvcs.w3.org/hg",
"//dom.spec.whatwg.org", b"//dom.spec.whatwg.org",
"//drafts.csswg.org", b"//drafts.csswg.org",
"//drafts.css-houdini.org", b"//drafts.css-houdini.org",
"//drafts.fxtf.org", b"//drafts.fxtf.org",
"//encoding.spec.whatwg.org", b"//encoding.spec.whatwg.org",
"//fetch.spec.whatwg.org", b"//fetch.spec.whatwg.org",
"//html.spec.whatwg.org", b"//html.spec.whatwg.org",
"//url.spec.whatwg.org", b"//url.spec.whatwg.org",
"//xhr.spec.whatwg.org", b"//xhr.spec.whatwg.org",
"//w3c.github.io", b"//w3c.github.io",
"//heycam.github.io/webidl", b"//heycam.github.io/webidl",
"//webbluetoothcg.github.io/web-bluetooth/", b"//webbluetoothcg.github.io/web-bluetooth/",
"//svgwg.org/svg2-draft", b"//svgwg.org/svg2-draft",
"//wicg.github.io", b"//wicg.github.io",
"//webaudio.github.io", b"//webaudio.github.io",
"//immersive-web.github.io/", b"//immersive-web.github.io/",
"//github.com/immersive-web/webxr-test-api/", b"//github.com/immersive-web/webxr-test-api/",
"//gpuweb.github.io", b"//gpuweb.github.io",
# Not a URL # Not a URL
"// This interface is entirely internal to Servo, and should not be" + b"// This interface is entirely internal to Servo, and should not be" +
" accessible to\n// web pages." b" accessible to\n// web pages."
] ]
def is_iter_empty(iterator): def is_iter_empty(iterator):
try: try:
obj = iterator.next() obj = next(iterator)
return True, itertools.chain((obj,), iterator) return True, itertools.chain((obj,), iterator)
except StopIteration: except StopIteration:
return False, iterator return False, iterator
@ -163,9 +163,9 @@ class FileList(object):
def _git_changed_files(self): def _git_changed_files(self):
args = ["git", "log", "-n1", "--merges", "--format=%H"] args = ["git", "log", "-n1", "--merges", "--format=%H"]
last_merge = subprocess.check_output(args).strip() last_merge = subprocess.check_output(args, universal_newlines=True).strip()
args = ["git", "diff", "--name-only", last_merge, self.directory] args = ["git", "diff", "--name-only", last_merge, self.directory]
file_list = normilize_paths(subprocess.check_output(args).splitlines()) file_list = normilize_paths(subprocess.check_output(args, universal_newlines=True).splitlines())
for f in file_list: for f in file_list:
if not any(os.path.join('.', os.path.dirname(f)).startswith(path) for path in self.excluded): if not any(os.path.join('.', os.path.dirname(f)).startswith(path) for path in self.excluded):
@ -179,7 +179,7 @@ class FileList(object):
yield os.path.join(root, rel_path) yield os.path.join(root, rel_path)
def __iter__(self): def __iter__(self):
return self return self.generator
def next(self): def next(self):
return next(self.generator) return next(self.generator)
@ -200,7 +200,7 @@ def filter_files(start_dir, only_changed_files, progress):
# always yield Cargo.lock so that the correctness of transitive dependacies is checked # always yield Cargo.lock so that the correctness of transitive dependacies is checked
yield "./Cargo.lock" yield "./Cargo.lock"
for file_name in file_iter: for file_name in iter(file_iter):
base_name = os.path.basename(file_name) base_name = os.path.basename(file_name)
if not any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_CHECK): if not any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_CHECK):
continue continue
@ -212,7 +212,7 @@ def filter_files(start_dir, only_changed_files, progress):
def uncomment(line): def uncomment(line):
for c in COMMENTS: for c in COMMENTS:
if line.startswith(c): if line.startswith(c):
if line.endswith("*/"): if line.endswith(b"*/"):
return line[len(c):(len(line) - 3)].strip() return line[len(c):(len(line) - 3)].strip()
return line[len(c):].strip() return line[len(c):].strip()
@ -227,15 +227,15 @@ def check_license(file_name, lines):
config["skip-check-licenses"]: config["skip-check-licenses"]:
raise StopIteration raise StopIteration
if lines[0].startswith("#!") and lines[1].strip(): if lines[0].startswith(b"#!") and lines[1].strip():
yield (1, "missing blank line after shebang") yield (1, "missing blank line after shebang")
blank_lines = 0 blank_lines = 0
max_blank_lines = 2 if lines[0].startswith("#!") else 1 max_blank_lines = 2 if lines[0].startswith(b"#!") else 1
license_block = [] license_block = []
for l in lines: for l in lines:
l = l.rstrip('\n') l = l.rstrip(b'\n')
if not l.strip(): if not l.strip():
blank_lines += 1 blank_lines += 1
if blank_lines >= max_blank_lines: if blank_lines >= max_blank_lines:
@ -245,7 +245,7 @@ def check_license(file_name, lines):
if line is not None: if line is not None:
license_block.append(line) license_block.append(line)
header = " ".join(license_block) header = (b" ".join(license_block)).decode("utf-8")
valid_license = OLD_MPL in header or MPL in header or is_apache_licensed(header) valid_license = OLD_MPL in header or MPL in header or is_apache_licensed(header)
acknowledged_bad_license = "xfail-license" in header acknowledged_bad_license = "xfail-license" in header
if not (valid_license or acknowledged_bad_license): if not (valid_license or acknowledged_bad_license):
@ -254,9 +254,9 @@ def check_license(file_name, lines):
def check_modeline(file_name, lines): def check_modeline(file_name, lines):
for idx, line in enumerate(lines[:5]): for idx, line in enumerate(lines[:5]):
if re.search('^.*[ \t](vi:|vim:|ex:)[ \t]', line): if re.search(b'^.*[ \t](vi:|vim:|ex:)[ \t]', line):
yield (idx + 1, "vi modeline present") yield (idx + 1, "vi modeline present")
elif re.search('-\*-.*-\*-', line, re.IGNORECASE): elif re.search(b'-\*-.*-\*-', line, re.IGNORECASE):
yield (idx + 1, "emacs file variables present") yield (idx + 1, "emacs file variables present")
@ -267,7 +267,7 @@ def check_length(file_name, idx, line):
# Prefer shorter lines when shell scripting. # Prefer shorter lines when shell scripting.
max_length = 80 if file_name.endswith(".sh") else 120 max_length = 80 if file_name.endswith(".sh") else 120
if len(line.rstrip('\n')) > max_length and not is_unsplittable(file_name, line): if len(line.rstrip(b'\n')) > max_length and not is_unsplittable(file_name, line):
yield (idx + 1, "Line is longer than %d characters" % max_length) yield (idx + 1, "Line is longer than %d characters" % max_length)
@ -279,38 +279,38 @@ def is_unsplittable(file_name, line):
return ( return (
contains_url(line) or contains_url(line) or
file_name.endswith(".rs") and file_name.endswith(".rs") and
line.startswith("use ") and line.startswith(b"use ") and
"{" not in line b"{" not in line
) )
def check_whatwg_specific_url(idx, line): def check_whatwg_specific_url(idx, line):
match = re.search(r"https://html\.spec\.whatwg\.org/multipage/[\w-]+\.html#([\w\:-]+)", line) match = re.search(br"https://html\.spec\.whatwg\.org/multipage/[\w-]+\.html#([\w\:-]+)", line)
if match is not None: if match is not None:
preferred_link = "https://html.spec.whatwg.org/multipage/#{}".format(match.group(1)) preferred_link = "https://html.spec.whatwg.org/multipage/#{}".format(match.group(1))
yield (idx + 1, "link to WHATWG may break in the future, use this format instead: {}".format(preferred_link)) yield (idx + 1, "link to WHATWG may break in the future, use this format instead: {}".format(preferred_link))
def check_whatwg_single_page_url(idx, line): def check_whatwg_single_page_url(idx, line):
match = re.search(r"https://html\.spec\.whatwg\.org/#([\w\:-]+)", line) match = re.search(br"https://html\.spec\.whatwg\.org/#([\w\:-]+)", line)
if match is not None: if match is not None:
preferred_link = "https://html.spec.whatwg.org/multipage/#{}".format(match.group(1)) preferred_link = "https://html.spec.whatwg.org/multipage/#{}".format(match.group(1))
yield (idx + 1, "links to WHATWG single-page url, change to multi page: {}".format(preferred_link)) yield (idx + 1, "links to WHATWG single-page url, change to multi page: {}".format(preferred_link))
def check_whitespace(idx, line): def check_whitespace(idx, line):
if line[-1] == "\n": if line.endswith(b"\n"):
line = line[:-1] line = line[:-1]
else: else:
yield (idx + 1, "no newline at EOF") yield (idx + 1, "no newline at EOF")
if line.endswith(" "): if line.endswith(b" "):
yield (idx + 1, "trailing whitespace") yield (idx + 1, "trailing whitespace")
if "\t" in line: if b"\t" in line:
yield (idx + 1, "tab on line") yield (idx + 1, "tab on line")
if "\r" in line: if b"\r" in line:
yield (idx + 1, "CR on line") yield (idx + 1, "CR on line")
@ -339,7 +339,7 @@ def check_flake8(file_name, contents):
output = "" output = ""
try: try:
args = ["flake8", "--ignore=" + ",".join(ignore), file_name] args = ["flake8", "--ignore=" + ",".join(ignore), file_name]
subprocess.check_output(args) subprocess.check_output(args, universal_newlines=True)
except subprocess.CalledProcessError as e: except subprocess.CalledProcessError as e:
output = e.output output = e.output
for error in output.splitlines(): for error in output.splitlines():
@ -360,7 +360,7 @@ def check_lock(file_name, contents):
# Package names to be neglected (as named by cargo) # Package names to be neglected (as named by cargo)
exceptions = config["ignore"]["packages"] exceptions = config["ignore"]["packages"]
content = toml.loads(contents) content = toml.loads(contents.decode("utf-8"))
packages_by_name = {} packages_by_name = {}
for package in content.get("package", []): for package in content.get("package", []):
@ -431,7 +431,7 @@ def check_toml(file_name, lines):
if not file_name.endswith("Cargo.toml"): if not file_name.endswith("Cargo.toml"):
raise StopIteration raise StopIteration
ok_licensed = False ok_licensed = False
for idx, line in enumerate(lines): for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
if idx == 0 and "[workspace]" in line: if idx == 0 and "[workspace]" in line:
raise StopIteration raise StopIteration
line_without_comment, _, _ = line.partition("#") line_without_comment, _, _ = line.partition("#")
@ -447,7 +447,7 @@ def check_shell(file_name, lines):
if not file_name.endswith(".sh"): if not file_name.endswith(".sh"):
raise StopIteration raise StopIteration
shebang = "#!/usr/bin/env bash" shebang = b"#!/usr/bin/env bash"
required_options = {"set -o errexit", "set -o nounset", "set -o pipefail"} required_options = {"set -o errexit", "set -o nounset", "set -o pipefail"}
did_shebang_check = False did_shebang_check = False
@ -459,10 +459,10 @@ def check_shell(file_name, lines):
if lines[0].rstrip() != shebang: if lines[0].rstrip() != shebang:
yield (1, 'script does not have shebang "{}"'.format(shebang)) yield (1, 'script does not have shebang "{}"'.format(shebang))
for idx in range(1, len(lines)): for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines[1:])):
stripped = lines[idx].rstrip() stripped = line.rstrip()
# Comments or blank lines are ignored. (Trailing whitespace is caught with a separate linter.) # Comments or blank lines are ignored. (Trailing whitespace is caught with a separate linter.)
if lines[idx].startswith("#") or stripped == "": if line.startswith("#") or stripped == "":
continue continue
if not did_shebang_check: if not did_shebang_check:
@ -548,7 +548,7 @@ def check_rust(file_name, lines):
decl_expected = "\n\t\033[93mexpected: {}\033[0m" decl_expected = "\n\t\033[93mexpected: {}\033[0m"
decl_found = "\n\t\033[91mfound: {}\033[0m" decl_found = "\n\t\033[91mfound: {}\033[0m"
for idx, original_line in enumerate(lines): for idx, original_line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
# simplify the analysis # simplify the analysis
line = original_line.strip() line = original_line.strip()
indent = len(original_line) - len(line) indent = len(original_line) - len(line)
@ -661,7 +661,7 @@ def check_rust(file_name, lines):
match = re.search(r"#!\[feature\((.*)\)\]", line) match = re.search(r"#!\[feature\((.*)\)\]", line)
if match: if match:
features = map(lambda w: w.strip(), match.group(1).split(',')) features = list(map(lambda w: w.strip(), match.group(1).split(',')))
sorted_features = sorted(features) sorted_features = sorted(features)
if sorted_features != features and check_alphabetical_order: if sorted_features != features and check_alphabetical_order:
yield(idx + 1, decl_message.format("feature attribute") yield(idx + 1, decl_message.format("feature attribute")
@ -683,7 +683,7 @@ def check_rust(file_name, lines):
# strip /(pub )?mod/ from the left and ";" from the right # strip /(pub )?mod/ from the left and ";" from the right
mod = line[4:-1] if line.startswith("mod ") else line[8:-1] mod = line[4:-1] if line.startswith("mod ") else line[8:-1]
if (idx - 1) < 0 or "#[macro_use]" not in lines[idx - 1]: if (idx - 1) < 0 or "#[macro_use]" not in lines[idx - 1].decode("utf-8"):
match = line.find(" {") match = line.find(" {")
if indent not in prev_mod: if indent not in prev_mod:
prev_mod[indent] = "" prev_mod[indent] = ""
@ -703,7 +703,7 @@ def check_rust(file_name, lines):
# match the derivable traits filtering out macro expansions # match the derivable traits filtering out macro expansions
match = re.search(r"#\[derive\(([a-zA-Z, ]*)", line) match = re.search(r"#\[derive\(([a-zA-Z, ]*)", line)
if match: if match:
derives = map(lambda w: w.strip(), match.group(1).split(',')) derives = list(map(lambda w: w.strip(), match.group(1).split(',')))
# sort, compare and report # sort, compare and report
sorted_derives = sorted(derives) sorted_derives = sorted(derives)
if sorted_derives != derives and check_alphabetical_order: if sorted_derives != derives and check_alphabetical_order:
@ -870,7 +870,7 @@ def check_spec(file_name, lines):
in_impl = False in_impl = False
pattern = "impl {}Methods for {} {{".format(file_name, file_name) pattern = "impl {}Methods for {} {{".format(file_name, file_name)
for idx, line in enumerate(lines): for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
if "// check-tidy: no specs after this line" in line: if "// check-tidy: no specs after this line" in line:
break break
if not patt.match(line): if not patt.match(line):
@ -878,7 +878,7 @@ def check_spec(file_name, lines):
in_impl = True in_impl = True
if ("fn " in line or macro_patt.match(line)) and brace_count == 1: if ("fn " in line or macro_patt.match(line)) and brace_count == 1:
for up_idx in range(1, idx + 1): for up_idx in range(1, idx + 1):
up_line = lines[idx - up_idx] up_line = lines[idx - up_idx].decode("utf-8")
if link_patt.match(up_line): if link_patt.match(up_line):
# Comment with spec link exists # Comment with spec link exists
break break
@ -1023,7 +1023,7 @@ def collect_errors_for_files(files_to_check, checking_functions, line_checking_f
for filename in files_to_check: for filename in files_to_check:
if not os.path.exists(filename): if not os.path.exists(filename):
continue continue
with open(filename, "r") as f: with open(filename, "rb") as f:
contents = f.read() contents = f.read()
if not contents.strip(): if not contents.strip():
yield filename, 0, "file is empty" yield filename, 0, "file is empty"